* [edk2-devel] [PATCH v1 1/1] Bug 2861 - HiiDatabaseDxe, ConfigRouting.c, GetElementsFromRequest incorrect error handling. [not found] <62690423D2A24D1DBB82CD22AE44EADE.ref@DESKTOPQUG2G9K> @ 2023-10-26 1:05 ` Charles Hyde 2023-11-13 14:59 ` Laszlo Ersek 0 siblings, 1 reply; 4+ messages in thread From: Charles Hyde @ 2023-10-26 1:05 UTC (permalink / raw) To: devel From: Charles Hyde <chip.programmer@att.net> BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=2861 I believe the attached ConfigRouting.txt 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 attached patch should address this bug and more. Signed-off-by: Charles Hyde <chip.programmer@att.net> --- 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..0b39f156f3 100644 --- a/MdeModulePkg/Universal/HiiDatabaseDxe/ConfigRouting.c +++ b/MdeModulePkg/Universal/HiiDatabaseDxe/ConfigRouting.c @@ -420,14 +420,19 @@ AppendToMultiString ( } AppendStringSize = StrSize (AppendString); + if (AppendStringSize <= sizeof(*AppendString)) // If the string is empty, there is no need to proceed further. + return EFI_SUCCESS; + MultiStringSize = StrSize (*MultiString); MaxLen = MAX_STRING_LENGTH / sizeof (CHAR16); // // 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) /*|| + (MultiStringSize > MAX_STRING_LENGTH)*/) // There is no need to check the second part. + // If the first part is false, the second part will always be false. + // If the second part is true, the first part must also be true. { *MultiString = (EFI_STRING)ReallocatePool ( MultiStringSize, @@ -1800,8 +1805,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 +5303,7 @@ HiiConfigRoutingExportConfig ( // IfrDataParsedFlag = FALSE; Progress = NULL; + AccessResults = NULL; HiiHandle = NULL; DefaultResults = NULL; Database = NULL; @@ -5326,6 +5338,14 @@ HiiConfigRoutingExportConfig ( &AccessResults ); if (EFI_ERROR (Status)) { + + // If an error was returned, then do not believe any results in these two pointers. + Progress = NULL; + if (AccessResults) { + FreePool (AccessResults); + AccessResults = NULL; + } + // // Update AccessResults by getting default setting from IFR when HiiPackage is registered to HiiHandle // @@ -5350,6 +5370,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 // -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#110066): https://edk2.groups.io/g/devel/message/110066 Mute This Topic: https://groups.io/mt/102191640/7686176 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io] -=-=-=-=-=-=-=-=-=-=-=- ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [edk2-devel] [PATCH v1 1/1] Bug 2861 - HiiDatabaseDxe, ConfigRouting.c, GetElementsFromRequest incorrect error handling. 2023-10-26 1:05 ` [edk2-devel] [PATCH v1 1/1] Bug 2861 - HiiDatabaseDxe, ConfigRouting.c, GetElementsFromRequest incorrect error handling Charles Hyde @ 2023-11-13 14:59 ` Laszlo Ersek 2023-11-18 13:06 ` Charles Hyde 0 siblings, 1 reply; 4+ messages in thread From: Laszlo Ersek @ 2023-11-13 14:59 UTC (permalink / raw) To: devel, chip.programmer Hi Charles, On 10/26/23 03:05, Charles Hyde wrote: > From: Charles Hyde <chip.programmer@att.net> > > BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=2861 > > I believe the attached ConfigRouting.txt 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 attached patch should address this bug and > more. > > Signed-off-by: Charles Hyde <chip.programmer@att.net> > --- Thanks for analyzing and fixing these bugs. Can you please split the separate fixes to separate patches? Also, the patch looks garbled; it shouldn't be attached / pasted but sent with git-send-email. Are you familiar with git-send-email? Here's the official docs: https://github.com/tianocore/tianocore.github.io/wiki/EDK-II-Development-Process and some unofficial tips: https://github.com/tianocore/tianocore.github.io/wiki/Laszlo's-unkempt-git-guide-for-edk2-contributors-and-maintainers Third, I suggest not to comment out, with /* */, dead code (such as a subcondition that always evaluates to false or true); instead, remove it, and explain in the commit message (or, if necessary, in a code comment) why that condition is a tautology. If the condition or argument is nontrivial, consider using an ASSERT(). Laszlo > > 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..0b39f156f3 100644 > --- a/MdeModulePkg/Universal/HiiDatabaseDxe/ConfigRouting.c > +++ b/MdeModulePkg/Universal/HiiDatabaseDxe/ConfigRouting.c > @@ -420,14 +420,19 @@ AppendToMultiString ( > } > > AppendStringSize = StrSize (AppendString); > + if (AppendStringSize <= sizeof(*AppendString)) // If the string is > empty, there is no need to proceed further. > + return EFI_SUCCESS; > + > MultiStringSize = StrSize (*MultiString); > MaxLen = MAX_STRING_LENGTH / sizeof (CHAR16); > > // > // 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) /*|| > + (MultiStringSize > MAX_STRING_LENGTH)*/) // There is no need to > check the second part. > + // If the first part is > false, the second part will always be false. > + // If the second part is > true, the first part must also be true. > { > *MultiString = (EFI_STRING)ReallocatePool ( > MultiStringSize, > @@ -1800,8 +1805,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 +5303,7 @@ HiiConfigRoutingExportConfig ( > // > IfrDataParsedFlag = FALSE; > Progress = NULL; > + AccessResults = NULL; > HiiHandle = NULL; > DefaultResults = NULL; > Database = NULL; > @@ -5326,6 +5338,14 @@ HiiConfigRoutingExportConfig ( > &AccessResults > ); > if (EFI_ERROR (Status)) { > + > + // If an error was returned, then do not believe any results in > these > two pointers. > + Progress = NULL; > + if (AccessResults) { > + FreePool (AccessResults); > + AccessResults = NULL; > + } > + > // > // Update AccessResults by getting default setting from IFR when > HiiPackage is registered to HiiHandle > // > @@ -5350,6 +5370,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 > // > > > > > -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#111158): https://edk2.groups.io/g/devel/message/111158 Mute This Topic: https://groups.io/mt/102191640/7686176 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/leave/12367111/7686176/1913456212/xyzzy [rebecca@openfw.io] -=-=-=-=-=-=-=-=-=-=-=- ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [edk2-devel] [PATCH v1 1/1] Bug 2861 - HiiDatabaseDxe, ConfigRouting.c, GetElementsFromRequest incorrect error handling. 2023-11-13 14:59 ` Laszlo Ersek @ 2023-11-18 13:06 ` Charles Hyde 2023-11-21 14:55 ` Laszlo Ersek 0 siblings, 1 reply; 4+ messages in thread From: Charles Hyde @ 2023-11-18 13:06 UTC (permalink / raw) To: lersek, devel How many different patches are you looking for? One patch to fix bug 2861 specifically, and a separate patch that fixes the uninitialized pointer issue? I provided the second previously (June and July 2023), and literally nobody commented in edk2-rfc or edk2-devel. Chip -----Original Message----- From: Laszlo Ersek Sent: Monday, November 13, 2023 9:59 AM To: devel@edk2.groups.io ; chip.programmer@att.net Subject: Re: [edk2-devel] [PATCH v1 1/1] Bug 2861 - HiiDatabaseDxe, ConfigRouting.c, GetElementsFromRequest incorrect error handling. Hi Charles, On 10/26/23 03:05, Charles Hyde wrote: > From: Charles Hyde <chip.programmer@att.net> > > BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=2861 > > I believe the attached ConfigRouting.txt 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 attached patch should address this bug > and > more. > > Signed-off-by: Charles Hyde <chip.programmer@att.net> > --- Thanks for analyzing and fixing these bugs. Can you please split the separate fixes to separate patches? Also, the patch looks garbled; it shouldn't be attached / pasted but sent with git-send-email. Are you familiar with git-send-email? Here's the official docs: https://github.com/tianocore/tianocore.github.io/wiki/EDK-II-Development-Process and some unofficial tips: https://github.com/tianocore/tianocore.github.io/wiki/Laszlo's-unkempt-git-guide-for-edk2-contributors-and-maintainers Third, I suggest not to comment out, with /* */, dead code (such as a subcondition that always evaluates to false or true); instead, remove it, and explain in the commit message (or, if necessary, in a code comment) why that condition is a tautology. If the condition or argument is nontrivial, consider using an ASSERT(). Laszlo > > 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..0b39f156f3 100644 > --- a/MdeModulePkg/Universal/HiiDatabaseDxe/ConfigRouting.c > +++ b/MdeModulePkg/Universal/HiiDatabaseDxe/ConfigRouting.c > @@ -420,14 +420,19 @@ AppendToMultiString ( > } > > AppendStringSize = StrSize (AppendString); > + if (AppendStringSize <= sizeof(*AppendString)) // If the string is > empty, there is no need to proceed further. > + return EFI_SUCCESS; > + > MultiStringSize = StrSize (*MultiString); > MaxLen = MAX_STRING_LENGTH / sizeof (CHAR16); > > // > // 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) /*|| > + (MultiStringSize > MAX_STRING_LENGTH)*/) // There is no need to > check the second part. > + // If the first part is > false, the second part will always be false. > + // If the second part is > true, the first part must also be true. > { > *MultiString = (EFI_STRING)ReallocatePool ( > MultiStringSize, > @@ -1800,8 +1805,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 +5303,7 @@ HiiConfigRoutingExportConfig ( > // > IfrDataParsedFlag = FALSE; > Progress = NULL; > + AccessResults = NULL; > HiiHandle = NULL; > DefaultResults = NULL; > Database = NULL; > @@ -5326,6 +5338,14 @@ HiiConfigRoutingExportConfig ( > &AccessResults > ); > if (EFI_ERROR (Status)) { > + > + // If an error was returned, then do not believe any results in > these > two pointers. > + Progress = NULL; > + if (AccessResults) { > + FreePool (AccessResults); > + AccessResults = NULL; > + } > + > // > // Update AccessResults by getting default setting from IFR when > HiiPackage is registered to HiiHandle > // > @@ -5350,6 +5370,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 > // > > > > > -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#111425): https://edk2.groups.io/g/devel/message/111425 Mute This Topic: https://groups.io/mt/102191640/7686176 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io] -=-=-=-=-=-=-=-=-=-=-=- ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [edk2-devel] [PATCH v1 1/1] Bug 2861 - HiiDatabaseDxe, ConfigRouting.c, GetElementsFromRequest incorrect error handling. 2023-11-18 13:06 ` Charles Hyde @ 2023-11-21 14:55 ` Laszlo Ersek 0 siblings, 0 replies; 4+ messages in thread From: Laszlo Ersek @ 2023-11-21 14:55 UTC (permalink / raw) To: Chip, devel On 11/18/23 14:06, Chip wrote: > How many different patches are you looking for? > > One patch to fix bug 2861 specifically, and a separate patch that fixes > the uninitialized pointer issue? I can't easily say, because I'm not familiar with HiiDatabaseDxe. So looking at the present patch, I can only say it seems to be doing "too much". I generally prefer patches that do the possible *minimum* semantically. Isolating the fix for the uninited pointer to one patch sounds good. I'm not sure if the rest of the code code changes (i.e., the fix(es) for bug 2861) belong to just *one* other patch though. If you think any one of those changes makes no sense without the other changes, or else if you think these changes are nearly identical all over, then keeping them in one patch may be good. Basically treat any patch (including commit message and code changes) like an "explain like I'm five" lesson to a reviewer. Advance in small steps, and explain liberally. IMO it's not possible to write a patch that is "too didactic", only a patch that's too terse. > I provided the second previously (June > and July 2023), and literally nobody commented in edk2-rfc or edk2-devel. That's too bad, my apologies. The project has been facing challenges with timely reviews. Laszlo > > Chip > > > -----Original Message----- From: Laszlo Ersek > Sent: Monday, November 13, 2023 9:59 AM > To: devel@edk2.groups.io ; chip.programmer@att.net > Subject: Re: [edk2-devel] [PATCH v1 1/1] Bug 2861 - HiiDatabaseDxe, > ConfigRouting.c, GetElementsFromRequest incorrect error handling. > > Hi Charles, > > On 10/26/23 03:05, Charles Hyde wrote: >> From: Charles Hyde <chip.programmer@att.net> >> >> BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=2861 >> >> I believe the attached ConfigRouting.txt 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 attached patch should address this >> bug and >> more. >> >> Signed-off-by: Charles Hyde <chip.programmer@att.net> >> --- > > Thanks for analyzing and fixing these bugs. > > Can you please split the separate fixes to separate patches? > > Also, the patch looks garbled; it shouldn't be attached / pasted but > sent with git-send-email. Are you familiar with git-send-email? > > Here's the official docs: > > https://github.com/tianocore/tianocore.github.io/wiki/EDK-II-Development-Process > > and some unofficial tips: > > https://github.com/tianocore/tianocore.github.io/wiki/Laszlo's-unkempt-git-guide-for-edk2-contributors-and-maintainers > > Third, I suggest not to comment out, with /* */, dead code (such as a > subcondition that always evaluates to false or true); instead, remove > it, and explain in the commit message (or, if necessary, in a code > comment) why that condition is a tautology. If the condition or argument > is nontrivial, consider using an ASSERT(). > > Laszlo > > >> >> 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..0b39f156f3 100644 >> --- a/MdeModulePkg/Universal/HiiDatabaseDxe/ConfigRouting.c >> +++ b/MdeModulePkg/Universal/HiiDatabaseDxe/ConfigRouting.c >> @@ -420,14 +420,19 @@ AppendToMultiString ( >> } >> >> AppendStringSize = StrSize (AppendString); >> + if (AppendStringSize <= sizeof(*AppendString)) // If the string is >> empty, there is no need to proceed further. >> + return EFI_SUCCESS; >> + >> MultiStringSize = StrSize (*MultiString); >> MaxLen = MAX_STRING_LENGTH / sizeof (CHAR16); >> >> // >> // 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) /*|| >> + (MultiStringSize > MAX_STRING_LENGTH)*/) // There is no need to >> check the second part. >> + // If the first part is >> false, the second part will always be false. >> + // If the second part is >> true, the first part must also be true. >> { >> *MultiString = (EFI_STRING)ReallocatePool ( >> MultiStringSize, >> @@ -1800,8 +1805,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 +5303,7 @@ HiiConfigRoutingExportConfig ( >> // >> IfrDataParsedFlag = FALSE; >> Progress = NULL; >> + AccessResults = NULL; >> HiiHandle = NULL; >> DefaultResults = NULL; >> Database = NULL; >> @@ -5326,6 +5338,14 @@ HiiConfigRoutingExportConfig ( >> &AccessResults >> ); >> if (EFI_ERROR (Status)) { >> + >> + // If an error was returned, then do not believe any results in >> these >> two pointers. >> + Progress = NULL; >> + if (AccessResults) { >> + FreePool (AccessResults); >> + AccessResults = NULL; >> + } >> + >> // >> // Update AccessResults by getting default setting from IFR when >> HiiPackage is registered to HiiHandle >> // >> @@ -5350,6 +5370,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 >> // >> >> >> >> >> > > > > > -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#111552): https://edk2.groups.io/g/devel/message/111552 Mute This Topic: https://groups.io/mt/102191640/7686176 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/leave/12367111/7686176/1913456212/xyzzy [rebecca@openfw.io] -=-=-=-=-=-=-=-=-=-=-=- ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2023-11-21 14:55 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <62690423D2A24D1DBB82CD22AE44EADE.ref@DESKTOPQUG2G9K>
2023-10-26 1:05 ` [edk2-devel] [PATCH v1 1/1] Bug 2861 - HiiDatabaseDxe, ConfigRouting.c, GetElementsFromRequest incorrect error handling Charles Hyde
2023-11-13 14:59 ` Laszlo Ersek
2023-11-18 13:06 ` Charles Hyde
2023-11-21 14:55 ` Laszlo Ersek
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox