From edc6681206c1a8791981a2f911d2fb8b3d2f5768 Mon Sep 17 00:00:00 2001 From: Laszlo Ersek Date: Tue, 13 Feb 2024 13:09:18 -0800 Subject: 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 Cc: Gerd Hoffmann Cc: Rahul Kumar Cc: Ray Ni Reported-by: Gerd Hoffmann Signed-off-by: Laszlo Ersek Reviewed-by: Michael D Kinney Reviewed-by: Leif Lindholm Reviewed-by: Rahul Kumar Reviewed-by: Gerd Hoffmann Tested-by: Gerd Hoffmann --- UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c | 7 +++++++ 1 file changed, 7 insertions(+) (limited to 'UefiCpuPkg/PiSmmCpuDxeSmm') diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c index 09382945dd..499f979d34 100644 --- a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c @@ -1146,6 +1146,13 @@ PiCpuSmmEntry ( // When the HOB doesn't exist, allocate new SMBASE itself. // DEBUG ((DEBUG_INFO, "PiCpuSmmEntry: gSmmBaseHobGuid not found!\n")); + + mCpuHotPlugData.SmBase = (UINTN *)AllocatePool (sizeof (UINTN) * mMaxNumberOfCpus); + if (mCpuHotPlugData.SmBase == NULL) { + ASSERT (mCpuHotPlugData.SmBase != NULL); + CpuDeadLoop (); + } + // // very old processors (i486 + pentium) need 32k not 4k alignment, exclude them. // -- cgit v1.2.3