Christopher,
Base on the spec description, I admit it's not clear whether ConfigResp is NULL
if error status return. So I check the gHiiConfigRouting->GetAltConfig
function, it will set ConfigResp to NULL if error status return.
I can't find a code pass where the ConfigResp will be not NULL and the return
status is error. Can you help to do more investigation and get more detail
about this error?
Thanks,
Eric
From: [email protected] [mailto:[email protected]]
Sent: Monday, May 05, 2014 11:47 PM
To: Dong, Eric
Cc: [email protected]
Subject: Re: [edk2] [PATCH] MdeModulePkg: Potential invalid free in HiiLib
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]<mailto:[email protected]>>
05/04/2014 08:48 PM
Please respond to
[email protected]<mailto:[email protected]>
To
"[email protected]<mailto:[email protected]>"
<[email protected]<mailto:[email protected]>>
cc
"[email protected]<mailto:[email protected]>"
<[email protected]<mailto:[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]>
[mailto:[email protected]]
Sent: Thursday, May 01, 2014 1:56 AM
To: [email protected]<mailto:[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]<mailto:[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]<mailto:[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
