From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mout01.posteo.de (mout01.posteo.de [185.67.36.65]) by mx.groups.io with SMTP id smtpd.web10.19056.1628488048305362048 for ; Sun, 08 Aug 2021 22:47:29 -0700 Authentication-Results: mx.groups.io; dkim=fail reason="body hash did not verify" header.i=@posteo.de header.s=2017 header.b=BobHkqjt; spf=pass (domain: posteo.de, ip: 185.67.36.65, mailfrom: mhaeuser@posteo.de) Received: from submission (posteo.de [89.146.220.130]) by mout01.posteo.de (Postfix) with ESMTPS id 7A15C240027 for ; Mon, 9 Aug 2021 07:47:26 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=posteo.de; s=2017; t=1628488046; bh=fRB0hFwHQmrfNxZAVgY3MSrFW9if7OBw+8dbAZgC+Qs=; h=Subject:To:Cc:From:Date:From; b=BobHkqjtNJycKMV2AkKx2a4AuuidqZoH2FfQl5WS4Yn+v10CaYf1+YYNhGofaHIsp g5zl+QZlxzBrfvbBkeH9f5WDRxIueNNfbxmjtrkidsqmaQFD1w/dV2VQLpFRsOQO7L Xd9hREneTphAgGcOABDuBIopOFO/7W1YewfMcsKBcPIjIr+3a8LCZIJHRpGkibtrSN qPDz/JxXRVHlG3IAa5DQyx02UC889s2p5Ik9fLY59f1LtYLBRhQtTWx1/cbMG6lxCO nGKrXyUcl3ak47+qytWfF/12q+y5ZmzKqu92/HeiaIrIJRpdQInJsKjPSks33hYGnA 69LxtoeUoPYDA== Received: from customer (localhost [127.0.0.1]) by submission (posteo.de) with ESMTPSA id 4GjlTn20pCz6tm5; Mon, 9 Aug 2021 07:47:25 +0200 (CEST) Subject: Re: [PATCH] UefiPayloadPkg/UefiPayloadEntry: Fix memory corruption To: "Ni, Ray" , "devel@edk2.groups.io" Cc: "Dong, Guo" , "Ma, Maurice" , "You, Benjamin" , Vitaly Cheptsov References: <5df11a13422732b9c03c120775a2b4dd0a49182f.1628444003.git.mhaeuser@posteo.de> From: =?UTF-8?B?TWFydmluIEjDpHVzZXI=?= Message-ID: <8c553b1a-042e-661f-681c-844b04c62fce@posteo.de> Date: Mon, 9 Aug 2021 05:47:24 +0000 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-GB Content-Transfer-Encoding: quoted-printable On 09/08/2021 06:20, Ni, Ray wrote: > It's so lucky that no code calls AllocatePool so the bug didn't cause r= eal issues. (I tried to remove AllocatePool() and build still passed.) > > Thanks for catching the bug. Reviewed-by: Ray Ni > > Can you kindly share how you found this issue? Hey Ray, clang-tidy gave me a hand. :) "Suspicious usage of 'sizeof(K)'; did you mean 'K'? clang-tidy(bugprone-sizeof-expression)" I set it up as follows (this is *not* sophisticated, just added things=20 to quickly move on): https://github.com/tianocore/edk2-staging/blob/2021-gsoc-secure-loader/co= mpile_flags.txt Best regards, Marvin > > Thanks, > Ray > > -----Original Message----- > From: Marvin H=C3=A4user > Sent: Monday, August 9, 2021 3:40 AM > To: devel@edk2.groups.io > Cc: Dong, Guo ; Ni, Ray ; Ma, Mau= rice ; You, Benjamin ; Vita= ly Cheptsov > Subject: [PATCH] UefiPayloadPkg/UefiPayloadEntry: Fix memory corruption > > UefiPayloadEntry's AllocatePool() applies the "sizeof" operator to > HOB index rather than the HOB header structure. This yields 4 Bytes > compared to the 8 Bytes the structure header requires. Fix the call > to allocate the required space instead. > > Cc: Guo Dong > Cc: Ray Ni > Cc: Maurice Ma > Cc: Benjamin You > Cc: Vitaly Cheptsov > Signed-off-by: Marvin H=C3=A4user > --- > UefiPayloadPkg/UefiPayloadEntry/MemoryAllocation.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/UefiPayloadPkg/UefiPayloadEntry/MemoryAllocation.c b/UefiP= ayloadPkg/UefiPayloadEntry/MemoryAllocation.c > index 1204573b3e09..f3494969e5ac 100644 > --- a/UefiPayloadPkg/UefiPayloadEntry/MemoryAllocation.c > +++ b/UefiPayloadPkg/UefiPayloadEntry/MemoryAllocation.c > @@ -163,7 +163,7 @@ AllocatePool ( > return NULL; > > } > > =20 > > - Hob =3D (EFI_HOB_MEMORY_POOL *)CreateHob (EFI_HOB_TYPE_MEMORY_POOL, = (UINT16)(sizeof (EFI_HOB_TYPE_MEMORY_POOL) + AllocationSize)); > > + Hob =3D (EFI_HOB_MEMORY_POOL *)CreateHob (EFI_HOB_TYPE_MEMORY_POOL, = (UINT16)(sizeof (EFI_HOB_MEMORY_POOL) + AllocationSize)); > > return (VOID *)(Hob + 1); > > } > > =20 >