Skip to content

Commit 13fad60

Browse files
kenlautnermergify[bot]
kenlautner
authored and
mergify[bot]
committed
UefiCpuPkg: Fix unchecked returns and potential integer overflows
Resolves several issues in UefiCpuPkg related to: 1. Unchecked returns leading to potential NULL or uninitialized access. 2. Potential unchecked integer overflows. 3. Incorrect comparison between integers of different sizes. Co-authored-by: kenlautner <85201046+kenlautner@users.noreply.github.com> Signed-off-by: Chris Fernald <chfernal@microsoft.com>
1 parent 843f0c1 commit 13fad60

File tree

19 files changed

+423
-73
lines changed

19 files changed

+423
-73
lines changed

‎UefiCpuPkg/CpuDxe/CpuMp.c

Lines changed: 30 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -622,8 +622,15 @@ InitializeExceptionStackSwitchHandlers (
622622
{
623623
EXCEPTION_STACK_SWITCH_CONTEXT *SwitchStackData;
624624
UINTN Index;
625+
EFI_STATUS Status;
626+
627+
Status = MpInitLibWhoAmI (&Index);
628+
629+
if (EFI_ERROR (Status)) {
630+
DEBUG ((DEBUG_ERROR, "[%a] - Failed to get processor number. The exception stack was not initialized.\n", __func__));
631+
return;
632+
}
625633

626-
MpInitLibWhoAmI (&Index);
627634
SwitchStackData = (EXCEPTION_STACK_SWITCH_CONTEXT *)Buffer;
628635

629636
//
@@ -758,25 +765,28 @@ InitializeMpSupport (
758765
Status = MpInitLibInitialize ();
759766
ASSERT_EFI_ERROR (Status);
760767

761-
MpInitLibGetNumberOfProcessors (&NumberOfProcessors, &NumberOfEnabledProcessors);
762-
mNumberOfProcessors = NumberOfProcessors;
763-
DEBUG ((DEBUG_INFO, "Detect CPU count: %d\n", mNumberOfProcessors));
768+
Status = MpInitLibGetNumberOfProcessors (&NumberOfProcessors, &NumberOfEnabledProcessors);
769+
ASSERT_EFI_ERROR (Status);
770+
if (!EFI_ERROR (Status)) {
771+
mNumberOfProcessors = NumberOfProcessors;
772+
DEBUG ((DEBUG_INFO, "Detect CPU count: %d\n", mNumberOfProcessors));
764773

765-
//
766-
// Initialize special exception handlers for each logic processor.
767-
//
768-
InitializeMpExceptionHandlers ();
774+
//
775+
// Initialize special exception handlers for each logic processor.
776+
//
777+
InitializeMpExceptionHandlers ();
769778

770-
//
771-
// Update CPU healthy information from Guided HOB
772-
//
773-
CollectBistDataFromHob ();
774-
775-
Status = gBS->InstallMultipleProtocolInterfaces (
776-
&mMpServiceHandle,
777-
&gEfiMpServiceProtocolGuid,
778-
&mMpServicesTemplate,
779-
NULL
780-
);
781-
ASSERT_EFI_ERROR (Status);
779+
//
780+
// Update CPU healthy information from Guided HOB
781+
//
782+
CollectBistDataFromHob ();
783+
784+
Status = gBS->InstallMultipleProtocolInterfaces (
785+
&mMpServiceHandle,
786+
&gEfiMpServiceProtocolGuid,
787+
&mMpServicesTemplate,
788+
NULL
789+
);
790+
ASSERT_EFI_ERROR (Status);
791+
}
782792
}

‎UefiCpuPkg/CpuMpPei/CpuMpPei.c

Lines changed: 19 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -437,8 +437,14 @@ InitializeExceptionStackSwitchHandlers (
437437
{
438438
EXCEPTION_STACK_SWITCH_CONTEXT *SwitchStackData;
439439
UINTN Index;
440+
EFI_STATUS Status;
441+
442+
Status = MpInitLibWhoAmI (&Index);
443+
if (EFI_ERROR (Status)) {
444+
DEBUG ((DEBUG_ERROR, "%a - Failed to allocate Switch Stack pages.\n", __func__));
445+
return;
446+
}
440447

441-
MpInitLibWhoAmI (&Index);
442448
SwitchStackData = (EXCEPTION_STACK_SWITCH_CONTEXT *)Buffer;
443449

444450
//
@@ -481,7 +487,12 @@ InitializeMpExceptionStackSwitchHandlers (
481487
}
482488

483489
SwitchStackData = AllocatePages (EFI_SIZE_TO_PAGES (NumberOfProcessors * sizeof (EXCEPTION_STACK_SWITCH_CONTEXT)));
484-
ASSERT (SwitchStackData != NULL);
490+
if (SwitchStackData == NULL) {
491+
ASSERT (SwitchStackData != NULL);
492+
DEBUG ((DEBUG_ERROR, "%a - Failed to allocate Switch Stack pages.\n", __func__));
493+
return;
494+
}
495+
485496
ZeroMem (SwitchStackData, NumberOfProcessors * sizeof (EXCEPTION_STACK_SWITCH_CONTEXT));
486497
for (Index = 0; Index < NumberOfProcessors; ++Index) {
487498
//
@@ -511,7 +522,12 @@ InitializeMpExceptionStackSwitchHandlers (
511522

512523
if (BufferSize != 0) {
513524
Buffer = AllocatePages (EFI_SIZE_TO_PAGES (BufferSize));
514-
ASSERT (Buffer != NULL);
525+
if (Buffer == NULL) {
526+
ASSERT (Buffer != NULL);
527+
DEBUG ((DEBUG_ERROR, "%a - Failed to allocate Buffer pages.\n", __func__));
528+
return;
529+
}
530+
515531
BufferSize = 0;
516532
for (Index = 0; Index < NumberOfProcessors; ++Index) {
517533
if (SwitchStackData[Index].Status == EFI_BUFFER_TOO_SMALL) {

‎UefiCpuPkg/Library/CpuCommonFeaturesLib/MachineCheck.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -174,7 +174,7 @@ McaInitialize (
174174
}
175175

176176
if (PcdGetBool (PcdIsPowerOnReset)) {
177-
for (BankIndex = 0; BankIndex < (UINTN)McgCap.Bits.Count; BankIndex++) {
177+
for (BankIndex = 0; BankIndex < (UINT32)McgCap.Bits.Count; BankIndex++) {
178178
CPU_REGISTER_TABLE_WRITE64 (
179179
ProcessorNumber,
180180
Msr,

‎UefiCpuPkg/Library/CpuExceptionHandlerLib/PeiCpuException.c

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -71,7 +71,11 @@ SetExceptionHandlerData (
7171
IdtTable = (IA32_IDT_GATE_DESCRIPTOR *)IdtDescriptor.Base;
7272

7373
Exception0StubHeader = AllocatePool (sizeof (*Exception0StubHeader));
74-
ASSERT (Exception0StubHeader != NULL);
74+
if (Exception0StubHeader == NULL) {
75+
ASSERT (Exception0StubHeader != NULL);
76+
return;
77+
}
78+
7579
CopyMem (
7680
Exception0StubHeader->ExceptionStubHeader,
7781
(VOID *)ArchGetIdtHandler (&IdtTable[0]),
@@ -165,10 +169,18 @@ InitializeCpuExceptionHandlers (
165169
RESERVED_VECTORS_DATA *ReservedVectors;
166170

167171
ReservedVectors = AllocatePool (sizeof (RESERVED_VECTORS_DATA) * CPU_EXCEPTION_NUM);
168-
ASSERT (ReservedVectors != NULL);
172+
if (ReservedVectors == NULL) {
173+
ASSERT (ReservedVectors != NULL);
174+
return EFI_OUT_OF_RESOURCES;
175+
}
169176

170177
ExceptionHandlerData = AllocatePool (sizeof (EXCEPTION_HANDLER_DATA));
171-
ASSERT (ExceptionHandlerData != NULL);
178+
if (ExceptionHandlerData == NULL) {
179+
ASSERT (ExceptionHandlerData != NULL);
180+
FreePool (ReservedVectors);
181+
return EFI_OUT_OF_RESOURCES;
182+
}
183+
172184
ExceptionHandlerData->IdtEntryCount = CPU_EXCEPTION_NUM;
173185
ExceptionHandlerData->ReservedVectors = ReservedVectors;
174186
ExceptionHandlerData->ExternalInterruptHandler = AllocateZeroPool (sizeof (EFI_CPU_INTERRUPT_HANDLER) * ExceptionHandlerData->IdtEntryCount);

‎UefiCpuPkg/Library/MpInitLib/AmdSev.c

Lines changed: 32 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,9 @@ GetProtectedMode16CS (
2525
IA32_DESCRIPTOR GdtrDesc;
2626
IA32_SEGMENT_DESCRIPTOR *GdtEntry;
2727
UINTN GdtEntryCount;
28-
UINT16 Index;
28+
UINTN Index;
29+
UINT16 CodeSegmentValue;
30+
EFI_STATUS Status;
2931

3032
Index = (UINT16)-1;
3133
AsmReadGdtr (&GdtrDesc);
@@ -42,8 +44,19 @@ GetProtectedMode16CS (
4244
GdtEntry++;
4345
}
4446

45-
ASSERT (Index != GdtEntryCount);
46-
return Index * 8;
47+
Status = SafeUintnToUint16 (Index, &CodeSegmentValue);
48+
if (EFI_ERROR (Status)) {
49+
ASSERT_EFI_ERROR (Status);
50+
return 0;
51+
}
52+
53+
Status = SafeUint16Mult (CodeSegmentValue, 8, &CodeSegmentValue);
54+
if (EFI_ERROR (Status)) {
55+
ASSERT_EFI_ERROR (Status);
56+
return 0;
57+
}
58+
59+
return CodeSegmentValue;
4760
}
4861

4962
/**
@@ -61,7 +74,9 @@ GetProtectedMode32CS (
6174
IA32_DESCRIPTOR GdtrDesc;
6275
IA32_SEGMENT_DESCRIPTOR *GdtEntry;
6376
UINTN GdtEntryCount;
64-
UINT16 Index;
77+
UINTN Index;
78+
UINT16 CodeSegmentValue;
79+
EFI_STATUS Status;
6580

6681
Index = (UINT16)-1;
6782
AsmReadGdtr (&GdtrDesc);
@@ -79,7 +94,19 @@ GetProtectedMode32CS (
7994
}
8095

8196
ASSERT (Index != GdtEntryCount);
82-
return Index * 8;
97+
Status = SafeUintnToUint16 (Index, &CodeSegmentValue);
98+
if (EFI_ERROR (Status)) {
99+
ASSERT_EFI_ERROR (Status);
100+
return 0;
101+
}
102+
103+
Status = SafeUint16Mult (CodeSegmentValue, 8, &CodeSegmentValue);
104+
if (EFI_ERROR (Status)) {
105+
ASSERT_EFI_ERROR (Status);
106+
return 0;
107+
}
108+
109+
return CodeSegmentValue;
83110
}
84111

85112
/**

‎UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,7 @@
6363

6464
[LibraryClasses.IA32, LibraryClasses.X64]
6565
AmdSvsmLib
66+
SafeIntLib
6667
CcExitLib
6768
LocalApicLib
6869
MicrocodeLib

‎UefiCpuPkg/Library/MpInitLib/DxeMpLib.c

Lines changed: 32 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -313,7 +313,9 @@ GetProtectedMode16CS (
313313
IA32_DESCRIPTOR GdtrDesc;
314314
IA32_SEGMENT_DESCRIPTOR *GdtEntry;
315315
UINTN GdtEntryCount;
316-
UINT16 Index;
316+
UINTN Index;
317+
UINT16 CodeSegmentValue;
318+
EFI_STATUS Status;
317319

318320
Index = (UINT16)-1;
319321
AsmReadGdtr (&GdtrDesc);
@@ -329,8 +331,19 @@ GetProtectedMode16CS (
329331
GdtEntry++;
330332
}
331333

332-
ASSERT (Index != GdtEntryCount);
333-
return Index * 8;
334+
Status = SafeUintnToUint16 (Index, &CodeSegmentValue);
335+
if (EFI_ERROR (Status)) {
336+
ASSERT_EFI_ERROR (Status);
337+
return 0;
338+
}
339+
340+
Status = SafeUint16Mult (CodeSegmentValue, 8, &CodeSegmentValue);
341+
if (EFI_ERROR (Status)) {
342+
ASSERT_EFI_ERROR (Status);
343+
return 0;
344+
}
345+
346+
return CodeSegmentValue;
334347
}
335348

336349
/**
@@ -346,7 +359,9 @@ GetProtectedModeCS (
346359
IA32_DESCRIPTOR GdtrDesc;
347360
IA32_SEGMENT_DESCRIPTOR *GdtEntry;
348361
UINTN GdtEntryCount;
349-
UINT16 Index;
362+
UINTN Index;
363+
UINT16 CodeSegmentValue;
364+
EFI_STATUS Status;
350365

351366
AsmReadGdtr (&GdtrDesc);
352367
GdtEntryCount = (GdtrDesc.Limit + 1) / sizeof (IA32_SEGMENT_DESCRIPTOR);
@@ -361,8 +376,19 @@ GetProtectedModeCS (
361376
GdtEntry++;
362377
}
363378

364-
ASSERT (Index != GdtEntryCount);
365-
return Index * 8;
379+
Status = SafeUintnToUint16 (Index, &CodeSegmentValue);
380+
if (EFI_ERROR (Status)) {
381+
ASSERT_EFI_ERROR (Status);
382+
return 0;
383+
}
384+
385+
Status = SafeUint16Mult (CodeSegmentValue, 8, &CodeSegmentValue);
386+
if (EFI_ERROR (Status)) {
387+
ASSERT_EFI_ERROR (Status);
388+
return 0;
389+
}
390+
391+
return CodeSegmentValue;
366392
}
367393

368394
/**

0 commit comments

Comments
 (0)