Eric,
I was running a driver with asserts turned on for testing, and was
triggering the "ASSERT_EFI_ERROR (Status);" in FreePool(). I ended up
narrowing it down to this function, with ConfigResp being set to something
other than NULL even though the GetAltCfg function returned bad status.
Since it was triggering an assert I figured I'd submit the patch and see
if it was useful to include.
My first thought was that it was a bug in my workstation's firmware, but
after double-checking with the UEFI specification I'm not sure it is. The
way I read it, there's no real defined state for ConfigResp if the
function fails. It won't be allocated if there's a failure, but the
specification doesn't say it'll remain unchanged. I'm still pretty fresh
when it comes to UEFI, though, so I'm not sure if my interpretation is
right... I could definitely be missing something here.
Chris Barr
"Dong, Eric" <[email protected]>
05/04/2014 08:48 PM
Please respond to
[email protected]
To
"[email protected]" <[email protected]>
cc
"[email protected]" <[email protected]>
Subject
Re: [edk2] [PATCH] MdeModulePkg: Potential invalid free in HiiLib
Christopher,
I think the current code is ok, because before use ConfigResp, we has set
it to NULL, and the ConfigResp will be used to save the return value only
when the return status is success, otherwise this parameter should keep as
NULL.
What do you think? Do you met any issue already?
Thanks,
Eric
From: [email protected] [mailto:[email protected]]
Sent: Thursday, May 01, 2014 1:56 AM
To: [email protected]
Subject: [edk2] [PATCH] MdeModulePkg: Potential invalid free in HiiLib
Dear MdeModulePkg maintainer,
I would like to submit this patch. Do I just post it to this list?
Thank you for your time.
==============================================
MdeModulePkg: Fix potential invalid free in HiiLib
This relates to a previous call to gHiiConfigRouting->GetAltConfig.
The only guarantee the UEFI specification makes about the state of
ConfigResp is that it will be allocated if the function returns
EFI_SUCCESS. Since there is no guarantee that ConfigResp will remain
NULL if GetAltConfig fails, ConfigResp should be reset to NULL so
that the cleanup code for this function does not attempt to call
FreePool() on a potentially invalid pointer.
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Chris Barr <[email protected]>
---
MdeModulePkg/Library/UefiHiiLib/HiiLib.c | 1 +
1 files changed, 1 insertions(+), 0 deletions(-)
diff --git a/MdeModulePkg/Library/UefiHiiLib/HiiLib.c
b/MdeModulePkg/Library/UefiHiiLib/HiiLib.c
index 09f1ff7..cfadbdb 100644
--- a/MdeModulePkg/Library/UefiHiiLib/HiiLib.c
+++ b/MdeModulePkg/Library/UefiHiiLib/HiiLib.c
@@ -2156,6 +2156,7 @@ InternalHiiIfrValueAction (
// The required setting can't be found. So, it is not required to be
validated and set.
//
if (EFI_ERROR (Status)) {
+ ConfigResp = NULL;
Status = EFI_SUCCESS;
goto NextConfigAltResp;
}
------------------------------------------------------------------------------
Is your legacy SCM system holding you back? Join Perforce May 7 to find
out:
• 3 signs your SCM is hindering your productivity
• Requirements for releasing software faster
• Expert tips and advice for migrating your SCM now
http://p.sf.net/sfu/perforce
_______________________________________________
edk2-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/edk2-devel
------------------------------------------------------------------------------
Is your legacy SCM system holding you back? Join Perforce May 7 to find out:
• 3 signs your SCM is hindering your productivity
• Requirements for releasing software faster
• Expert tips and advice for migrating your SCM now
http://p.sf.net/sfu/perforce
_______________________________________________
edk2-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/edk2-devel
