Fixes: #11113
Currently, the global handle key and key inside handle structure is
incremented only when a new handle is allocated for protocol interface
to be installed.
However, when caller already supplies a handle gHandleDatabaseKey never
get incremented.
Move handle key incremental outside if block, just below the else
statement which allows gHandleDatabaseKey to always incremented whether
handle is supplied or not.
Signed-off-by: Khalid Ali <khaliidcaliy@gmail.com>
When IA32 DxeIpl is setting up the IDT, it should populate
256 entries, the architectural size, so that exceptions are
handled in a defined way, going to the common exception handler.
This includes the stack cookie exception.
Signed-off-by: Oliver Smith-Denny <osde@microsoft.com>
This change adds validation to CoreGetMemoryMap to ensure that special
memory bins are fully respected. Specifically, any memory map entry that
falls within a special bin must be entirely contained within that bin,
and its memory type must match the bin's designated type.
This check helps preventing unintended changes that could cause the
system memory map to cross bin boundaries unexpectedly.
Signed-off-by: Kun Qin <kun.qin@microsoft.com>
Fixes: #11764
Currently, mSmiManageCallingDepth is incremented always whether SMI
handler is present or not. However get decremented only when SMI handler
is found. This causes mSmiManageCallingDepth to grow infinitely as long
as SMI handler isn't present.
Increment mSmiManageCallingDepth only when SMI handler presence is
confirmed.
Signed-off-by: Khalid Ali <khaliidcaliy@gmail.com>
Replace traditional `#ifndef`/`#define`/`#endif` include guards with
`#pragma` once.
`#pragma once` is a widely supported preprocessor directive that
prevents header files from being included multiple times. It is
supported by all toolchains used to build edk2: GCC, Clang/LLVM, and
MSVC.
Compared to macro-based include guards, `#pragma once`:
- Eliminates the risk of macro name collisions or copy/paste errors
where two headers inadvertently use the same guard macro.
- Eliminate inconsistency in the way include guard macros are named
(e.g., some files use `__FILE_H__`, others use `FILE_H_`, etc.).
- Reduces boilerplate (three lines replaced by one).
- Avoids polluting the macro namespace with guard symbols.
- Can improve build times as the preprocessor can skip re-opening the
file entirely, rather than re-reading it to find the matching
`#endif` ("multiple-include optimization").
- Note that some compilers may already optimize traditional include
guards, by recognzining the idiomatic pattern.
This change is made acknowledging that overall portability of the
code will technically be reduced, as `#pragma once` is not part of the
C/C++ standards.
However, this is considered acceptable given:
1. edk2 already defines a subset of supported compilers in
BaseTools/Conf/tools_def.template, all of which have supported
`#pragma once` for over two decades.
2. There have been concerns raised to the project about inconsistent
include guard naming and potential macro collisions.
Approximate compiler support dates:
- MSVC: Supported since Visual C++ 4.2 (1996)
- GCC: Supported since 3.4 (2004)
(http://gnu.ist.utl.pt/software/gcc/gcc-3.4/changes.html)
- Clang (LLVM based): Since initial release in 2007
Signed-off-by: Michael Kubacki <michael.kubacki@microsoft.com>
The documentation of InstallPpi() and NotifyPpi() in both the spec and
function comment indicate to return EFI_OUT_OF_RESOURCES if memory
allocation fails.
However, the implementation of those two services assert if memory
allocation fails. This is a mismatch between what the function expected to
return and what actually returns.
Fix this by returning EFI_OUT_OF_RESOURCES if memory allocation fails,
so the code matches the documentation and comply with the spec. It is
expected service consumers(callers of these functions) to handle
failures appriciately.
Signed-off-by: Khalid Ali <khaliidcaliy@gmail.com>
Fix alignment issues in memory map entries returned by
GetMemoryMap() when RUNTIME_PAGE_ALLOCATION_GRANULARITY is
larger than DEFAULT_PAGE_ALLOCATION_GRANULARITY.
There are no issues in the Page/Pool Allocation/Free services.
Logic issues issues are addressed in the memory map returned
by GetMemoryMap() due to missing cases for memory map entries
of type EfiConventionalMemory that overlap special memory bins.
Add logic to handle all possible memory map splits required to
convert internal memory map entries into an EFI Memory Map with
EFI Memory Map entries the follow alignment requirements when
the EFI Memory Map entries cover memory bins.
The four cases that must be handled are:
* Memory map entry contained within a bin. [Already covered]
Convert memory map entry type
* Memory map entry overlaps beginning of bin. [Added]
Split memory map entry at beginning of bin.
* Memory map entry overlaps end of bin. [Added]
Split memory map entry at end of bin.
* Memory map entry overlaps entire bin. [Added]
Split memory map entry at both ends of bin.
Signed-off-by: Michael D Kinney <michael.d.kinney@intel.com>
Fix alignment issues in memory map entries returned by
GetMemoryMap() when RUNTIME_PAGE_ALLOCATION_GRANULARITY is
larger than DEFAULT_PAGE_ALLOCATION_GRANULARITY.
Alignment issues are addressed in the initial memory map
layout when Memory Type Information is provided with memory
bins that use RUNTIME_PAGE_ALLOCATION_GRANULARITY.
There are no issues in the Page/Pool Allocation/Free services.
* CoreSetMemoryTypeInformationRange() make sure there is room
for all bins when accounting for alignment requirements.
Allocate space for bins with base and length following
alignment requirements.
* CoreSetMemoryTypeInformationRange() round up NumberOfPages in
Memory Type Information based on alignment requirements.
This is required so GetMemoryMap() will generate memory
map entries that always follow alignment requirements.
* CoreAddMemoryDescriptor() round up NumberOfPages in
Memory Type Information based on alignment requirements.
This is required so GetMemoryMap() will generate memory
map entries that always follow alignment requirements.
Signed-off-by: Michael D Kinney <michael.d.kinney@intel.com>
Commit 2d69507a4d added an attribute check to
prevent freeing memory that is read-only, read-protected, or for which
attribute retrieval fails. In such cases the code returned EFI_SUCCESS and
leaked the memory.
This introduced a regression in the System Architecture Compliance Suite
(ACS) BS.FreePages – Not Existent Memory test.
Link: https://github.com/tianocore/edk2-test/blob/edk2-test-stable202509/uefi-sct/Doc/TestCaseSpec/03_Services_Boot_Services.md#freepages
Test number: 5.1.2.2.1
GetMemoryAttributes() returns EFI_UNSUPPORTED for memory regions outside
system memory. The previous change treated all errors as a reason to leak
memory, while only the EFI_NO_MAPPING error code should trigger that
behavior. As a result, freeing non-existent memory incorrectly returned
EFI_SUCCESS instead of EFI_NOT_FOUND.
To fix this, memory is now leaked only when:
- GetMemoryAttributes() returns EFI_NO_MAPPING (inconsistent attributes),
or
- GetMemoryAttributes() succeeds and the pages are marked RO or RP.
All other errors fall through to CoreInternalFreePages(), restoring the
previous and correct behavior.
Signed-off-by: Piotr Wejman <piotr.wejman@arm.com>
Replace UINTN casts with EFI_PHYSICAL_ADDRESS in places where the result
is negated, as otherwise, the top bits may remain 0 unexpectedly.
VS2022 started warning about this, and thus breaking the IA32 CI build.
Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
Address the build regressions, introduced in #11724, #11688, #11686#11685.
These build regressions are for uninitialized variables before use.
Signed-off-by: Aaron Pop <aaronpop@microsoft.com>
https://codeql.github.com/codeql-query-help/cpp/cpp-comparison-with-wider-type
If the narrow type (smaller range) is compared against a wide type
(larger range), the narrow value may overflow before reaching the wide
value. This can cause unexpected behavior, such as:
Infinite loops (loop condition never becomes false).
Incorrect logic (comparison results are misleading).
Signed-off-by: Aaron Pop <aaronpop@microsoft.com>
During DXE core memory service initialization, the system would check
available resource descriptor hobs against the memory top from PHIT hob.
However, it is possible that a given resource descriptor hob will not be
larger than the cover the memory top, causing the Length calculation to
underflow.
This change adds a check for potential underflow before performing the
subtraction.
Signed-off-by: Kun Qin <kun.qin@microsoft.com>
Current GCD logic uses plain addition calulation when iterating through
the resource descriptor hobs. However, if the resource descriptor is
incorrectly prepared, this could cause incorrect memory initialization
and other failures down the boot process.
This change adds an overflow check before using the value.
Signed-off-by: Kun Qin <kun.qin@microsoft.com>
The current implementation from Dxe/Image/Image.c does not handle the
configuration where there might be multiple MemoryAllocationModules.
Given that the `ModuleName` is included in the hob data and used for
targetting the consumer, DXE core should specify the GUID when looking up
for its own MemoryAllocationModule.
This change adds a check to ensure the located hob is targetting DXE
core.
Signed-off-by: Kun Qin <kuqin12@gmail.com>
The DXE_SAL_DRIVER module type was introduced to support
Itanium (IPF) platforms. Since support for Itanium processors
has been dropped, the instances of DXE_SAL_DRIVER
have been removed.
Ref: [3cb0a311cb]
Cc: Sachin Ganesh <sachinganesh@ami.com>
Signed-off-by: Sathya Ravichandran <sathyar@ami.com>
PR #11443 introduced a regression by adding a DEBUG() message
when the lock for events is acquired and that lock is at
TPL_HIGH_LEVEL. If DEBUG() messages are routed through Report
Status Code, and the Report Status Code Protocol has not been
located yet, then a call to gBS->LocateProtocol() is made and
that call raises TPL to TPL_NOTIFY which causes a TPL inversion.
The event lock is used to atomically update gEventSignalQueue.
There is no need for the DEBUG() message to within the event
lock scope.
The fix is to scope the event lock to only the InsertHeadList()
call to update gEventSignalQueue.
Signed-off-by: Michael D Kinney <michael.d.kinney@intel.com>
Memory Attributes Table needs to be updated to contain executable
permissions for UEFI runtime drivers loaded after EndOfDxe.
Fixes a regression introduced by bb248a9.
Signed-off-by: Vitaly Cheptsov <vit9696@protonmail.com>
Following the APs now always initializing separate exception
stacks, this commit always initializes a separate exception
stack for the BSP as well. Previously, this was only enabled
when PcdCpuStackGuard was set.
However, even when a stack guard page is not present,
stack overflows can still occur and corrupt the stack; if an
exception is taken here, it is still valuable to have a separate
exception stack for sanity.
Signed-off-by: Oliver Smith-Denny <osde@microsoft.com>
Commit 43e306806e added
EFIGcdMemoryTypeUnaccepted (as it was later renamed) to be
returned in the EFI_MEMORY_MAP. However, it did not add it
to the number of entries calculation, so if any
EfiGcdMemoryTypeUnaccepted entries exist in the GCD they will
overflow the EFI_MEMORY_MAP buffer provided by the bootloader.
This resolves that by accounting for unaccepted entries in
the number of entries calculation.
Signed-off-by: Oliver Smith-Denny <osde@microsoft.com>
edk2 commit 3bd5c994c8 added usage
of EFI_MEMORY_ATTRIBUTE_MASK to edk2. However, it applied it
incorrectly to some places that should instead use
EFI_MEMORY_ACCESS_MASK. EFI_MEMORY_ACCESS_MASK contains the actual
HW page table access attributes (read protect, read only, no-execute),
whereas EFI_MEMORY_ATTRIBUTE_MASK contains the access attributes in
addition to some virtual attributes (special purpose and cpu crypto).
The GCD has a behavior where if SetMemorySpaceAttributes() is called
with only virtual attributes set, it will not call into CpuDxe to
change the attributes; 0 is a valid page table attribute set (it means
RWX). However, after the above change, this behavior was altered so
that if EFI_MEMORY_SP or EFI_MEMORY_CPU_CRYPTO is applied, in attempt
to just update these virtual attributes, the GCD will call into CpuDxe
and apply RWX instead, which is not the intention of the caller.
One other place this was done incorrectly was in CoreGetMemoryMap,
but that was fixed in f1567720b1.
SetUefiImageMemoryAttributes() is also updated here because that
logic was copied from the check the GCD has about whether to call
CpuDxe or not. Now that the GCD has been corrected, this also
needs to be corrected.
Signed-off-by: Oliver Smith-Denny <osde@microsoft.com>
edk2 is dropping support for the ARM32 architecture. This
commit removes ARM32 support from MdeModulePkg. This also
drops irrelevant VALID_ARCHITECTURE comments from infs that
are not arch specific.
Signed-off-by: Oliver Smith-Denny <osde@microsoft.com>
The current fixed value of 8 for `DELAYED_DISPATCH_MAX_ENTRIES` is
not large enough to accommodate platform usage. This change replaces
the macro with a PCD that can be configured by platforms.
In the case the default PCD value is too small, an error message
explaining that the PCD should be updated will be printed followed
by an assert.
Signed-off-by: Michael Kubacki <michael.kubacki@microsoft.com>
There are a lot of notify callback events for Event Groups. Usually they
are not reported unless there is a debug code in the callback itself.
The debug message helps to check which/when the callback is registered and
executed in POST. Also helps to notice the callback sequence. It depends
on DEBUG_EVENT flag enabled by PcdFixedDebugPrintErrorLevel PCD token.
Signed-off-by: Phil Noh <Phil.Noh@amd.com>
When a BIOS implementation splits the PEIMs in multiple firmware volumes and
the FVs are reported to PEI Core by different PEIMs at different time,
it is hard to conclude the exact FV dispatch order.
The change updates the existing PEI dispatch logic to print the GUID of the
firmware volume and the FFS file which contains a nested FV.
It helps developers to match the FV being dispatched to the FV defined in
FDF file.
Signed-off-by: Ray Ni <ray.ni@intel.com>
Cc: Liming Gao <gaoliming@byosoft.com.cn>
The various cores all attempt to print the EfiFileName when
loading/dispatching drivers, but they are not unified on
approach. This commit ensures they are using the same buffer
size and the loop parsing variables are unsigned, as we should
not have a negative index.
Signed-off-by: Oliver Smith-Denny <osde@microsoft.com>
Today, DXE/PEI/SMM Core's image loaders only print driver load messages
if debug code is enabled. However, these are some of the most
important prints in the codebase: on a given system even if you
have nothing else to debug with, you can see the last driver
executed.
Debug code blocks are used to skip logic that only exists for
debug purposes and wastes time on a release build. However, the
logic to print a line and determine the filename from the PDB
is not extensive and provides critical information, so it is
inappropriate to wrap in a debug code section.
Platforms can still choose to disable logging at DEBUG_INFO/DEBUG_LOAD
and will not see the error messages.
Signed-off-by: Oliver Smith-Denny <osde@microsoft.com>
Currently, if the DebugClearMemory bit is set in the
PcdDebugPropertyMask, CoreConvertPagesEx will attempt to write
a pattern to the pages being freed. However, it does not check
that the page is writeable, which will cause a page fault if not.
Furthermore, if NX protections are not enabled, the core does not
ensure that any freed pages are RW, which is the state expected
when they are allocated next. If they are not RW, the allocating
driver will crash trying to use them.
This patch updates the page freeing code to query the memory
attributes protocol, if present, for the attributes. If this call
fails or the attributes are not RW at a minimum, the core leaks
the memory (returning success to the caller). If the memory
attribute protocol is not present (either because a platform doesn't
produce it or it is before the protocol has been produced, the core
continues with freeing memory. This is either before the CPU Arch
protocol is available (so drivers can't change memory attributes) or
otherwise matches existing behavior. This was deemed the best
approach to let memory that can't be guaranteed to be RW leak
instead of letting a driver crash when allocating it. It was deemed
less brittle to simply leak the memory instead of attempting to
change the attributes.
Signed-off-by: Oliver Smith-Denny <osde@microsoft.com>
Currently DxeIpl attempts to set page 0 to all 0's and to
create a memory allocation HOB for it. However, DxeIpl will
also unmap the page when mapping page tables and if null
detection is not enabled, DxeCore will set the page to 0,
regardless of allocation status.
Because no consumers are using the memory allocation HOB for
page 0, drop it. Instead, ensure that PeiCore and DxeCore do
not allow allocating page 0; it should always be reserved for
null pointer detection. It also complicates the story for
platforms that are attempting to audit the system and ensure that
no modules are using page 0. With these memory allocation HOBs
in place, it is difficult to tell if it is simply DxeIpl who
has allocated the memory or another module.
This commit drops the memory allocation HOB publishing and ensures
that DxeCore and PeiCore do not allocate page 0. DxeCore already
will not allocate page 0 to callers of AllocatePages who call with
a type other than AllocateAddress, this just changes so that
AllocateAddress cannot allocate at page 0 (which if null detection
is enabled will cause a page fault). PeiCore does not have
AllocateAddress and so this ensures standard allocations do not
receive page 0.
Signed-off-by: Oliver Smith-Denny <osde@microsoft.com>
This change adds a check to ensure the incoming buffer is correctly using
MM communicate v3 header before dereferencing the content.
Signed-off-by: Kun Qin <kun.qin@microsoft.com>
The DebugImageInfoTable contains an array of image info
structures. The current implementation removes an entry by
freeing the info structure and putting NULL in that entry of
the array. It then decrements the table size tracked in the table.
However, the array is invalid at this point, it contains a NULL
entry, which the UEFI spec does not envision and it contains a valid
entry past the end of the array as tracked in the spec defined config
table. If the table is consumed at this point it can lead to an
invalid assessment of the image state, which defeats the purpose of
the table.
When a new info structure is added, it then scans for the first NULL
entry adds a pointer to the new info structure there and increments
the table size to cover the entrythat was formerly past the end of
the array.
The current implementation requires that once an unload happens,
more loads happen than unloads and that the last operation is not
an unload (which won't be true in the shell, e.g.). This is
needlessly complex, as the order of the table doesn't matter
(and in fact this implementation doesn't preserve image loading
order either).
This patch updates the removal function to free the desired
info structure, move the last entry of the array to this freed
spot, mark the last entry as NULL, and decrement the table count.
The entry addition function then just always puts a new entry at
the end of the array, expanding it as necessary. This simplifies
the logic and covers the gaps that were present.
Signed-off-by: Oliver Smith-Denny <osde@microsoft.com>
Commit 5ccb5fff02 updated the
image memory protection code to set the protection
attributes through the GCD instead of directly to the page
table. However, this code had an implicit assumption that
each base address passed to it was the beginning of a GCD
descriptor. On the virtual platforms tested, this was the case.
However, on a physical platform, a scenario was encountered
where the base address was not the beginning of a GCD
descriptor, thus causing memory attributes to be applied
incorrectly.
This assumption does not need to be made and this patch
updates the code to handle the case where the base address
is not the beginning of a GCD descriptor.
Signed-off-by: Oliver Smith-Denny <osde@microsoft.com>
CoreDumpGcdMemorySpaceMap() gets called on every update to
the GCD, but it only prints if DEBUG_GCD is set. However,
the compiler is not smart enough to remove all of this logic
if we are not printing anything, so we end up needlessly
allocating memory for the copy of the map and spending many
cycles looping through each entry, only to not print anything.
This code is compiled out on release builds, but slows down
debug builds that aren't printing at DEBUG_GCD level.
This patch updates CoreDumpGcdMemorySpaceMap() to shortcircuit
and immediately exit if DEBUG_GCD is not set. It also adds
the same logic to CoreDumpGcdIoSpaceMap(), which is called
less frequently, but has the same issue.
Signed-off-by: Oliver Smith-Denny <osde@microsoft.com>
Today, SetUefiImageMemoryAttributes calls directly to the
CPU Arch protocol to set EFI_MEMORY_XP or EFI_MEMORY_RO on
image memory. However, this bypasses the GCD and so the GCD
is out of sync with the actual state of memory.
This can cause an issue in the scenario where a new attribute
is being set (whether a virtual attribute or a real HW attribute),
if the GCD attributes are queried for a region and the new attribute
is appended to the existing GCD attributes (which are incorrect),
then the incorrect attributes can get applied. This can result in
setting EFI_MEMORY_XP on code sections of images and causing an
execution fault.
This patch updates SetUefiImageMemoryAttributes to call into the
GCD to update the attributes there and let the GCD code call into
the CPU Arch protocol to update the page table.
Signed-off-by: Oliver Smith-Denny <osde@microsoft.com>
The print that describes memory attributes being applied to image
memory sections is currently at info level and very noisy, being
printed multiple times per image.
Reduce this to the verbose logging level.
Signed-off-by: Oliver Smith-Denny <osde@microsoft.com>
With current implemenation, all 3 SmmCommunication* functions go through
the same routine, which will dereference the incoming pointer to inspect
whether this is a V3 buffer or not.
However, the caller always pass in the physical addresses, which could
cause the system to page fault after OS take over the runtime control.
This change reverted the common routine to its previous form to handle MM
communicate v1 and v2. Additionally, a specific communicate function for
v3 was created to support MM communicate v3.
Co-authored-by: Michael D Kinney <michael.d.kinney@intel.com>
Signed-off-by: Kun Qin <kun.qin@microsoft.com>
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3398
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3430
MM communicate protocols are expanded with EFI_MM_COMMUNICATE_HEADER_V3
structure that cooperates with updated field types and flexible array.
The PiSmmCore implementation is updated to detect and process incoming
data accordingly.
Two checks are also performed to prevent legacy communicate data or
unsupported data is fed into MM core under agreed header guid.
Cc: Jian J Wang <jian.j.wang@intel.com>
Cc: Hao A Wu <hao.a.wu@intel.com>
Cc: Eric Dong <eric.dong@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Signed-off-by: Kun Qin <kuqin12@gmail.com>
Hot Pluggable resource attribute was introduced in UEFI 2.11 and PI 1.9
specifications.
This type should have an entry in the Attribute Conversion Table.
Signed-off-by: Sachin Ganesh <sachinganesh@ami.com>
Add the EFI_RESOURCE_ATTRIBUTE_SPECIAL_PURPOSE resource attribute as
per the PI 1.8 spec. This flag is used to indicate that the memory
should be treated as special purpose memory (SPM).
Cc: Liming Gao <gaoliming@byosoft.com.cn>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Signed-off-by: Michael Kubacki <michael.kubacki@microsoft.com>
Handle.c:1302:24: error: 'Prot' may be used uninitialized
in this function [-Werror=maybe-uninitialized]
*Interface = Prot->Interface;
~~~~^~~~~~~~~~~
cc1: all warnings being treated as errors
Signed-off-by: Mike Maslenkin <mike.maslenkin@gmail.com>
Dispatcher.c:1639:34: error: 'FvMigrationFlags' may be used uninitialized
in this function [-Werror=maybe-uninitialized]
(((FvMigrationFlags & FLAGS_FV_MIGRATE_BEFORE_PEI_CORE_REENTRY) == 0) ||
~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
cc1: all warnings being treated as errors
Signed-off-by: Mike Maslenkin <mike.maslenkin@gmail.com>
PeiCoreBuildHobHandoffInfoTable() always returns EFI_SUCCESS, and also
its return is not consumed at all, so this patch removes return for
PeiCoreBuildHobHandoffInfoTable().
Signed-off-by: Star Zeng <star.zeng@intel.com>
Add a late initialize in DxeMain for the debug agent. This is required
for the debug agent to be able to setup events to handle image loads,
exit boot services, and other important callbacks.
Define a reinitialize debug agent.
Signed-off-by: Aaron Pop <aaronpop@microsoft.com>
The scratch buffer (EfiBootServicesData) is assigned to extract DXE FVs
that are compressed. The matching decompression library returns the buffer
size as below. The buffer is no longer used after completing extraction.
Need to free the buffer to optimize memory allocation and usage.
BaseUefiDecompressLib : sizeof (SCRATCH_DATA)
LzmaCustomDecompressLib : SCRATCH_BUFFER_REQUEST_SIZE (64KB)
BrotliCustomDecompressLib : From EncodeData header (usually, xxMB checked)
In case of Brotli decompression, it is found that a big chunk of memory is
required, based on EncodeData header. (e.g. a 4MB compressed FV reports
about 39MB scratch size)
Signed-off-by: Phil Noh <Phil.Noh@amd.com>
