Possible UEFI Shell DrvDiag Command Error

Possible UEFI Shell DrvDiag Command Error

[Jim.Dailey]

Although this question is about the shell's drvdiag command, the answer
is related to the interpretation of the UEFI specification with respect
to the diagnostics protocol's RunDiagnostics() function.

Ultimately, the question is, is the drvdiag command in error in trying
to free the ErrorType pointer, or is a driver vendor in error when they
return an ErrorType parameter that does not point to memory that was
allocated using gBS->AllocatePool?

The diagnostics protocol's RunDiagnostics() function is partially
described in the UEFI spec like this:

  typedef
  EFI_STATUS
  (EFIAPI *EFI_DRIVER_DIAGNOSTICS2_RUN_DIAGNOSTICS) (
    IN EFI_DRIVER_DIAGNOSTICS2_PROTOCOL *This,
    ...
    OUT EFI_GUID                        **ErrorType,
    OUT UINTN                           *BufferSize,
    OUT CHAR16                          **Buffer
    );

  ...

  ErrorType       A GUID that defines the format of the data returned in
                  Buffer.
  BufferSize      The size, in bytes, of the data returned in Buffer.
  Buffer          A buffer that contains a Null-terminated string plus
                  some additional data whose format is defined by
                  ErrorType. Buffer is allocated by this function with
                  EFI_BOOT_SERVICES.AllocatePool(), and it is the
                  caller's responsibility to free it with a call to
                  EFI_BOOT_SERVICES.FreePool().

After a call to RunDiagnostics, the drvdiag command attempts to free the
ErrorType pointer if it is non-NULL, yet nothing in the UEFI spec says
that this is proper.

123456789012345678901234567890123456789012345678901234567890123456789012
The UEFI spec goes to great length to point out that *Buffer* must be
allocated by the callee using gBS->AllocatePool and that the caller must
use gBS->FreePool to release the memory once it is no longer needed.
However, there is no such language with respect to ErrorType.

So, repeating the question, is the drvdiag command in error in trying
to free the ErrorType pointer, or is a driver vendor in error when they
return an ErrorType parameter that does not point to memory that was
allocated using gBS->AllocatePool?

Regards,
Jim

Re: Possible UEFI Shell DrvDiag Command Error

[Carsey, Jaben]

I would say the shell should not free the GUID.

> -----Original Message-----
> From: Jim.Dailey@dell.com [mailto:Jim.Dailey@dell.com]
> Sent: Monday, April 24, 2017 5:46 AM
> To: edk2-devel@lists.01.org
> Cc: Ni, Ruiyu <ruiyu.ni@intel.com>; Carsey, Jaben <jaben.carsey@intel.com>
> Subject: [edk2] Possible UEFI Shell DrvDiag Command Error
> Importance: High
> 
> Although this question is about the shell's drvdiag command, the answer
> is related to the interpretation of the UEFI specification with respect
> to the diagnostics protocol's RunDiagnostics() function.
> 
> Ultimately, the question is, is the drvdiag command in error in trying
> to free the ErrorType pointer, or is a driver vendor in error when they
> return an ErrorType parameter that does not point to memory that was
> allocated using gBS->AllocatePool?
> 
> The diagnostics protocol's RunDiagnostics() function is partially
> described in the UEFI spec like this:
> 
>   typedef
>   EFI_STATUS
>   (EFIAPI *EFI_DRIVER_DIAGNOSTICS2_RUN_DIAGNOSTICS) (
>     IN EFI_DRIVER_DIAGNOSTICS2_PROTOCOL *This,
>     ...
>     OUT EFI_GUID                        **ErrorType,
>     OUT UINTN                           *BufferSize,
>     OUT CHAR16                          **Buffer
>     );
> 
>   ...
> 
>   ErrorType       A GUID that defines the format of the data returned in
>                   Buffer.
>   BufferSize      The size, in bytes, of the data returned in Buffer.
>   Buffer          A buffer that contains a Null-terminated string plus
>                   some additional data whose format is defined by
>                   ErrorType. Buffer is allocated by this function with
>                   EFI_BOOT_SERVICES.AllocatePool(), and it is the
>                   caller's responsibility to free it with a call to
>                   EFI_BOOT_SERVICES.FreePool().
> 
> After a call to RunDiagnostics, the drvdiag command attempts to free the
> ErrorType pointer if it is non-NULL, yet nothing in the UEFI spec says
> that this is proper.
> 
> 1234567890123456789012345678901234567890123456789012345678901234567
> 89012
> The UEFI spec goes to great length to point out that *Buffer* must be
> allocated by the callee using gBS->AllocatePool and that the caller must
> use gBS->FreePool to release the memory once it is no longer needed.
> However, there is no such language with respect to ErrorType.
> 
> So, repeating the question, is the drvdiag command in error in trying
> to free the ErrorType pointer, or is a driver vendor in error when they
> return an ErrorType parameter that does not point to memory that was
> allocated using gBS->AllocatePool?
> 
> Regards,
> Jim