public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* [PATCH] MdeModulePkg/SdDxe: Fix potential NULL pointer access
@ 2018-11-09 17:17 Jeff Brasen
  2018-11-16  5:33 ` Wu, Hao A
  0 siblings, 1 reply; 3+ messages in thread
From: Jeff Brasen @ 2018-11-09 17:17 UTC (permalink / raw)
  To: edk2-devel; +Cc: Jeff Brasen

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));
+    }
     Lba   += BlockNum;
     Buffer = (UINT8*)Buffer + BufferSize;
     Remaining -= BlockNum;
-- 
2.7.4



^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH] MdeModulePkg/SdDxe: Fix potential NULL pointer access
  2018-11-09 17:17 [PATCH] MdeModulePkg/SdDxe: Fix potential NULL pointer access Jeff Brasen
@ 2018-11-16  5:33 ` Wu, Hao A
  2018-11-16  6:11   ` Jeff Brasen
  0 siblings, 1 reply; 3+ messages in thread
From: Wu, Hao A @ 2018-11-16  5:33 UTC (permalink / raw)
  To: Jeff Brasen, edk2-devel@lists.01.org

> -----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


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] MdeModulePkg/SdDxe: Fix potential NULL pointer access
  2018-11-16  5:33 ` Wu, Hao A
@ 2018-11-16  6:11   ` Jeff Brasen
  0 siblings, 0 replies; 3+ messages in thread
From: Jeff Brasen @ 2018-11-16  6:11 UTC (permalink / raw)
  To: Wu, Hao A, edk2-devel@lists.01.org

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.
-----------------------------------------------------------------------------------


^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2018-11-16  6:11 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-11-09 17:17 [PATCH] MdeModulePkg/SdDxe: Fix potential NULL pointer access Jeff Brasen
2018-11-16  5:33 ` Wu, Hao A
2018-11-16  6:11   ` Jeff Brasen

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox