[edk2-devel] [edk2-libc Patch] StdLib/LibC/StdLib: Handle possible math overflow in malloc()

Michael D Kinney posted 1 patch 2 years, 7 months ago
Failed in applying to current master (apply log)
StdLib/LibC/StdLib/Malloc.c | 6 ++++++
1 file changed, 6 insertions(+)
[edk2-devel] [edk2-libc Patch] StdLib/LibC/StdLib: Handle possible math overflow in malloc()
Posted by Michael D Kinney 2 years, 7 months ago
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1510

Check for addition overflow in malloc() when computing NodeSize
and return error if overflow is detected.

Cc: Rebecca Cran <rebecca@nuviainc.com>
Cc: Yitzhak Briskman <yitzhak.briskman@intel.com>
Cc: Jian J Wang <jian.j.wang@intel.com>
Cc: Yonghong Zhu <yonghong.zhu@intel.com>
Signed-off-by: Michael D Kinney <michael.d.kinney@intel.com>
---
 StdLib/LibC/StdLib/Malloc.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/StdLib/LibC/StdLib/Malloc.c b/StdLib/LibC/StdLib/Malloc.c
index c131b9e..7bf8827 100644
--- a/StdLib/LibC/StdLib/Malloc.c
+++ b/StdLib/LibC/StdLib/Malloc.c
@@ -94,6 +94,12 @@ malloc(size_t Size)
     return NULL;
   }
 
+  if ((Size + sizeof(CPOOL_HEAD)) < Size) {
+    RetVal  = NULL;
+    errno   = ENOMEM;
+    DEBUG((DEBUG_ERROR, "\nERROR malloc: Size overflow\n"));
+  }
+
   NodeSize = (UINTN)(Size + sizeof(CPOOL_HEAD));
 
   DEBUG((DEBUG_POOL, "malloc(%d): NodeSz: %d", Size, NodeSize));
-- 
2.32.0.windows.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#79538): https://edk2.groups.io/g/devel/message/79538
Mute This Topic: https://groups.io/mt/84983903/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-


Re: [edk2-devel] [edk2-libc Patch] StdLib/LibC/StdLib: Handle possible math overflow in malloc()
Posted by Rebecca Cran 2 years, 7 months ago
Reviewed-by: Rebecca Cran <rebecca@nuviainc.com>


Not sure the first \n is needed though.


-- 

Rebecca Cran


On 8/18/21 4:03 PM, Michael D Kinney wrote:
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1510
>
> Check for addition overflow in malloc() when computing NodeSize
> and return error if overflow is detected.
>
> Cc: Rebecca Cran <rebecca@nuviainc.com>
> Cc: Yitzhak Briskman <yitzhak.briskman@intel.com>
> Cc: Jian J Wang <jian.j.wang@intel.com>
> Cc: Yonghong Zhu <yonghong.zhu@intel.com>
> Signed-off-by: Michael D Kinney <michael.d.kinney@intel.com>
> ---
>   StdLib/LibC/StdLib/Malloc.c | 6 ++++++
>   1 file changed, 6 insertions(+)
>
> diff --git a/StdLib/LibC/StdLib/Malloc.c b/StdLib/LibC/StdLib/Malloc.c
> index c131b9e..7bf8827 100644
> --- a/StdLib/LibC/StdLib/Malloc.c
> +++ b/StdLib/LibC/StdLib/Malloc.c
> @@ -94,6 +94,12 @@ malloc(size_t Size)
>       return NULL;
>     }
>   
> +  if ((Size + sizeof(CPOOL_HEAD)) < Size) {
> +    RetVal  = NULL;
> +    errno   = ENOMEM;
> +    DEBUG((DEBUG_ERROR, "\nERROR malloc: Size overflow\n"));
> +  }
> +
>     NodeSize = (UINTN)(Size + sizeof(CPOOL_HEAD));
>   
>     DEBUG((DEBUG_POOL, "malloc(%d): NodeSz: %d", Size, NodeSize));


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#79540): https://edk2.groups.io/g/devel/message/79540
Mute This Topic: https://groups.io/mt/84983903/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-


Re: [edk2-devel] [edk2-libc Patch] StdLib/LibC/StdLib: Handle possible math overflow in malloc()
Posted by Michael D Kinney 2 years, 7 months ago
I just followed the DEBUG() message style in the rest of the malloc() function.

It does guarantee that the message starts at the left column, which is important
if the same console is used for both stdout and DEBUG() which can happen for
UEFI Applications.

PEI/DXE/MM/SMM components may have a dedicated device for DEBUG() messages or
may run before the local console is initialized, so the extra \n is not needed.

Mike

> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Rebecca Cran
> Sent: Wednesday, August 18, 2021 3:08 PM
> To: devel@edk2.groups.io; Kinney, Michael D <michael.d.kinney@intel.com>
> Cc: Briskman, Yitzhak <yitzhak.briskman@intel.com>; Wang, Jian J <jian.j.wang@intel.com>; Zhu, Yonghong
> <yonghong.zhu@intel.com>
> Subject: Re: [edk2-devel] [edk2-libc Patch] StdLib/LibC/StdLib: Handle possible math overflow in malloc()
> 
> Reviewed-by: Rebecca Cran <rebecca@nuviainc.com>
> 
> 
> Not sure the first \n is needed though.
> 
> 
> --
> 
> Rebecca Cran
> 
> 
> On 8/18/21 4:03 PM, Michael D Kinney wrote:
> > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1510
> >
> > Check for addition overflow in malloc() when computing NodeSize
> > and return error if overflow is detected.
> >
> > Cc: Rebecca Cran <rebecca@nuviainc.com>
> > Cc: Yitzhak Briskman <yitzhak.briskman@intel.com>
> > Cc: Jian J Wang <jian.j.wang@intel.com>
> > Cc: Yonghong Zhu <yonghong.zhu@intel.com>
> > Signed-off-by: Michael D Kinney <michael.d.kinney@intel.com>
> > ---
> >   StdLib/LibC/StdLib/Malloc.c | 6 ++++++
> >   1 file changed, 6 insertions(+)
> >
> > diff --git a/StdLib/LibC/StdLib/Malloc.c b/StdLib/LibC/StdLib/Malloc.c
> > index c131b9e..7bf8827 100644
> > --- a/StdLib/LibC/StdLib/Malloc.c
> > +++ b/StdLib/LibC/StdLib/Malloc.c
> > @@ -94,6 +94,12 @@ malloc(size_t Size)
> >       return NULL;
> >     }
> >
> > +  if ((Size + sizeof(CPOOL_HEAD)) < Size) {
> > +    RetVal  = NULL;
> > +    errno   = ENOMEM;
> > +    DEBUG((DEBUG_ERROR, "\nERROR malloc: Size overflow\n"));
> > +  }
> > +
> >     NodeSize = (UINTN)(Size + sizeof(CPOOL_HEAD));
> >
> >     DEBUG((DEBUG_POOL, "malloc(%d): NodeSz: %d", Size, NodeSize));
> 
> 
> 
> 



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#79541): https://edk2.groups.io/g/devel/message/79541
Mute This Topic: https://groups.io/mt/84983903/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-