From 8de8d5b8518e22fb8040304f2b7db47ab1fdcc3f Mon Sep 17 00:00:00 2001 From: Mike Ebersol Date: Fri, 1 May 2026 22:54:25 +0000 Subject: [PATCH 1/2] Merged PR 15215195: [Copilot] Harden VmbfsDxe against malicious host responses MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit [Copilot] Harden VmbfsDxe against malicious host responses Remove VMBFS_BAD_HOST macro (was ASSERT(FALSE), no-op in release) and replace all call sites with FAIL_FAST_UNEXPECTED_HOST_BEHAVIOR(). Remove dead code after each FAIL_FAST (status assignments, goto Cleanup, buffer clamping) since FAIL_FAST terminates the system. Hardened checks: version response size/type, file info response size/type, read payload response size/type, response byte count exceeding request, and oversized receive callback packets. ---- Security hardening: enforce fail-fast behavior and stricter validation to defend VmbfsDxe against malicious/unexpected host responses. Strengthens VmbfsDxe’s handling of incorrect/hostile VMBFS messages by replacing recoverable error paths with fail-fast crashes and tightening message buffer handling. - `MsvmPkg/VmbfsDxe/VmbfsEfi.h`: remove `VMBFS_BAD_HOST` macro and add `CrashLib` include to support fail-fast behavior. - `MsvmPkg/VmbfsDxe/Vmbfs.c`: fail fast on invalid version response messages; zero-initialize the packet buffer before use. - `MsvmPkg/VmbfsDxe/VmbfsFile.c`: replace multiple “bad host” error/cleanup paths with `FAIL_FAST_UNEXPECTED_HOST_BEHAVIOR()` for malformed responses, size mismatches, and oversize buffer lengths. - `MsvmPkg/VmbfsDxe/VmbfsDxe.inf`: add `CrashLib` dependency to wire in the new fail-fast mechanism. Related work items: #61642528 --- MsvmPkg/VmbfsDxe/Vmbfs.c | 7 +++---- MsvmPkg/VmbfsDxe/VmbfsDxe.inf | 1 + MsvmPkg/VmbfsDxe/VmbfsEfi.h | 3 +-- MsvmPkg/VmbfsDxe/VmbfsFile.c | 24 ++++++------------------ 4 files changed, 11 insertions(+), 24 deletions(-) diff --git a/MsvmPkg/VmbfsDxe/Vmbfs.c b/MsvmPkg/VmbfsDxe/Vmbfs.c index d6700a59a7..f190513faa 100644 --- a/MsvmPkg/VmbfsDxe/Vmbfs.c +++ b/MsvmPkg/VmbfsDxe/Vmbfs.c @@ -155,6 +155,8 @@ Return Value: goto Cleanup; } + ZeroMem(fileSystemInformation->PacketBuffer, VMBFS_MAXIMUM_MESSAGE_SIZE); + status = gBS->AllocatePool(EfiBootServicesData, sizeof(*allocatedFileProtocol), (void**)&allocatedFileProtocol); if (EFI_ERROR(status)) @@ -219,10 +221,7 @@ Return Value: if (bytesRead != sizeof(*VersionResponseMessage) || VersionResponseMessage->Header.Type != VmbfsMessageTypeVersionResponse) { - - VMBFS_BAD_HOST; - status = EFI_DEVICE_ERROR; - goto Cleanup; + FAIL_FAST_UNEXPECTED_HOST_BEHAVIOR(); } if (VersionResponseMessage->Status != VmbfsVersionSupported) diff --git a/MsvmPkg/VmbfsDxe/VmbfsDxe.inf b/MsvmPkg/VmbfsDxe/VmbfsDxe.inf index 7e4822a1e5..9ad1e0ff8f 100644 --- a/MsvmPkg/VmbfsDxe/VmbfsDxe.inf +++ b/MsvmPkg/VmbfsDxe/VmbfsDxe.inf @@ -36,6 +36,7 @@ [LibraryClasses] BaseLib BaseMemoryLib + CrashLib DebugLib EmclLib MemoryAllocationLib diff --git a/MsvmPkg/VmbfsDxe/VmbfsEfi.h b/MsvmPkg/VmbfsDxe/VmbfsEfi.h index be61831d79..ea89416a7f 100644 --- a/MsvmPkg/VmbfsDxe/VmbfsEfi.h +++ b/MsvmPkg/VmbfsDxe/VmbfsEfi.h @@ -23,13 +23,12 @@ #include #include #include +#include #include #include #include -#define VMBFS_BAD_HOST ASSERT(FALSE) - #define GetPacketBuffer(fileInformation, Type) ((Type*)((fileInformation)->FileSystem->FileSystemInformation.PacketBuffer)) #define GetPacketSize(fileInformation) (((fileInformation)->FileSystem->FileSystemInformation.PacketSize)) #define GetFileSystemInformation(fileInformation) (&((fileInformation)->FileSystem->FileSystemInformation)) diff --git a/MsvmPkg/VmbfsDxe/VmbfsFile.c b/MsvmPkg/VmbfsDxe/VmbfsFile.c index b22e7e1ad3..e916da5d69 100644 --- a/MsvmPkg/VmbfsDxe/VmbfsFile.c +++ b/MsvmPkg/VmbfsDxe/VmbfsFile.c @@ -85,8 +85,7 @@ Return Value: if (BufferLength > VMBFS_MAXIMUM_MESSAGE_SIZE) { - VMBFS_BAD_HOST; - BufferLength = MIN(BufferLength, VMBFS_MAXIMUM_MESSAGE_SIZE); + FAIL_FAST_UNEXPECTED_HOST_BEHAVIOR(); } CopyMem(fileSystemInformation->PacketBuffer, Buffer, BufferLength); @@ -381,10 +380,7 @@ Return Value: if (bytesRead != sizeof(*getFileInfoResponseMessage) || getFileInfoResponseMessage->Header.Type != VmbfsMessageTypeGetFileInfoResponse) { - - VMBFS_BAD_HOST; - status = EFI_DEVICE_ERROR; - goto Cleanup; + FAIL_FAST_UNEXPECTED_HOST_BEHAVIOR(); } if (getFileInfoResponseMessage->Status == VmbfsFileNotFound) @@ -591,9 +587,7 @@ Return Value: if (bytesReceived < sizeof(*readFileResponseMessage) || readFileResponseMessage->Header.Type != VmbfsMessageTypeReadFileResponse) { - VMBFS_BAD_HOST; - status = EFI_DEVICE_ERROR; - goto Cleanup; + FAIL_FAST_UNEXPECTED_HOST_BEHAVIOR(); } status = VmbfsErrorToEfiError(readFileResponseMessage->Status); @@ -605,9 +599,7 @@ Return Value: bytesReceived -= sizeof(VMBFS_MESSAGE_READ_FILE_RESPONSE); if (bytesReceived > bytesRequested) { - VMBFS_BAD_HOST; - status = EFI_DEVICE_ERROR; - goto Cleanup; + FAIL_FAST_UNEXPECTED_HOST_BEHAVIOR(); } CopyMem((UINT8*)Buffer, @@ -702,9 +694,7 @@ Return Value: if (bytesReceived < sizeof(*readFileResponseMessage) || readFileResponseMessage->Header.Type != VmbfsMessageTypeReadFileRdmaResponse) { - VMBFS_BAD_HOST; - status = EFI_DEVICE_ERROR; - goto Cleanup; + FAIL_FAST_UNEXPECTED_HOST_BEHAVIOR(); } status = VmbfsErrorToEfiError(readFileResponseMessage->Status); @@ -715,9 +705,7 @@ Return Value: if (readFileResponseMessage->ByteCount > bytesRequested) { - VMBFS_BAD_HOST; - status = EFI_DEVICE_ERROR; - goto Cleanup; + FAIL_FAST_UNEXPECTED_HOST_BEHAVIOR(); } *BytesRead = readFileResponseMessage->ByteCount; From a8c2fd4af4ef717eb9cbd7c7a3d525ebfbc813d3 Mon Sep 17 00:00:00 2001 From: Maheer Aeron Date: Fri, 22 May 2026 21:09:22 +0000 Subject: [PATCH 2/2] Merged PR 15664084: Bump mmio config logs to DEBUG_INFO and increase pre-mem pages Failures in PEI may not contain enough logs, so bump the pre-mem pages in the advanced logger. Additionally, use DEBUG_INFO for mmio information. ---- #### AI description (iteration 1) #### PR Classification Bug fix to improve MMIO configuration logging and increase pre-memory logger buffer size to address ARM64 UEFI MMU configuration failures. #### PR Summary This pull request enhances debugging capabilities for MMIO configuration issues in ARM64 UEFI by promoting log levels from DEBUG_VERBOSE to DEBUG_INFO and increasing the pre-memory logger buffer size to capture more boot-time diagnostics. - `Config.c`: Added DEBUG_INFO logging for resolved MMIO PCDs (LowGap and HighGap base/size) in bytes to make host-supplied MMIO layout visible in EfiDiagnostics ETW - `Config.c`: Bumped MMIO ranges debug output from DEBUG_VERBOSE to DEBUG_INFO for better visibility - `Mmu.c`: Changed ConfigureMmu debug log level from DEBUG_VERBOSE to DEBUG_INFO - `MsvmPkgAARCH64.dsc` and `MsvmPkgX64.dsc`: Increased PcdAdvancedLoggerPreMemPages from 1 (4KB) to 8 (32KB) to accommodate additional logging Related work items: #48431797, #62345035 --- MsvmPkg/MsvmPkgAARCH64.dsc | 2 +- MsvmPkg/MsvmPkgX64.dsc | 2 +- MsvmPkg/PlatformPei/AArch64/Mmu.c | 2 +- MsvmPkg/PlatformPei/Config.c | 41 +++++++++++++++++++++++++++++++ 4 files changed, 44 insertions(+), 3 deletions(-) diff --git a/MsvmPkg/MsvmPkgAARCH64.dsc b/MsvmPkg/MsvmPkgAARCH64.dsc index 48dccb27c1..645e66254b 100644 --- a/MsvmPkg/MsvmPkgAARCH64.dsc +++ b/MsvmPkg/MsvmPkgAARCH64.dsc @@ -355,7 +355,7 @@ [PcdsFixedAtBuild.common] # Advanced Logger Config - gAdvLoggerPkgTokenSpaceGuid.PcdAdvancedLoggerPreMemPages|1 # Size is 4KB + gAdvLoggerPkgTokenSpaceGuid.PcdAdvancedLoggerPreMemPages|8 # Size is 32KB gAdvLoggerPkgTokenSpaceGuid.PcdAdvancedLoggerPages|1024 # Size is 4MB !if $(DEBUGLIB_SERIAL) == 1 !ifdef DEBUG_NOISY diff --git a/MsvmPkg/MsvmPkgX64.dsc b/MsvmPkg/MsvmPkgX64.dsc index a62c04c226..2c048ac43b 100644 --- a/MsvmPkg/MsvmPkgX64.dsc +++ b/MsvmPkg/MsvmPkgX64.dsc @@ -376,7 +376,7 @@ # Original value: 0xFA000000 # gAdvLoggerPkgTokenSpaceGuid.PcdAdvancedLoggerBase|0x0 - gAdvLoggerPkgTokenSpaceGuid.PcdAdvancedLoggerPreMemPages|1 # Size is 4KB + gAdvLoggerPkgTokenSpaceGuid.PcdAdvancedLoggerPreMemPages|8 # Size is 32KB gAdvLoggerPkgTokenSpaceGuid.PcdAdvancedLoggerPages|1024 # Size is 4MB !if $(DEBUGLIB_SERIAL) == 1 !ifdef DEBUG_NOISY diff --git a/MsvmPkg/PlatformPei/AArch64/Mmu.c b/MsvmPkg/PlatformPei/AArch64/Mmu.c index 093beaf4cc..20e6860b9d 100644 --- a/MsvmPkg/PlatformPei/AArch64/Mmu.c +++ b/MsvmPkg/PlatformPei/AArch64/Mmu.c @@ -418,7 +418,7 @@ ConfigureMmu( return EFI_INVALID_PARAMETER; } - DEBUG((DEBUG_VERBOSE, "ConfigureMmu(0x%lx, 0x%lx, 0x%lx, 0x%lx)\n", + DEBUG((DEBUG_INFO, "ConfigureMmu(0x%lx, 0x%lx, 0x%lx, 0x%lx)\n", MaxAddress, lowMmioSize, highMmioBaseAddress, highMmioSize)); // diff --git a/MsvmPkg/PlatformPei/Config.c b/MsvmPkg/PlatformPei/Config.c index fbadf15987..f3e788e250 100644 --- a/MsvmPkg/PlatformPei/Config.c +++ b/MsvmPkg/PlatformPei/Config.c @@ -1561,6 +1561,47 @@ Return Value: PEI_FAIL_FAST_IF_FAILED(PcdSet64S(PcdLowMmioGapSizeInPages, mmioRanges->Ranges[lowGap].MmioSizeInPages)); PEI_FAIL_FAST_IF_FAILED(PcdSet64S(PcdHighMmioGapBasePageNumber, mmioRanges->Ranges[highGap].MmioPageNumberStart)); PEI_FAIL_FAST_IF_FAILED(PcdSet64S(PcdHighMmioGapSizeInPages, mmioRanges->Ranges[highGap].MmioSizeInPages)); + + // Log resolved MMIO PCDs in bytes; SafeUint64Mult guards the page-to-byte conversion. + { + UINT64 lowBaseBytes; + UINT64 lowSizeBytes; + UINT64 highBaseBytes; + UINT64 highSizeBytes; + + if (RETURN_ERROR(SafeUint64Mult(mmioRanges->Ranges[lowGap].MmioPageNumberStart, (UINT64)SIZE_4KB, &lowBaseBytes)) || + RETURN_ERROR(SafeUint64Mult(mmioRanges->Ranges[lowGap].MmioSizeInPages, (UINT64)SIZE_4KB, &lowSizeBytes))) + { + DEBUG((DEBUG_WARN, + "ConfigMmio: LowGap (pages) base=0x%lx size=0x%lx (byte conversion overflow)\n", + mmioRanges->Ranges[lowGap].MmioPageNumberStart, + mmioRanges->Ranges[lowGap].MmioSizeInPages)); + } + else + { + DEBUG((DEBUG_INFO, + "ConfigMmio: LowGap base=0x%lx size=0x%lx\n", + lowBaseBytes, + lowSizeBytes)); + } + + if (RETURN_ERROR(SafeUint64Mult(mmioRanges->Ranges[highGap].MmioPageNumberStart, (UINT64)SIZE_4KB, &highBaseBytes)) || + RETURN_ERROR(SafeUint64Mult(mmioRanges->Ranges[highGap].MmioSizeInPages, (UINT64)SIZE_4KB, &highSizeBytes))) + { + DEBUG((DEBUG_WARN, + "ConfigMmio: HighGap (pages) base=0x%lx size=0x%lx (byte conversion overflow)\n", + mmioRanges->Ranges[highGap].MmioPageNumberStart, + mmioRanges->Ranges[highGap].MmioSizeInPages)); + } + else + { + DEBUG((DEBUG_INFO, + "ConfigMmio: HighGap base=0x%lx size=0x%lx\n", + highBaseBytes, + highSizeBytes)); + } + } + requiredStructures.UefiConfigMmioRanges = 1; break; }