diff options
author | Laszlo Ersek <lersek@redhat.com> | 2024-02-13 13:09:18 -0800 |
---|---|---|
committer | mergify[bot] <37929162+mergify[bot]@users.noreply.github.com> | 2024-02-14 17:26:43 +0000 |
commit | edc6681206c1a8791981a2f911d2fb8b3d2f5768 (patch) | |
tree | 4575c631e6663e7ada2410581b513ad79aa6e2d4 /UefiCpuPkg | |
parent | 72c441df36af347cd54f4ad3869a075b1f34d925 (diff) | |
download | edk2-edc6681206c1a8791981a2f911d2fb8b3d2f5768.zip edk2-edc6681206c1a8791981a2f911d2fb8b3d2f5768.tar.gz edk2-edc6681206c1a8791981a2f911d2fb8b3d2f5768.tar.bz2 |
UefiCpuPkg/PiSmmCpuDxeSmm: fix NULL deref when gSmmBaseHobGuid is missingedk2-stable202402
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 <dun.tan@intel.com>
Cc: Gerd Hoffmann <kraxel@redhat.com>
Cc: Rahul Kumar <rahul1.kumar@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Reported-by: Gerd Hoffmann <kraxel@redhat.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>
Diffstat (limited to 'UefiCpuPkg')
-rw-r--r-- | UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c | 7 |
1 files changed, 7 insertions, 0 deletions
diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c index 0938294..499f979 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.
//
|