From nobody Sun May 5 15:51:47 2024 Delivered-To: importer@patchew.org Received-SPF: none (zoho.com: 198.145.21.10 is neither permitted nor denied by domain of lists.01.org) client-ip=198.145.21.10; envelope-from=edk2-devel-bounces@lists.01.org; helo=ml01.01.org; Authentication-Results: mx.zohomail.com; spf=none (zoho.com: 198.145.21.10 is neither permitted nor denied by domain of lists.01.org) smtp.mailfrom=edk2-devel-bounces@lists.01.org Return-Path: Received: from ml01.01.org (ml01.01.org [198.145.21.10]) by mx.zohomail.com with SMTPS id 1499251584230790.088416604672; Wed, 5 Jul 2017 03:46:24 -0700 (PDT) Received: from [127.0.0.1] (localhost [IPv6:::1]) by ml01.01.org (Postfix) with ESMTP id 9662121A00AF6; Wed, 5 Jul 2017 03:44:41 -0700 (PDT) Received: from mga05.intel.com (mga05.intel.com [192.55.52.43]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id D35AF21A07AB5 for ; Wed, 5 Jul 2017 03:44:39 -0700 (PDT) Received: from orsmga004.jf.intel.com ([10.7.209.38]) by fmsmga105.fm.intel.com with ESMTP; 05 Jul 2017 03:46:18 -0700 Received: from ray-dev.ccr.corp.intel.com ([10.239.9.12]) by orsmga004.jf.intel.com with ESMTP; 05 Jul 2017 03:46:17 -0700 X-Original-To: edk2-devel@lists.01.org X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.40,311,1496127600"; d="scan'208";a="104624252" From: Ruiyu Ni To: edk2-devel@lists.01.org Date: Wed, 5 Jul 2017 18:46:14 +0800 Message-Id: <20170705104614.295620-1-ruiyu.ni@intel.com> X-Mailer: git-send-email 2.12.2.windows.2 Subject: [edk2] [PATCH] MdeModulePkg/DxeCore: Avoid accessing non-owned memory X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.22 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Michael D Kinney , Hao A Wu , Star Zeng , Liming Gao MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable Errors-To: edk2-devel-bounces@lists.01.org Sender: "edk2-devel" X-ZohoMail: RSF_4 Z_629925259 SPT_0 Content-Type: text/plain; charset="utf-8" The patch fixes two kinds of issues in DxeCore that accesses memory which might be freed or owned by other modules. The existing bugs don't cause functionality issue. 1. CoreValidateHandle() checks whether the handle is valid by validating its signature. The proper way is to check whether the handle is in the handle database. 2. CoreDisconnectControllersUsingProtocolInterface() and CoreOpenProtocol() de-reference Link pointer which is already freed. The proper way is to not de-reference the pointer. Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Ruiyu Ni Cc: Michael D Kinney Cc: Liming Gao Cc: Star Zeng Cc: Hao A Wu Reviewed-by: Star Zeng --- MdeModulePkg/Core/Dxe/Hand/Handle.c | 54 ++++++++++++++++++---------------= ---- 1 file changed, 27 insertions(+), 27 deletions(-) diff --git a/MdeModulePkg/Core/Dxe/Hand/Handle.c b/MdeModulePkg/Core/Dxe/Ha= nd/Handle.c index 59b89148c8..8e2f361805 100644 --- a/MdeModulePkg/Core/Dxe/Hand/Handle.c +++ b/MdeModulePkg/Core/Dxe/Hand/Handle.c @@ -72,15 +72,20 @@ CoreValidateHandle ( ) { IHANDLE *Handle; + LIST_ENTRY *Link; =20 - Handle =3D (IHANDLE *)UserHandle; - if (Handle =3D=3D NULL) { + if (UserHandle =3D=3D NULL) { return EFI_INVALID_PARAMETER; } - if (Handle->Signature !=3D EFI_HANDLE_SIGNATURE) { - return EFI_INVALID_PARAMETER; + + for (Link =3D gHandleList.ForwardLink; Link !=3D &gHandleList; Link =3D = Link->ForwardLink) { + Handle =3D CR (Link, IHANDLE, AllHandles, EFI_HANDLE_SIGNATURE); + if (Handle =3D=3D (IHANDLE *) UserHandle) { + return EFI_SUCCESS; + } } - return EFI_SUCCESS; + + return EFI_INVALID_PARAMETER; } =20 =20 @@ -643,19 +648,16 @@ CoreDisconnectControllersUsingProtocolInterface ( // do { ItemFound =3D FALSE; - for ( Link =3D Prot->OpenList.ForwardLink; - (Link !=3D &Prot->OpenList) && !ItemFound; - Link =3D Link->ForwardLink ) { + for (Link =3D Prot->OpenList.ForwardLink; Link !=3D &Prot->OpenList; L= ink =3D Link->ForwardLink) { OpenData =3D CR (Link, OPEN_PROTOCOL_DATA, Link, OPEN_PROTOCOL_DATA_= SIGNATURE); if ((OpenData->Attributes & EFI_OPEN_PROTOCOL_BY_DRIVER) !=3D 0) { - ItemFound =3D TRUE; CoreReleaseProtocolLock (); Status =3D CoreDisconnectController (UserHandle, OpenData->AgentHa= ndle, NULL); CoreAcquireProtocolLock (); - if (EFI_ERROR (Status)) { - ItemFound =3D FALSE; - break; + if (!EFI_ERROR (Status)) { + ItemFound =3D TRUE; } + break; } } } while (ItemFound); @@ -664,21 +666,17 @@ CoreDisconnectControllersUsingProtocolInterface ( // // Attempt to remove BY_HANDLE_PROTOOCL and GET_PROTOCOL and TEST_PROT= OCOL Open List items // - do { - ItemFound =3D FALSE; - for ( Link =3D Prot->OpenList.ForwardLink; - (Link !=3D &Prot->OpenList) && !ItemFound; - Link =3D Link->ForwardLink ) { - OpenData =3D CR (Link, OPEN_PROTOCOL_DATA, Link, OPEN_PROTOCOL_DAT= A_SIGNATURE); - if ((OpenData->Attributes & - (EFI_OPEN_PROTOCOL_BY_HANDLE_PROTOCOL | EFI_OPEN_PROTOCOL_GET_= PROTOCOL | EFI_OPEN_PROTOCOL_TEST_PROTOCOL)) !=3D 0) { - ItemFound =3D TRUE; - RemoveEntryList (&OpenData->Link); - Prot->OpenListCount--; - CoreFreePool (OpenData); - } + for (Link =3D Prot->OpenList.ForwardLink; Link !=3D &Prot->OpenList;) { + OpenData =3D CR (Link, OPEN_PROTOCOL_DATA, Link, OPEN_PROTOCOL_DATA_= SIGNATURE); + if ((OpenData->Attributes & + (EFI_OPEN_PROTOCOL_BY_HANDLE_PROTOCOL | EFI_OPEN_PROTOCOL_GET_PR= OTOCOL | EFI_OPEN_PROTOCOL_TEST_PROTOCOL)) !=3D 0) { + Link =3D RemoveEntryList (&OpenData->Link); + Prot->OpenListCount--; + CoreFreePool (OpenData); + } else { + Link =3D Link->ForwardLink; } - } while (ItemFound); + } } =20 // @@ -1132,7 +1130,7 @@ CoreOpenProtocol ( if (ByDriver) { do { Disconnect =3D FALSE; - for ( Link =3D Prot->OpenList.ForwardLink; (Link !=3D &Prot->OpenL= ist) && (!Disconnect); Link =3D Link->ForwardLink) { + for (Link =3D Prot->OpenList.ForwardLink; Link !=3D &Prot->OpenLis= t; Link =3D Link->ForwardLink) { OpenData =3D CR (Link, OPEN_PROTOCOL_DATA, Link, OPEN_PROTOCOL_D= ATA_SIGNATURE); if ((OpenData->Attributes & EFI_OPEN_PROTOCOL_BY_DRIVER) !=3D 0)= { Disconnect =3D TRUE; @@ -1142,6 +1140,8 @@ CoreOpenProtocol ( if (EFI_ERROR (Status)) { Status =3D EFI_ACCESS_DENIED; goto Done; + } else { + break; } } } --=20 2.12.2.windows.2 _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel