From nobody Fri May 3 16:37:21 2024 Delivered-To: importer@patchew.org Received-SPF: pass (zohomail.com: domain of groups.io designates 66.175.222.108 as permitted sender) client-ip=66.175.222.108; envelope-from=bounce+27952+78882+1787277+3901457@groups.io; helo=mail02.groups.io; Authentication-Results: mx.zohomail.com; dkim=pass; spf=pass (zohomail.com: domain of groups.io designates 66.175.222.108 as permitted sender) smtp.mailfrom=bounce+27952+78882+1787277+3901457@groups.io; dmarc=fail(p=none dis=none) header.from=posteo.de ARC-Seal: i=1; a=rsa-sha256; t=1628451636; cv=none; d=zohomail.com; s=zohoarc; b=Vc+EYhtDPZl55VNHpLWN9aJXrRDHjVkdQYz5LSOC0IGu26mfdkYDjPZ29+K+cS+QGoSrCA9gGN9tieaBgpcRProcBvg2++8k7KDcFFg5csnCR3n9gnyNY4VD71IcQbNPmrTnShWamrEOnoVYcxzaqnaxr1OwDIQs5G2ujiyVChM= ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=zohomail.com; s=zohoarc; t=1628451636; h=Content-Type:Content-Transfer-Encoding:Cc:Date:From:In-Reply-To:List-Subscribe:List-Id:List-Help:List-Unsubscribe:MIME-Version:Message-ID:Reply-To:References:Sender:Subject:To; bh=Ts+kfDSgjCOh4UPFrWncW+vPjwLKRDeI8cTR/kNpuYk=; b=c5QIyeXWcNkGTRteqLKFRjixELbzBf4vrCWdQHG0lt/vrkM596Q/FE/1yhjCQRrOKcTLpDbK4HPegmShUch44KLWbXQ3zooX6Y9LydTNX1aqH/gtc/pAIQ8R8GQJZUZ9oMASu3e1s/t7LrTyCfH6SVUKXpKtpf27gmgFFsboVdA= ARC-Authentication-Results: i=1; mx.zohomail.com; dkim=pass; spf=pass (zohomail.com: domain of groups.io designates 66.175.222.108 as permitted sender) smtp.mailfrom=bounce+27952+78882+1787277+3901457@groups.io; dmarc=fail header.from= (p=none dis=none) Received: from mail02.groups.io (mail02.groups.io [66.175.222.108]) by mx.zohomail.com with SMTPS id 1628451636276371.81017902008875; Sun, 8 Aug 2021 12:40:36 -0700 (PDT) Return-Path: X-Received: by 127.0.0.2 with SMTP id nC9JYY1788612xQRuvUqnLks; Sun, 08 Aug 2021 12:40:36 -0700 X-Received: from mout01.posteo.de (mout01.posteo.de [185.67.36.65]) by mx.groups.io with SMTP id smtpd.web10.13763.1628451633813222031 for ; Sun, 08 Aug 2021 12:40:34 -0700 X-Received: from submission (posteo.de [89.146.220.130]) by mout01.posteo.de (Postfix) with ESMTPS id 4C296240029 for ; Sun, 8 Aug 2021 21:40:32 +0200 (CEST) X-Received: from customer (localhost [127.0.0.1]) by submission (posteo.de) with ESMTPSA id 4GjV1W3dB9z6tmH; Sun, 8 Aug 2021 21:40:31 +0200 (CEST) From: =?UTF-8?B?TWFydmluIEjDpHVzZXI=?= To: devel@edk2.groups.io Cc: Jian J Wang , Hao A Wu , Dandan Bi , Liming Gao , Vitaly Cheptsov Subject: [edk2-devel] [PATCH] MdeModulePkg/DxeCore: Consistent DebugImageInfoTable updates Date: Sun, 8 Aug 2021 19:39:40 +0000 Message-Id: <88816e99692b15cf61f3057ffab4d54455159c7c.1628443860.git.mhaeuser@posteo.de> In-Reply-To: <5df11a13422732b9c03c120775a2b4dd0a49182f.1628444003.git.mhaeuser@posteo.de> References: <5df11a13422732b9c03c120775a2b4dd0a49182f.1628444003.git.mhaeuser@posteo.de> MIME-Version: 1.0 Precedence: Bulk List-Unsubscribe: List-Subscribe: List-Help: Sender: devel@edk2.groups.io List-Id: Mailing-List: list devel@edk2.groups.io; contact devel+owner@edk2.groups.io Reply-To: devel@edk2.groups.io,mhaeuser@posteo.de X-Gm-Message-State: kfyjqLs8W0RpYdxcnp6GH57lx1787277AA= Content-Transfer-Encoding: quoted-printable DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=groups.io; q=dns/txt; s=20140610; t=1628451636; bh=KvZoVi2ctZWpciZxFqFkWBRpls9p99bt1NC7PDaonkU=; h=Cc:Content-Type:Date:From:Reply-To:Subject:To; b=k0fpIv+bbweCrQYmyzCby4fQyohUj5TxIzUQtJ9Kz7dLEYelCCsOayN1IvR1WKINkOo cSCzi0VcYqRvtWpUY6fFs+c9QvltwaIdc9/Wnm/eg1JyRL2EawM9u4HIZ5Nh5vO2azI2i 3Nizu3ZRtxOUoJ9NnN2BWwY+jUilQQ80980= X-ZohoMail-DKIM: pass (identity @groups.io) X-ZM-MESSAGEID: 1628451637461100021 Content-Type: text/plain; charset="utf-8" In theory, modifications to the DebugImageInfoTable may cause exceptions. If the exception handler parses the table, this can lead to subsequent exceptions if the table state is inconsistent. Ensure the DebugImageInfoTable remains consistent during modifications. This includes: 1) Free the old table only only after the new table has been published. Mitigates use-after-free of the old table. 2) Do not insert an image entry till it is fully initialised. Entries may be inserted in the live range if an entry was deleted previously. Mitigaes the usage of inconsistent entries. 3) Free the old image entry only after the table has been updated with the NULL value. Mitigates use-after-free of the old entry. 4) Set the MODIFIED state before performing any modifications. Cc: Jian J Wang Cc: Hao A Wu Cc: Dandan Bi Cc: Liming Gao Cc: Vitaly Cheptsov Signed-off-by: Marvin H=C3=A4user --- MdeModulePkg/Core/Dxe/Misc/DebugImageInfo.c | 60 +++++++++++--------- 1 file changed, 34 insertions(+), 26 deletions(-) diff --git a/MdeModulePkg/Core/Dxe/Misc/DebugImageInfo.c b/MdeModulePkg/Cor= e/Dxe/Misc/DebugImageInfo.c index a75d4158280b..7bd970115111 100644 --- a/MdeModulePkg/Core/Dxe/Misc/DebugImageInfo.c +++ b/MdeModulePkg/Core/Dxe/Misc/DebugImageInfo.c @@ -165,10 +165,11 @@ CoreNewDebugImageInfoEntry ( IN EFI_HANDLE ImageHandle ) { - EFI_DEBUG_IMAGE_INFO *Table; - EFI_DEBUG_IMAGE_INFO *NewTable; - UINTN Index; - UINTN TableSize; + EFI_DEBUG_IMAGE_INFO *Table; + EFI_DEBUG_IMAGE_INFO *NewTable; + UINTN Index; + UINTN TableSize; + EFI_DEBUG_IMAGE_INFO_NORMAL *NormalImage; =20 // // Set the flag indicating that we're in the process of updating the tab= le. @@ -203,14 +204,6 @@ CoreNewDebugImageInfoEntry ( // Copy the old table into the new one // CopyMem (NewTable, Table, TableSize); - // - // Free the old table - // - CoreFreePool (Table); - // - // Update the table header - // - Table =3D NewTable; mDebugInfoTableHeader.EfiDebugImageInfoTable =3D NewTable; // // Enlarge the max table entries and set the first empty entry index to @@ -218,24 +211,34 @@ CoreNewDebugImageInfoEntry ( // Index =3D mMaxTableEntries; mMaxTableEntries +=3D EFI_PAGE_SIZE / EFI_DEBUG_TABLE_ENTRY_SIZE; + // + // Free the old table + // + CoreFreePool (Table); + // + // Update the table header + // + Table =3D NewTable; } =20 // // Allocate data for new entry // - Table[Index].NormalImage =3D AllocateZeroPool (sizeof (EFI_DEBUG_IMAGE_I= NFO_NORMAL)); - if (Table[Index].NormalImage !=3D NULL) { + NormalImage =3D AllocateZeroPool (sizeof (EFI_DEBUG_IMAGE_INFO_NORMAL)); + if (NormalImage !=3D NULL) { // // Update the entry // - Table[Index].NormalImage->ImageInfoType =3D (UINT32) Ima= geInfoType; - Table[Index].NormalImage->LoadedImageProtocolInstance =3D LoadedImage; - Table[Index].NormalImage->ImageHandle =3D ImageHandle; + NormalImage->ImageInfoType =3D (UINT32) ImageInfoType; + NormalImage->LoadedImageProtocolInstance =3D LoadedImage; + NormalImage->ImageHandle =3D ImageHandle; // - // Increase the number of EFI_DEBUG_IMAGE_INFO elements and set the mD= ebugInfoTable in modified status. + // Set the mDebugInfoTable in modified status, insert the entry, and + // increase the number of EFI_DEBUG_IMAGE_INFO elements. // - mDebugInfoTableHeader.TableSize++; mDebugInfoTableHeader.UpdateStatus |=3D EFI_DEBUG_IMAGE_INFO_TABLE_MOD= IFIED; + Table[Index].NormalImage =3D NormalImage; + mDebugInfoTableHeader.TableSize++; } mDebugInfoTableHeader.UpdateStatus &=3D ~EFI_DEBUG_IMAGE_INFO_UPDATE_IN_= PROGRESS; } @@ -253,8 +256,9 @@ CoreRemoveDebugImageInfoEntry ( EFI_HANDLE ImageHandle ) { - EFI_DEBUG_IMAGE_INFO *Table; - UINTN Index; + EFI_DEBUG_IMAGE_INFO *Table; + UINTN Index; + EFI_DEBUG_IMAGE_INFO_NORMAL *NormalImage; =20 mDebugInfoTableHeader.UpdateStatus |=3D EFI_DEBUG_IMAGE_INFO_UPDATE_IN_P= ROGRESS; =20 @@ -263,16 +267,20 @@ CoreRemoveDebugImageInfoEntry ( for (Index =3D 0; Index < mMaxTableEntries; Index++) { if (Table[Index].NormalImage !=3D NULL && Table[Index].NormalImage->Im= ageHandle =3D=3D ImageHandle) { // - // Found a match. Free up the record, then NULL the pointer to indic= ate the slot - // is free. + // Found a match. Set the mDebugInfoTable in modified status and NUL= L the + // pointer to indicate the slot is free and. // - CoreFreePool (Table[Index].NormalImage); + NormalImage =3D Table[Index].NormalImage; + mDebugInfoTableHeader.UpdateStatus |=3D EFI_DEBUG_IMAGE_INFO_TABLE_M= ODIFIED; Table[Index].NormalImage =3D NULL; // - // Decrease the number of EFI_DEBUG_IMAGE_INFO elements and set the = mDebugInfoTable in modified status. + // Decrease the number of EFI_DEBUG_IMAGE_INFO elements. // mDebugInfoTableHeader.TableSize--; - mDebugInfoTableHeader.UpdateStatus |=3D EFI_DEBUG_IMAGE_INFO_TABLE_M= ODIFIED; + // + // Free up the record. + // + CoreFreePool (NormalImage); break; } } --=20 2.31.1 -=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#78882): https://edk2.groups.io/g/devel/message/78882 Mute This Topic: https://groups.io/mt/84754054/1787277 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org] -=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-