Re: [PATCH 1/1] ShellPkgDisconnect: zero-initialize handles

["Gerd Hoffmann" <kraxel@redhat.com>]

Ping.
Any comments on this?

On Wed, Apr 26, 2023 at 01:39:27PM +0200, Gerd Hoffmann wrote:
> In case ShellConvertStringToUint64() fails the Handles are left
> uninitialized.  That can for example happen for Handle2 and Handle3 in
> case only one parameter was specified on the command line.  Which can
> trigger the ASSERT() in line 185.
> 
> Reproducer: boot ovmf to efi shell in qemu, using q35 machine type, then
> try disconnect the sata controller in efi shell.
> 
> Fix that by explicitly setting them to NULL in that case.  While being
> at it also simplify the logic and avoid pointlessly calling
> ShellConvertStringToUint64() in case ParamN is NULL.
> 
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> ---
>  .../UefiShellDriver1CommandsLib/Disconnect.c  | 19 +++++++++++++------
>  1 file changed, 13 insertions(+), 6 deletions(-)
> 
> diff --git a/ShellPkg/Library/UefiShellDriver1CommandsLib/Disconnect.c b/ShellPkg/Library/UefiShellDriver1CommandsLib/Disconnect.c
> index fd49d1f7ceb4..fac6463e3c28 100644
> --- a/ShellPkg/Library/UefiShellDriver1CommandsLib/Disconnect.c
> +++ b/ShellPkg/Library/UefiShellDriver1CommandsLib/Disconnect.c
> @@ -160,16 +160,23 @@ ShellCommandRunDisconnect (
>          Param1 = ShellCommandLineGetRawValue (Package, 1);
>          Param2 = ShellCommandLineGetRawValue (Package, 2);
>          Param3 = ShellCommandLineGetRawValue (Package, 3);
> -        if (!EFI_ERROR (ShellConvertStringToUint64 (Param1, &Intermediate1, TRUE, FALSE))) {
> -          Handle1 = Param1 != NULL ? ConvertHandleIndexToHandle ((UINTN)Intermediate1) : NULL;
> +
> +        if (Param1 && !EFI_ERROR (ShellConvertStringToUint64 (Param1, &Intermediate1, TRUE, FALSE))) {
> +          Handle1 = ConvertHandleIndexToHandle ((UINTN)Intermediate1);
> +        } else {
> +          Handle1 = NULL;
>          }
>  
> -        if (!EFI_ERROR (ShellConvertStringToUint64 (Param2, &Intermediate2, TRUE, FALSE))) {
> -          Handle2 = Param2 != NULL ? ConvertHandleIndexToHandle ((UINTN)Intermediate2) : NULL;
> +        if (Param2 && !EFI_ERROR (ShellConvertStringToUint64 (Param2, &Intermediate2, TRUE, FALSE))) {
> +          Handle2 = ConvertHandleIndexToHandle ((UINTN)Intermediate2);
> +        } else {
> +          Handle2 = NULL;
>          }
>  
> -        if (!EFI_ERROR (ShellConvertStringToUint64 (Param3, &Intermediate3, TRUE, FALSE))) {
> -          Handle3 = Param3 != NULL ? ConvertHandleIndexToHandle ((UINTN)Intermediate3) : NULL;
> +        if (Param3 && !EFI_ERROR (ShellConvertStringToUint64 (Param3, &Intermediate3, TRUE, FALSE))) {
> +          Handle3 = ConvertHandleIndexToHandle ((UINTN)Intermediate3);
> +        } else {
> +          Handle3 = NULL;
>          }
>  
>          if ((Param1 != NULL) && (Handle1 == NULL)) {
> -- 
> 2.40.0
> 

--