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

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

[Michael D Kinney]

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

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

[Rebecca Cran]

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

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

[Michael D Kinney]

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