Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
UefiCpuPkg/PiSmmCpuDxeSmm: distinguish GetSmBase() failure modes
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=4682 Commit 725acd0 ("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 725acd0, "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 725acd0 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 <dun.tan@intel.com> Cc: Gerd Hoffmann <kraxel@redhat.com> Cc: Rahul Kumar <rahul1.kumar@intel.com> Cc: Ray Ni <ray.ni@intel.com> Signed-off-by: Laszlo Ersek <lersek@redhat.com> Reviewed-by: Michael D Kinney <michael.d.kinney@intel.com> Reviewed-by: Leif Lindholm <quic_llindhol@quicinc.com> Reviewed-by: Rahul Kumar <rahul1.kumar@intel.com> Reviewed-by: Gerd Hoffmann <kraxel@redhat.com> Tested-by: Gerd Hoffmann <kraxel@redhat.com>
- Loading branch information
Same here about considering
PanicLib
instead ofCpuDeadLoop()
in the future.