From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail05.groups.io (mail05.groups.io [45.79.224.7]) by spool.mail.gandi.net (Postfix) with ESMTPS id A0F2C780091 for ; Sun, 11 Aug 2024 18:44:26 +0000 (UTC) DKIM-Signature: a=rsa-sha256; bh=6g5FjmUtzD/IZC3mkPJ1F4IdY05QhC7/gj0PKdkHOhc=; c=relaxed/simple; d=groups.io; h=Date:From:To:Message-ID:Subject:References:Precedence:List-Subscribe:List-Help:Sender:List-Id:Mailing-List:Delivered-To:Resent-Date:Resent-From:Reply-To:List-Unsubscribe-Post:List-Unsubscribe:Content-Transfer-Encoding:Content-Type:Content-Length; s=20240206; t=1723401866; v=1; b=mgIjOYuz/vfxGXmcvjh7ngR2FRDVux0tFTCcuFFVMMZ7kFH/SfKpp2ocD/dnxmr4zTH/YDpt 4/5lThJwfJO4zy4iuQBcx9LphhK2zOfJHg43rWacdXYnnwN42wwjBkGy0Gbxa9G46+EHHloHTkF 6nBs+ltdTcVPboidKXfdywXWptH5ZFp2tskXWqXJZ4KZMw0tlMxKnk8B7xOHMjYBUuvfPDnHmM1 2SdgEghwZo+H4UKljafvJ/TzgecSeDbcnuQ2EPOcwbJml8KCZ4iRaFlBp/uzndKu1bD1YDlP5dm yWbQTF+qHi7Lm4hiBFbWP8b+N49SuXZGneNjXBMuwgd5A== X-Received: by 127.0.0.2 with SMTP id ECqEYY7687511xBEL5a5naqM; Sun, 11 Aug 2024 11:44:24 -0700 X-Received: from sonic314-24.consmr.mail.ne1.yahoo.com (sonic314-24.consmr.mail.ne1.yahoo.com [66.163.189.150]) by mx.groups.io with SMTP id smtpd.web10.29855.1723401864016107143 for ; Sun, 11 Aug 2024 11:44:24 -0700 X-SONIC-DKIM-SIGN: v=1; a=rsa-sha256; c=relaxed/relaxed; d=yahoo.com; s=s2048; t=1723401863; bh=mpjBmOeja/o0BREUQGZMfNSDj297g2ptMkCcbjxSecd=; h=X-Sonic-MF:Date:From:To:Subject:From:Subject; b=U9zPj/bYw06VDEI4Jy3qqoOLAK5BFtJL8B4C0HZCxOIs1jkE25CWRRmlH6vXYL+/+HamSKCIdusZdNWNcPAOWcEMemL/LTTMVWtcW+9GvoJyh1oOR/pZpTneJO0nVJasOcZlT+JGMLxjb4RoYJaPFcZdHtnO4KIb6VuMt71T4K2dxxInX18cUBzAUq75Ic+2YMfcbrVBVW8qoJPg+ttZohTDJzRD1iK4vFxKOyKPz9Fi3BbsMtIPpIK4ANbODSgsAkrEZzhtsj12BKO2wbLnYG7oq0vX8RkA2Bpf/mMKcg1Osz7wrmGsgXd1HKuUVao5rA7tnN4PvRhGMA8Zw6OghA== X-Sonic-MF: X-Sonic-ID: dacc6eec-9be6-4463-acea-c3cdf8388b9e X-Received: from sonic.gate.mail.ne1.yahoo.com by sonic314.consmr.mail.ne1.yahoo.com with HTTP; Sun, 11 Aug 2024 18:44:23 +0000 X-Received: by hermes--production-bf1-774ddfff8-dcw6h (Yahoo Inc. Hermes SMTP Server) with ESMTPA ID 29abac6cd7a7cee5ab0e971bb028459d; Sun, 11 Aug 2024 18:44:19 +0000 (UTC) Date: Sun, 11 Aug 2024 14:44:17 -0400 From: "Charles Hyde" To: devel@edk2.groups.io Message-ID: <01daec1e$Blat.v3.2.25$7860fca8$20904da6@127.0.0.1> Subject: [edk2-devel] [PATCH v2 1/1] Bug 2861 - HiiDatabaseDxe, ConfigRouting.c, GetElementsFromRequest incorrect error handling. References: <01daec1e$Blat.v3.2.25$7860fca8$20904da6.ref@127.0.0.1> 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 Resent-Date: Sun, 11 Aug 2024 11:44:24 -0700 Resent-From: chip.programmer@att.net Reply-To: devel@edk2.groups.io,chip.programmer@att.net List-Unsubscribe-Post: List-Unsubscribe=One-Click List-Unsubscribe: X-Gm-Message-State: QTaNKsV4YMWiT7rKDht7QUU5x7686176AA= Content-Transfer-Encoding: 8BIT Content-Type: text/plain; charset="ISO-8859-1" X-GND-Status: LEGIT Authentication-Results: spool.mail.gandi.net; dkim=fail ("headers rsa verify failed") header.d=groups.io header.s=20240206 header.b=mgIjOYuz; dmarc=none; spf=pass (spool.mail.gandi.net: domain of bounce@groups.io designates 45.79.224.7 as permitted sender) smtp.mailfrom=bounce@groups.io From: Charles Hyde BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=2861 I believe the below patch will resolve bug 2861, plus resolve an uninitialized pointer issue in HiiConfigRoutingExportConfig(). The uninitialized pointer was identified when running the EDK2 Self Certification Test with all tests selected, having caused the CPU to issue an exception error (most times) or completely trashed the system (sometimes). I found a second instance of GetElementsFromRequest(), located in HiiLib.c, that also needed an update. The below patch should address this bug and more. This patch shown below is being sent with an alternate email program so that all lines should be exact, without line wrapping. The last line of this patch addresses a spelling error. I have noticed a lot of file updates recently where the only changes were for spelling, so I chose to include one here in the same file I have been looking at. I am not explaining these edits to a five year old, none of my grandkids understand anything of what I say with regards to software engineering. Signed-off-by: Charles Hyde --- diff --git a/MdeModulePkg/Library/UefiHiiLib/HiiLib.c b/MdeModulePkg/Library/UefiHiiLib/HiiLib.c index 63a37ab59a..c3dc7bf558 100644 --- a/MdeModulePkg/Library/UefiHiiLib/HiiLib.c +++ b/MdeModulePkg/Library/UefiHiiLib/HiiLib.c @@ -2272,8 +2272,14 @@ GetElementsFromRequest ( { EFI_STRING TmpRequest; + ASSERT (ConfigRequest != NULL); + if (ConfigRequest == NULL) + return FALSE; + TmpRequest = StrStr (ConfigRequest, L"PATH="); ASSERT (TmpRequest != NULL); + if (TmpRequest == NULL) + return FALSE; if ((StrStr (TmpRequest, L"&OFFSET=") != NULL) || (StrStr (TmpRequest, L"&") != NULL)) { return TRUE; diff --git a/MdeModulePkg/Universal/HiiDatabaseDxe/ConfigRouting.c b/MdeModulePkg/Universal/HiiDatabaseDxe/ConfigRouting.c index 5ae6189a28..e04f56bc56 100644 --- a/MdeModulePkg/Universal/HiiDatabaseDxe/ConfigRouting.c +++ b/MdeModulePkg/Universal/HiiDatabaseDxe/ConfigRouting.c @@ -419,6 +419,9 @@ AppendToMultiString ( return EFI_INVALID_PARAMETER; } + if (0 == *AppendString) // We are done here if string is empty. + return EFI_SUCCESS; + AppendStringSize = StrSize (AppendString); MultiStringSize = StrSize (*MultiString); MaxLen = MAX_STRING_LENGTH / sizeof (CHAR16); @@ -426,8 +429,9 @@ AppendToMultiString ( // // Enlarge the buffer each time when length exceeds MAX_STRING_LENGTH. // - if ((MultiStringSize + AppendStringSize > MAX_STRING_LENGTH) || - (MultiStringSize > MAX_STRING_LENGTH)) + if ((MultiStringSize + AppendStringSize) > MAX_STRING_LENGTH) + // AppendStringSize is unsigned, it cannot be less than zero. + // Remove unneeded check for MultiStringSize > MAX_STRING_LENGTH { *MultiString = (EFI_STRING)ReallocatePool ( MultiStringSize, @@ -1800,8 +1804,14 @@ GetElementsFromRequest ( { EFI_STRING TmpRequest; + ASSERT (ConfigRequest != NULL); + if (ConfigRequest == NULL) + return FALSE; + TmpRequest = StrStr (ConfigRequest, L"PATH="); ASSERT (TmpRequest != NULL); + if (TmpRequest == NULL) + return FALSE; if ((StrStr (TmpRequest, L"&OFFSET=") != NULL) || (StrStr (TmpRequest, L"&") != NULL)) { return TRUE; @@ -5292,6 +5302,7 @@ HiiConfigRoutingExportConfig ( // IfrDataParsedFlag = FALSE; Progress = NULL; + AccessResults = NULL; HiiHandle = NULL; DefaultResults = NULL; Database = NULL; @@ -5326,6 +5337,14 @@ HiiConfigRoutingExportConfig ( &AccessResults ); if (EFI_ERROR (Status)) { + + // If an error was returned, then these pointers are no good. + Progress = NULL; + if (AccessResults) { + FreePool (AccessResults); + AccessResults = NULL; + } + // // Update AccessResults by getting default setting from IFR when HiiPackage is registered to HiiHandle // @@ -5350,6 +5369,17 @@ HiiConfigRoutingExportConfig ( } if (!EFI_ERROR (Status)) { + + // If AccessResults == NULL, there is nothing to be done. + if (AccessResults == NULL) { + Progress = NULL; + + if (ConfigRequest != NULL) + FreePool (ConfigRequest); + + continue; + } + // // Update AccessResults by getting default setting from IFR when HiiPackage is registered to HiiHandle // @@ -5370,7 +5400,7 @@ HiiConfigRoutingExportConfig ( } // - // Merge the default sting from IFR code into the got setting from driver. + // Merge the default string from IFR code into the got setting from driver. // if (DefaultResults != NULL) { Status = MergeDefaultString (&AccessResults, DefaultResults); -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#120313): https://edk2.groups.io/g/devel/message/120313 Mute This Topic: https://groups.io/mt/107844238/7686176 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io] -=-=-=-=-=-=-=-=-=-=-=-