From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail02.groups.io (mail02.groups.io [66.175.222.108]) by spool.mail.gandi.net (Postfix) with ESMTPS id B24C874003C for ; Thu, 31 Aug 2023 17:33:48 +0000 (UTC) DKIM-Signature: a=rsa-sha256; bh=9OVyCwB8LB3Pp30Yq/bandq8IOsU7jlWiRwdyvjzLrY=; c=relaxed/simple; d=groups.io; h=ARC-Seal:ARC-Message-Signature:ARC-Authentication-Results:From:To:CC:Subject:Thread-Topic:Thread-Index:Date:Message-ID:References:In-Reply-To:Accept-Language:MIME-Version:Precedence:List-Subscribe:List-Help:Sender:List-Id:Mailing-List:Delivered-To:Reply-To:List-Unsubscribe-Post:List-Unsubscribe:Content-Language:Content-Type:Content-Transfer-Encoding; s=20140610; t=1693503227; v=1; b=ezzyBo0u5c3p/KhMoKIOPZEFnuAY1qaDx09c4zQFSKjrcEigRqa16eSO/joE5cKmEivOXjFY YTlj9CZPZBr9j0MIgJV6XLwvsCThz4WL7X04HfUgW8CbtuwH0JJ4+haOLyJFz2yuNwOucNlBMdr VA8gDrJdhb5V2tCP83xNbWPU= X-Received: by 127.0.0.2 with SMTP id LhIBYY7687511xnsidSEeLEy; Thu, 31 Aug 2023 10:33:47 -0700 X-Received: from mgamail.intel.com (mgamail.intel.com [134.134.136.100]) by mx.groups.io with SMTP id smtpd.web11.859.1693503226758359534 for ; Thu, 31 Aug 2023 10:33:46 -0700 X-IronPort-AV: E=McAfee;i="6600,9927,10819"; a="442409985" X-IronPort-AV: E=Sophos;i="6.02,217,1688454000"; d="scan'208";a="442409985" X-Received: from fmsmga004.fm.intel.com ([10.253.24.48]) by orsmga105.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 31 Aug 2023 10:33:46 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=McAfee;i="6600,9927,10819"; a="809727949" X-IronPort-AV: E=Sophos;i="6.02,217,1688454000"; d="scan'208";a="809727949" X-Received: from fmsmsx602.amr.corp.intel.com ([10.18.126.82]) by fmsmga004.fm.intel.com with ESMTP/TLS/AES256-GCM-SHA384; 31 Aug 2023 10:33:45 -0700 X-Received: from fmsmsx610.amr.corp.intel.com (10.18.126.90) by fmsmsx602.amr.corp.intel.com (10.18.126.82) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2507.27; Thu, 31 Aug 2023 10:33:45 -0700 X-Received: from fmsedg602.ED.cps.intel.com (10.1.192.136) by fmsmsx610.amr.corp.intel.com (10.18.126.90) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2507.27 via Frontend Transport; Thu, 31 Aug 2023 10:33:45 -0700 X-Received: from NAM11-CO1-obe.outbound.protection.outlook.com (104.47.56.170) by edgegateway.intel.com (192.55.55.71) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.1.2507.27; Thu, 31 Aug 2023 10:33:44 -0700 ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=QbA0zpeJCJkcEgmJPO3LyvrmAP7CDZDVlnNmRQVk+IQFwxsRN5JgVq1ZPAGHGeGHwy9Y6NN8rSyGQ8O+C7VdVN9SZGr8oS17gnYI1stC8seXVq9H5amJAPdFS51+xNUq+S/PVCLrv0D/yQEeHrF8CpqCb+YOIgit4DQ9igXicgWfGjUxMfIjpfpH04nJ7vmNFSHhUFEYSWdidPhwKtGfEyIjxxaojBErCHZSt8cfK0dImqJldJ/VXvFx4OCxDBU3eRCV7r761EbMG8f8VYfgK7u/nyjeG+074CjluKClkv0Uc5dfsUQahhBhkw6v3bpJN7nsocr2wSs0PSZP46bS5Q== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=C7XzO+PEolu/RLPVsY2mUgeeN/Zxii7IiXLbAtHOGuo=; b=Cgwa3Pxk6lHeJRWcL9PNUspUtaOwYooJKcBc6esJW6gc58hpEEN+GlZKV/LRpzvrscnqa6VNPhBY+zbqifcjUzNbrwvMXGN7TotfXBNx2Qce5yljfzQnbgiG4LWI8ds3ftYEP3eU22xQ7I4PQDlUXCURjn5TR0vpwK8SezNKkIAupk3JUcmvM6l4jZQrHHn5222cHIzHiICPSjMkVuLi/gROaV1gLVWK/9RFoXT37z19c5la8v6W5EkAzY3Hk3G4jsMKMtuWJNPSsba1vb4EgPKpuM5N32uqkTiFW56wZCWL0Xz6OnCayTLjDACRDga44IZkL/vcg8oL6cKcL0H19A== ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=intel.com; dmarc=pass action=none header.from=intel.com; dkim=pass header.d=intel.com; arc=none X-Received: from CO1PR11MB4929.namprd11.prod.outlook.com (2603:10b6:303:6d::19) by CH3PR11MB7762.namprd11.prod.outlook.com (2603:10b6:610:151::15) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.6699.35; Thu, 31 Aug 2023 17:33:34 +0000 X-Received: from CO1PR11MB4929.namprd11.prod.outlook.com ([fe80::eaa6:1c0:c33f:2a11]) by CO1PR11MB4929.namprd11.prod.outlook.com ([fe80::eaa6:1c0:c33f:2a11%2]) with mapi id 15.20.6699.035; Thu, 31 Aug 2023 17:33:34 +0000 From: "Michael D Kinney" To: "Desimone, Nathaniel L" , "devel@edk2.groups.io" CC: "Gao, Liming" , "Wang, Jian J" , "Bi, Dandan" , "Kinney, Michael D" Subject: Re: [edk2-devel] [PATCH v1] MdeModulePkg: Fix memory leak in LocateHandleBuffer() Thread-Topic: [PATCH v1] MdeModulePkg: Fix memory leak in LocateHandleBuffer() Thread-Index: AQHZ22GrMTTqwXSP+0ay5ZaisevlALAEqhjw Date: Thu, 31 Aug 2023 17:33:34 +0000 Message-ID: References: <20230830164641.588-1-nathaniel.l.desimone@intel.com> In-Reply-To: <20230830164641.588-1-nathaniel.l.desimone@intel.com> Accept-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-ms-publictraffictype: Email x-ms-traffictypediagnostic: CO1PR11MB4929:EE_|CH3PR11MB7762:EE_ x-ms-office365-filtering-correlation-id: 8086d891-9a37-45d1-c58d-08dbaa486716 x-ld-processed: 46c98d88-e344-4ed4-8496-4ed7712e255d,ExtAddr x-ms-exchange-senderadcheck: 1 x-ms-exchange-antispam-relay: 0 x-microsoft-antispam-message-info: JG90gE2h5I9jBRoywuJGh8IweBbyPvoZvnQ6SeDL2b40gK7olQhATycepNZtZuyYOx3H2lInwbFlTtMITTAeRl1kSgErrImXbyxYFkstHutPTPfS7QREeuEmNK1+j3+E9VMztqQ6yOfDyJPPQ689EEhTBniMcfjvcVa7zxe3/L2ZKQ5aGU3ICHLjfzZrPSMK+17bDQNiuS4dxOUrrRSFHeG4UW1cLE7y0f+69P3n/w5DRXvRdfHSphE3aJ/npSLBkus1IqVWOrvdhHK94PfrXKhMUot9/Z6wsPI4lGMH6SjudFpd0vxgt+fn/xgOgdZ9c5NKODwVjA3ggkER8kYO2HwEQp2zT595hBDisksX1UZtnIuAYkbJ9wmlW+vaBaCVfCvMJhviBx2dB3Hw3eX2WDvRgHZPWd6rSm+y9pZY21K+Fivom2n8VqHcuZDboB2RZkjyxUHULrl4qzFPJYmUXWzCglz5NrOTnyeASfQXlel+J44JvMpIUCxGOldJS0tINPlqIk4p0jopOvH6NGPtZSY1G+E2d+/IPGixGrY5QWMiB/JDTYR4G3JHKFut+/eXzCpTYeF5Ds3fmw/KjwzQ8s6ew5Y+aNQlUJKs6yY+6Y+bDq76ytJ9w7uslkijVdLJRqkqeSSZjXJyFa1sGVLivQ== x-ms-exchange-antispam-messagedata-chunkcount: 1 x-ms-exchange-antispam-messagedata-0: =?us-ascii?Q?aDGczlv1n/CgeCNhYcTpgqXciDh6rMkBA2FLD9izyg5nNwFY1PD23blnyu4A?= =?us-ascii?Q?udpVpzm4Tp7y3R+wGZyAt/6R3yl5IMM+Qk9nJLF0yawexXLu/m9EIt3/Nwa6?= =?us-ascii?Q?T1AKbiK7C3NIleDPu9SM/vZUgxnUvICmlmOSY4dsfmeavLGZhUdAElJsQjW0?= =?us-ascii?Q?1AmmP0eZEPc2J5lr6gFhB6JVZSQzfNmp26lFp822/WuU8j5/TN+vBVizQ9+N?= =?us-ascii?Q?DZZBLnIfTzBQWewnfRWU+ulrULQIcje2AkQ4v/Vg5eEvH/dzsHPWdf7cWyTa?= =?us-ascii?Q?VRMcjRUaloQPXDQm5JDZmatIgICmX/NtjiZ4kZUeScPwobtEsG76uiL4u9MF?= =?us-ascii?Q?fqMf8sJckSZwZHa5nWgFrN6HU6DAqPI6hwtvGi4T7cwhYxI6nzkCAFjDvf5D?= =?us-ascii?Q?Vh9dQLNv06fJCdk9n0iGbIvxIsLeaFr3Tq9tLdMmqDsMNA7VH2CAhHG2zukN?= =?us-ascii?Q?p1w0/TfONk1NpWN1yN6e3lpgNxkf341tmvq6SJPTqcsZ0UgCBLw7HNkN/+9C?= =?us-ascii?Q?6NsCGGbXA0kbDzrbVo3bcKJ+3mt4DaS7a7etLvART7eo7+1wOFlSdgW+d0NM?= =?us-ascii?Q?Encc+sf3WArl/AnkyjkY0E9xmIEw4IBX7H6Kvhmr0lfZvLGYELYs6gwOb7Su?= =?us-ascii?Q?0dtb8nnM3IaxKRzbtnxVRzB9ZI2ORFWmDABJ0BmD/F41VxHZxj9ABevYObZb?= =?us-ascii?Q?tawDXnyL98ARpE4W2Uxae010VgPNw0yCyUt7+8s/+lDVliyDA5icvWYmJ3Dw?= =?us-ascii?Q?oYbfddtvyutH5IK9hON8jTDt7RnwTIg5Wsau+EFA0qeLlYc+MhE2dWEpD43J?= =?us-ascii?Q?Acnlj4yayy/45EK3dxZqjs2zoTbIUgq/y1yQT8x1P8urvwLzu3HeUg0Zk3rl?= =?us-ascii?Q?q5lKggKH72HBvsvkXepZKIv26vVWOp7ujsny1fFVgRnu3S/IVR9H8EPu5oLs?= =?us-ascii?Q?dJQBpf89vuPrD7QEa+1EjHJJscrK/RHiDrq1w5gnvvQqz2m99tnDPfBmckyK?= =?us-ascii?Q?4jaPLicrdGFmcHD5K0vikus695sbq9A9NLXyocS22sP01r3dxtZJC0APnuKd?= =?us-ascii?Q?keEDTLYCEDUxPMnTJx19mgoEShJKPzy3u7O1JwEihUr7EF56f42Z4sprn5Po?= =?us-ascii?Q?jeFMxX+pJbxOHEvXS6du+wqc6Yq6SJAkSAuVOMlqM8PYuRb62vUVdDG6VPeE?= =?us-ascii?Q?Nr8QNOFg/Z0SZ6KTgYjRQqer9U14OfDd5oNgukzqgirU+yRXTt43JQ7ASkuk?= =?us-ascii?Q?Q6nZf4Cnxnrf+VARlZwgXwxZO74ByfdE94RXVza15G19f7LxpqsBmuoZqJsN?= =?us-ascii?Q?H35oeDXN31Hfoy/3T4VGDUnk6ELAE3WdUu7vSi4pV3edE5Jvg0H9YAWdvWs2?= =?us-ascii?Q?eU9e14H4DFVarnw3FGc9soZqd4AE7JHCq8llINoAHJ32LwNmfVaUkAA9wgLh?= =?us-ascii?Q?YsJqVGLiVSilcqck67Jy6FBLy0z37hGy1dh4SIswY7/OEovgshHfEOHiGv1k?= =?us-ascii?Q?wUFOrSdisx5jBiCXvSUpaS+Zv5hivPCtDQ7DIDB/fkQb+ka2R+C6NuWqRsuG?= =?us-ascii?Q?hwVj0nD7o/+rPD9Ux3kxAUer4o4sX15NjN2eDCGkzolj3w47SWqFWqRoysVY?= =?us-ascii?Q?Xw=3D=3D?= MIME-Version: 1.0 X-MS-Exchange-CrossTenant-AuthAs: Internal X-MS-Exchange-CrossTenant-AuthSource: CO1PR11MB4929.namprd11.prod.outlook.com X-MS-Exchange-CrossTenant-Network-Message-Id: 8086d891-9a37-45d1-c58d-08dbaa486716 X-MS-Exchange-CrossTenant-originalarrivaltime: 31 Aug 2023 17:33:34.6810 (UTC) X-MS-Exchange-CrossTenant-fromentityheader: Hosted X-MS-Exchange-CrossTenant-id: 46c98d88-e344-4ed4-8496-4ed7712e255d X-MS-Exchange-CrossTenant-mailboxtype: HOSTED X-MS-Exchange-CrossTenant-userprincipalname: 4/ciJqJ7csTLMglXKaf3MjS4pmgvy2+sBv0iAFzd3nCt9oSo10p32c9k4vKrY5i/ER+NfT+8D7+AWyP3Fj1SlVsknZaER9vXlILs96Uqibg= X-MS-Exchange-Transport-CrossTenantHeadersStamped: CH3PR11MB7762 X-OriginatorOrg: intel.com Precedence: Bulk 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,michael.d.kinney@intel.com List-Unsubscribe-Post: List-Unsubscribe=One-Click List-Unsubscribe: X-Gm-Message-State: I4ZKgtE9wucEtR1xyDplNzpNx7686176AA= Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable X-GND-Status: LEGIT Authentication-Results: spool.mail.gandi.net; dkim=pass header.d=groups.io header.s=20140610 header.b=ezzyBo0u; dmarc=fail reason="SPF not aligned (relaxed), DKIM not aligned (relaxed)" header.from=intel.com (policy=none); spf=pass (spool.mail.gandi.net: domain of bounce@groups.io designates 66.175.222.108 as permitted sender) smtp.mailfrom=bounce@groups.io; arc=reject ("signature check failed: fail, {[1] = sig:microsoft.com:reject}") Hi Nate, I do not disagree with the logic of the patch. Reviewed-by: Michael D Kinney However, I do not understand how the 1st call to InternalCoreLocateHandle() can succeed and the 2nd call can fail if the handle database protocol lock is in the acquired state. Is there a real scenario where this can happen? How would the state of the handle database change=20 between the 2 calls? If the answer is that this scenario can not happen,then the impact to any code calling this API for this change is zero. Mike > -----Original Message----- > From: Desimone, Nathaniel L > Sent: Wednesday, August 30, 2023 9:47 AM > To: devel@edk2.groups.io > Cc: Gao, Liming ; Wang, Jian J > ; Kinney, Michael D ; > Bi, Dandan > Subject: [PATCH v1] MdeModulePkg: Fix memory leak in LocateHandleBuffer() >=20 > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3D4543 > REF: https://uefi.org/specs/UEFI/2.10/07_Services_Boot_Services.html#efi- > boot-services-locatehandlebuffer >=20 > CoreLocateHandleBuffer() can in certain cases, can return > an error and not free an allocated buffer. This scenario > occurs if the first call to InternalCoreLocateHandle() > returns success and the second call returns an error. >=20 > On a successful return, LocateHandleBuffer() passes > ownership of the buffer to the caller. However, the UEFI > specification is not explicit about what the expected > ownership of this buffer is in the case of an error. > However, it is heavily implied by the code example given > in section 7.3.15 of v2.10 of the UEFI specificaton that > if LocateHandleBuffer() returns a non-successful status > code then the ownership of the buffer does NOT transfer > to the caller. This code example explicitly refrains from > calling FreePool() if LocateHandleBuffer() returns an > error. >=20 > From a practical standpoint, it is logical to assume that > a non-successful status code indicates that no buffer of > handles was ever allocated. Indeed, in most error cases, > LocateHandleBuffer() does not go far enough to get to the > point where a buffer is allocated. Therefore, all existing > users of this API must already be coded to support the case > of a non-successful status code resulting in an invalid > handle buffer being returned. Therefore, this change will > not cause any backwards compatibility issues with existing > code. >=20 > In conclusion, this boils down to a fix for a memory leak > that also brings the behavior of our LocateHandleBuffer() > implementation into alignment with the original intentions > of the UEFI specification authors. >=20 > Cc: Liming Gao > Cc: Jian J Wang > Cc: Michael D Kinney > Cc: Dandan Bi > Signed-off-by: Nate DeSimone > --- > MdeModulePkg/Core/Dxe/Hand/Locate.c | 6 +++++- > 1 file changed, 5 insertions(+), 1 deletion(-) >=20 > diff --git a/MdeModulePkg/Core/Dxe/Hand/Locate.c > b/MdeModulePkg/Core/Dxe/Hand/Locate.c > index a29010a545..8f20c6332d 100644 > --- a/MdeModulePkg/Core/Dxe/Hand/Locate.c > +++ b/MdeModulePkg/Core/Dxe/Hand/Locate.c > @@ -1,7 +1,7 @@ > /** @file > Locate handle functions >=20 > -Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved.
> +Copyright (c) 2006 - 2023, Intel Corporation. All rights reserved.
> SPDX-License-Identifier: BSD-2-Clause-Patent >=20 > **/ > @@ -730,6 +730,10 @@ CoreLocateHandleBuffer ( > *NumberHandles =3D BufferSize / sizeof (EFI_HANDLE); > if (EFI_ERROR (Status)) { > *NumberHandles =3D 0; > + if (*Buffer !=3D NULL) { > + CoreFreePool (*Buffer); > + *Buffer =3D NULL; > + } > } >=20 > CoreReleaseProtocolLock (); > -- > 2.34.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 (#108203): https://edk2.groups.io/g/devel/message/108203 Mute This Topic: https://groups.io/mt/101056724/7686176 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/leave/12367111/7686176/19134562= 12/xyzzy [rebecca@openfw.io] -=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-