Branch: refs/heads/master
Home: https://github.com/tianocore/edk2
Commit: 72c441df36af347cd54f4ad3869a075b1f34d925
https://github.com/tianocore/edk2/commit/72c441df36af347cd54f4ad3869a075b1f34d925
Author: Laszlo Ersek <[email protected]>
Date: 2024-02-14 (Wed, 14 Feb 2024)
Changed paths:
M UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c
Log Message:
-----------
UefiCpuPkg/PiSmmCpuDxeSmm: distinguish GetSmBase() failure modes
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=4682
Commit 725acd0b9cc0 ("UefiCpuPkg: Avoid assuming only one smmbasehob",
2023-12-12) introduced a helper function called GetSmBase(), replacing
the lookup of the first and only "gSmmBaseHobGuid" GUID HOB and
unconditional "mCpuHotPlugData.SmBase" allocation, with iterated lookups
plus conditional memory allocation.
This introduced a new failure mode for setting "mCpuHotPlugData.SmBase".
Namely, before commit 725acd0b9cc0, "mCpuHotPlugData.SmBase" would be
allocated regardless of the GUID HOB being absent. After the commit,
"mCpuHotPlugData.SmBase" could remain NULL if the GUID HOB was absent,
*or* one of the memory allocations inside GetSmBase() failed; and in the
former case, we'd even proceed to the rest of PiCpuSmmEntry().
In relation to this conflation of distinct failure modes, commit
725acd0b9cc0 actually introduced a NULL pointer dereference. Namely, a
NULL "mCpuHotPlugData.SmBase" is not handled properly at all now. We're
going to fix that NULL pointer dereference in a subsequent patch; however,
as a pre-requisite for that we need to tell apart the failure modes of
GetSmBase().
For memory allocation failures, return EFI_OUT_OF_RESOURCES. Move the
"assertion" that SMRAM cannot be exhausted happen out to the caller
(PiCpuSmmEntry()). Strengthen the assertion by adding an explicit
CpuDeadLoop() call. (Note: GetSmBase() *already* calls CpuDeadLoop() if
(NumberOfProcessors != MaxNumberOfCpus).)
For the absence of the GUID HOB, return EFI_NOT_FOUND.
For good measure, make GetSmBase() STATIC (it should have been STATIC from
the start).
This is just a refactoring, no behavioral difference is intended (beyond
the explicit CpuDeadLoop() upon SMRAM exhaustion).
Cc: Dun Tan <[email protected]>
Cc: Gerd Hoffmann <[email protected]>
Cc: Rahul Kumar <[email protected]>
Cc: Ray Ni <[email protected]>
Signed-off-by: Laszlo Ersek <[email protected]>
Reviewed-by: Michael D Kinney <[email protected]>
Reviewed-by: Leif Lindholm <[email protected]>
Reviewed-by: Rahul Kumar <[email protected]>
Reviewed-by: Gerd Hoffmann <[email protected]>
Tested-by: Gerd Hoffmann <[email protected]>
Commit: edc6681206c1a8791981a2f911d2fb8b3d2f5768
https://github.com/tianocore/edk2/commit/edc6681206c1a8791981a2f911d2fb8b3d2f5768
Author: Laszlo Ersek <[email protected]>
Date: 2024-02-14 (Wed, 14 Feb 2024)
Changed paths:
M UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c
Log Message:
-----------
UefiCpuPkg/PiSmmCpuDxeSmm: fix NULL deref when gSmmBaseHobGuid is missing
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=4682
Fixes: 725acd0b9cc0
Before commit 725acd0b9cc0 ("UefiCpuPkg: Avoid assuming only one
smmbasehob", 2023-12-12), PiCpuSmmEntry() used to look up
"gSmmBaseHobGuid", and allocate "mCpuHotPlugData.SmBase" regardless of the
GUID's presence:
> - mCpuHotPlugData.SmBase = (UINTN *)AllocatePool (sizeof (UINTN) *
> mMaxNumberOfCpus);
> - ASSERT (mCpuHotPlugData.SmBase != NULL);
After commit 725acd0b9cc0, PiCpuSmmEntry() -> GetSmBase() would allocate
"mCpuHotPlugData.SmBase" only on the success path, and no allocation would
be performed on *any* of the error paths.
This caused a problem: if "mCpuHotPlugData.SmBase" was left NULL because
the GUID HOB was missing, PiCpuSmmEntry() would still be supposed to
allocate "mCpuHotPlugData.SmBase", just like earlier. However, because
commit 725acd0b9cc0 conflated the two possible error modes (out of SMRAM,
and no GUID HOB), PiCpuSmmEntry() could not decide whether it should
allocate "mCpuHotPlugData.SmBase", or not. Currently, we never allocate if
GetSmBase() fails -- for any reason --, which means that on platforms that
don't produce the GUID HOB, "mCpuHotPlugData.SmBase" is left NULL, leading
to null pointer dereferences later, in PiCpuSmmEntry().
Now that a prior patch in the series distinguishes the two error modes
from each other, we can tell exactly when the GUID HOB is not found, and
reinstate the earlier "mCpuHotPlugData.SmBase" allocation for that case.
(With an actual error check thrown in, in addition to the original
"assertion".)
Cc: Dun Tan <[email protected]>
Cc: Gerd Hoffmann <[email protected]>
Cc: Rahul Kumar <[email protected]>
Cc: Ray Ni <[email protected]>
Reported-by: Gerd Hoffmann <[email protected]>
Signed-off-by: Laszlo Ersek <[email protected]>
Reviewed-by: Michael D Kinney <[email protected]>
Reviewed-by: Leif Lindholm <[email protected]>
Reviewed-by: Rahul Kumar <[email protected]>
Reviewed-by: Gerd Hoffmann <[email protected]>
Tested-by: Gerd Hoffmann <[email protected]>
Compare: https://github.com/tianocore/edk2/compare/5fd3078a2e08...edc6681206c1
_______________________________________________
edk2-commits mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/edk2-commits
