Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
UefiCpuPkg/PiSmmCpuDxeSmm: fix NULL deref when gSmmBaseHobGuid is mis…
…sing Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=4682 Fixes: 725acd0 Before commit 725acd0 ("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 725acd0, 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 725acd0 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]>
- Loading branch information
Shouldn't be done in this PR, but we might want to replace this with
PanicLib
in our code.