Re: [PATCH] MdeModulePkg/SdDxe: Fix potential NULL pointer access

[Jeff Brasen <jbrasen@nvidia.com>]

Sounds good, will upload a v2 patch with that.

Thanks,
Jeff

-----Original Message-----
From: Wu, Hao A <hao.a.wu@intel.com> 
Sent: Thursday, November 15, 2018 9:33 PM
To: Jeff Brasen <jbrasen@nvidia.com>; edk2-devel@lists.01.org
Subject: RE: [edk2] [PATCH] MdeModulePkg/SdDxe: Fix potential NULL pointer access

> -----Original Message-----
> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of 
> Jeff Brasen
> Sent: Saturday, November 10, 2018 1:17 AM
> To: edk2-devel@lists.01.org
> Cc: Jeff Brasen
> Subject: [edk2] [PATCH] MdeModulePkg/SdDxe: Fix potential NULL pointer 
> access
> 
> SdReadWrite can be called with a NULL Token for synchronous operations.
> Add guard for DEBUG print to only print event pointer with Token is 
> not NULL.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Jeff Brasen <jbrasen@nvidia.com>
> ---
>  MdeModulePkg/Bus/Sd/SdDxe/SdBlockIo.c | 7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/MdeModulePkg/Bus/Sd/SdDxe/SdBlockIo.c
> b/MdeModulePkg/Bus/Sd/SdDxe/SdBlockIo.c
> index b8d115a..920e3cd 100644
> --- a/MdeModulePkg/Bus/Sd/SdDxe/SdBlockIo.c
> +++ b/MdeModulePkg/Bus/Sd/SdDxe/SdBlockIo.c
> @@ -670,8 +670,11 @@ SdReadWrite (
>      if (EFI_ERROR (Status)) {
>        return Status;
>      }
> -    DEBUG ((DEBUG_BLKIO, "Sd%a(): Lba 0x%x BlkNo 0x%x Event %p with %r\n",
> IsRead ? "Read" : "Write", Lba, BlockNum, Token->Event, Status));
> -
> +    if (Token != NULL) {
> +      DEBUG ((DEBUG_BLKIO, "Sd%a(): Lba 0x%x BlkNo 0x%x Event %p
> with %r\n", IsRead ? "Read" : "Write", Lba, BlockNum, Token->Event, 
> Status));
> +    } else {
> +      DEBUG ((DEBUG_BLKIO, "Sd%a(): Lba 0x%x BlkNo 0x%x with %r\n",
> IsRead ? "Read" : "Write", Lba, BlockNum, Status));
> +    }

How about:

    DEBUG ((DEBUG_BLKIO,
      "Sd%a(): Lba 0x%x BlkNo 0x%x Event %p with %r\n",
      IsRead ? "Read" : "Write", Lba, BlockNum,
      (Token != NULL) ? Token->Event : NULL, Status));

which is similar to the debug messages in the EMMC counterpart function
EmmcReadWrite() in file \MdeModulePkg\Bus\Sd\EmmcDxe\EmmcBlockIo.c.

Best Regards,
Hao Wu

>      Lba   += BlockNum;
>      Buffer = (UINT8*)Buffer + BufferSize;
>      Remaining -= BlockNum;
> --
> 2.7.4
> 
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
-----------------------------------------------------------------------------------
This email message is for the sole use of the intended recipient(s) and may contain
confidential information.  Any unauthorized review, use, disclosure or distribution
is prohibited.  If you are not the intended recipient, please contact the sender by
reply email and destroy all copies of the original message.
-----------------------------------------------------------------------------------