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 7E9587803D7 for ; Wed, 30 Aug 2023 16:47:27 +0000 (UTC) DKIM-Signature: a=rsa-sha256; bh=a748bDCbXhvMDZ9ZqmSyz75lM4GEy/XcXLPNF70E1YA=; c=relaxed/simple; d=groups.io; h=From:To:Cc:Subject:Date:Message-Id:MIME-Version:Precedence:List-Subscribe:List-Help:Sender:List-Id:Mailing-List:Delivered-To:Reply-To:List-Unsubscribe-Post:List-Unsubscribe:Content-Transfer-Encoding; s=20140610; t=1693414046; v=1; b=pzx99y2gxjp4+2fkuE9QOFVBoOd6dcqa1GL6/H45QVfNBmoLtp21W2ppnU2CsXBssRZyprWy H1X6+i8hcGnPxswnUIu2qPXlvy/AL8wrSYorFlFugTfkuTi6FWF9t/Rdhh5nxL0upQ5PjYeLNqS UlmRTWmuC5N7wAFZ/lj9bK2g= X-Received: by 127.0.0.2 with SMTP id XAEhYY7687511xqrWyekq8o4; Wed, 30 Aug 2023 09:47:26 -0700 X-Received: from mgamail.intel.com (mgamail.intel.com [192.55.52.136]) by mx.groups.io with SMTP id smtpd.web11.271.1693414045082315682 for ; Wed, 30 Aug 2023 09:47:25 -0700 X-IronPort-AV: E=McAfee;i="6600,9927,10818"; a="355199361" X-IronPort-AV: E=Sophos;i="6.02,214,1688454000"; d="scan'208";a="355199361" X-Received: from orsmga003.jf.intel.com ([10.7.209.27]) by fmsmga106.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 30 Aug 2023 09:47:24 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=McAfee;i="6600,9927,10818"; a="688977910" X-IronPort-AV: E=Sophos;i="6.02,214,1688454000"; d="scan'208";a="688977910" X-Received: from nldesimo-desk.amr.corp.intel.com ([10.241.240.243]) by orsmga003-auth.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 30 Aug 2023 09:47:23 -0700 From: "Nate DeSimone" To: devel@edk2.groups.io Cc: Liming Gao , Jian J Wang , Michael D Kinney , Dandan Bi Subject: [edk2-devel] [PATCH v1] MdeModulePkg: Fix memory leak in LocateHandleBuffer() Date: Wed, 30 Aug 2023 09:46:41 -0700 Message-Id: <20230830164641.588-1-nathaniel.l.desimone@intel.com> MIME-Version: 1.0 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,nathaniel.l.desimone@intel.com List-Unsubscribe-Post: List-Unsubscribe=One-Click List-Unsubscribe: X-Gm-Message-State: vc4R5hLhZVPOJSIIhtumQOxpx7686176AA= Content-Transfer-Encoding: 8bit X-GND-Status: LEGIT Authentication-Results: spool.mail.gandi.net; dkim=pass header.d=groups.io header.s=20140610 header.b=pzx99y2g; 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 REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4543 REF: https://uefi.org/specs/UEFI/2.10/07_Services_Boot_Services.html#efi-boot-services-locatehandlebuffer 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. 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. >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. 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. 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(-) 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 -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 **/ @@ -730,6 +730,10 @@ CoreLocateHandleBuffer ( *NumberHandles = BufferSize / sizeof (EFI_HANDLE); if (EFI_ERROR (Status)) { *NumberHandles = 0; + if (*Buffer != NULL) { + CoreFreePool (*Buffer); + *Buffer = NULL; + } } CoreReleaseProtocolLock (); -- 2.34.1 -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#108149): https://edk2.groups.io/g/devel/message/108149 Mute This Topic: https://groups.io/mt/101056724/7686176 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io] -=-=-=-=-=-=-=-=-=-=-=-