V2 update: Directly use NetIp4IsUnicast() to check station address in IP driver.
This patch is to follow RFC3021 which allows to use 31-bit mask
in point-to-point link.
If a 31-bit subnet mask is assigned to a point-to-point link, it
leaves the <Host-number> with only 1 bit. Consequently, only two
possible addresses may result:
{<Network-number>, 0} and {<Network-number>, -1}
These addresses have historically been associated with network and
broadcast addresses (see Section 2.2). In a point-to-point link with
a 31-bit subnet mask, the two addresses above MUST be interpreted as
host addresses.
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Fu Siyuan <siyuan.fu@intel.com>
Reviewed-by: Wu Jiaxin <jiaxin.wu@intel.com>
https://lists.01.org/pipermail/edk2-devel/2017-October/016479.html
reported "Xen Console input very slow in recent UEFI" that appears
after 4cf3f37c87 "MdeModulePkg
SerialDxe: Process timeout consistently in SerialRead".
Julien did more debugging and find out the following is happening in
TerminalConInTimerHandler (MdeModulePkg/Universal/Console/TerminalDxe)
when a character is received:
1) GetControl will return EFI_SERIAL_INPUT_BUFFER_EMPTY unset
=> Entering in the loop to fetch character from the serial
2) GetOneKeyFromSerial()
=> Return directly with the character read
3) Looping as the fifo is not full and no error
4) GetOneKeyFromSerial() -> SerialRead()
=> No more character so SerialPortPoll() will return FALSE and loop
until timeout
=> Return EFI_TIMEOUT
5) Exiting the loop from TerminalConInTimerHandler
6) Characters are printed
After some investigation, I found it is related to the Timeout value.
The Timeout is 1000000 (1s) by default to follow UEFI spec.
And the Terminal driver will recalculate and set the Timeout value
based on the properties of UART in TerminalDriverBindingStart()/
TerminalConInTimerHandler().
SerialInTimeOut = 0;
if (Mode->BaudRate != 0) {
//
// According to BAUD rate to calculate the timeout value.
//
SerialInTimeOut = (1 + Mode->DataBits + Mode->StopBits) *
2 * 1000000 / (UINTN) Mode->BaudRate;
}
For example, based on the PCD values of PcdUartDefaultBaudRate,
PcdUartDefaultDataBits and PcdUartDefaultStopBits, SerialInTimeOut =
(1 + 8 + 1) * 2 * 1000000 / (UINTN) 115200 = 173 (us).
When SerialDxe is used,
TerminalDriverBindingStart()/TerminalConInTimerHandler() ->
SerialIo->SetAttributes() ->
SerialSetAttributes() ->
SerialPortSetAttributes()
Some implementations of SerialPortSetAttributes() could handle the
input parameters and return RETURN_SUCCESS, for example
BaseSerialPortLib16550, then Timeout value will be changed to 173 (us),
no "slow down" will be observed.
But some implementations of SerialPortSetAttributes() just return
RETURN_UNSUPPORTED, for example XenConsoleSerialPortLib, then Timeout
value will be not changed and kept 1000000 (1s), "slow down" will be
observed.
SerialPortLib instance can be enhanced to
1. Handle the input parameters and return status accordingly instead of
just returning RETURN_UNSUPPORTED in SerialPortSetAttributes().
2. Just return RETURN_SUCCESS instead of RETURN_UNSUPPORTED in
SerialPortSetAttributes() if the instance does not care the input
parameters at all.
And SerialDxe can also be enhanced like this patch to be more robust
to handle Timeout change.
Cc: Julien Grall <julien.grall@linaro.org>
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Ruiyu Ni <ruiyu.ni@intel.com>
Compare against the original parameters
Suggested-by: Laszlo Ersek <lersek@redhat.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Star Zeng <star.zeng@intel.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Tested-by: Julien Grall <julien.grall@linaro.org>
AllocateCopyPool(AllocationSize, *Buffer) will copy "AllocationSize" bytes of
memory from old "Buffer" to new allocated one. If "AllocationSize" is bigger
than size of "Buffer", heap memory overflow occurs during copy.
One solution is to allocate pool first then copy the necessary bytes to new
memory. Another is using ReallocatePool instead if old buffer will be freed
on spot.
Cc: Star Zeng <star.zeng@intel.com>
Cc: Eric Dong <eric.dong@intel.com>
Cc: Bi Dandan <dandan.bi@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Jian J Wang <jian.j.wang@intel.com>
Reviewed-by: Star Zeng <star.zeng@intel.com>
Reviewed-by: Bi Dandan <dandan.bi@intel.com>
For PciIoPciRead interface, memory prior to Buffer would be written
with zeros if Offset was larger than sizeof (Dev->ConfigSpace), which
would cause serious system exception.
So we add a pre-check branch to avoid memory override.
Cc: Star Zeng <star.zeng@intel.com>
Cc: Eric Dong <eric.dong@intel.com>
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Ruiyu Ni <ruiyu.ni@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Heyi Guo <heyi.guo@linaro.org>
Reviewed-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
The patch ensures all DMA transactions are blocked after
ExitBootService.
If a platform enables IOMMU before and needs disable IOMMU after
ExitBootService, the IOMMU should be disabled after PCI bus driver
disables BME.
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Michael Turner <michael.turner@microsoft.com>
Signed-off-by: Ruiyu Ni <ruiyu.ni@intel.com>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Reviewed-by: Jiewen Yao <jiewen.yao@intel.com>
Cc: Jeff Fan <vanjeff_919@hotmail.com>
V3:
(1) Reset QuestionStoredInBitField to FALSE at end opcode(EFI_IFR_END_OP)
(2) Fix typo and format issues(line alignment for debug print message
and value assignment...)
V2:
(1)Remove the VarOffsetBitLevel/StorageWidthBitLevel to reduce the final
VarCheckBinSize and update the implementation accordingly.
(2)Update the VAR_CHECK_HII_REVISION
(3)Refine the Debug message and function comments,like update oneof",
"checkbox", "numeric" to "OneOf", "CheckBox", "Numeric".
VarCheckHiiLib check the value set to storage based on the possible value
listed in the vfr file. Since we have enhanced vfr to support Question
value stored in bit field, so now enhance VarCheckHiiLib to support bit
field check.
Cc: Star Zeng <star.zeng@intel.com>
Cc: Eric Dong <eric.dong@intel.com>
Cc: Liming Gao <liming.gao@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Dandan Bi <dandan.bi@intel.com>
Reviewed-by: Star Zeng <star.zeng@intel.com>
In patch 2, we will introduce DEBUG_INFO in VarCheckHiiLib,in order to keep
consistence, replace all EFI_D_INFO with DEBUG_INFO firstly in this pacth.
Cc: Star Zeng <star.zeng@intel.com>
Cc: Eric Dong <eric.dong@intel.com>
Cc: Liming Gao <liming.gao@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Dandan Bi <dandan.bi@intel.com>
Reviewed-by: Star Zeng <star.zeng@intel.com>
Currently, we complete a synchronous operation without unmapping the
DMA mappings, and free the pages using FreePages () rather than calling
EFI_PCI_IO_PROTOCOL::FreeBuffer. This is simply incorrect, but it also
breaks non-coherent DMA as well as DMA protection and/or memory encryption
so let's do it correctly and call SdMmcFreeTrb() instead.
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Reviewed-by: Hao Wu <hao.a.wu@intel.com>
Reviewed-by: Star Zeng <star.zeng@intel.com>
Mark EFI_VARIABLE_AUTHENTICATED_WRITE_ACCESS as deprecated.
1. Make SetVariable/QueryVariableInfo return EFI_UNSUPPORTED with this
attribute
2. No change to GetVariable/GetNextVariableName
Also update several function descriptors accordingly
Cc: Long Qin <qin.long@intel.com>
Cc: Star Zeng <star.zeng@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Chao Zhang <chao.b.zhang@intel.com>
Reviewed-by: Long Qin <qin.long@intel.com>
It's a regression of below commit:
SHA-1: 8be37a5cee
* MdeModulePkg/SecurityStubDxe: Defer 3rd party image before EndOfDxe
When PciBus driver fails to load the Option ROM, it doesn't produce
BusOverride protocol. It was a correct behavior before the above
commit. But due to the above commit, BusOverride protocol never is
produced by PciBus driver.
The patch fixes this issue using the following solution:
1. PciBus records the image device path when LoadImage fails.
2. Override.GetDriver() tries to look for the image handle using
the stored image device path.
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ruiyu Ni <ruiyu.ni@intel.com>
Reviewed-by: Star Zeng <star.zeng@intel.com>
The patch shouldn't impact functionality.
Original code records the ImageHandle of Option ROM by creating a
new entry. It's not necessary.
The patch updates the ImageHandle in the old entry.
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ruiyu Ni <ruiyu.ni@intel.com>
Reviewed-by: Star Zeng <star.zeng@intel.com>
The patch doesn't impact real functionality.
It only renames EFI_PCI_ROM_IMAGE_MAPPING to PCI_ROM_IMAGE,
and changes prototype of PciRomAddImageMapping so that
no explicit type cast is needed when calling this function.
It also removes unused field RomBase from PCI_IO_DEVICE structure.
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ruiyu Ni <ruiyu.ni@intel.com>
Reviewed-by: Star Zeng <star.zeng@intel.com>
Clearing I/O port decoding in the PCI command register at
ExitBootServices() breaks IDE boot in Windows, on QEMU's "pc" (i440fx)
machine type. (AHCI boot on "q35" is unaffected.) Windows seems repeatedly
stuck, apparently waiting for a timeout of sorts.
This is arguably a Windows bug; a native OS driver should not expect the
firmware to leave the PCI command register in any particular state.
Strictly speaking, we only need to disable BM-DMA at ExitBootServices(),
in order to abort pending transfers to/from RAM, which is soon to be owned
by the OS. BM-DMA is also the only bit that's explicitly named by the UEFI
Driver Writers' Guide, for clearing at ExitBootServices().
I've verified that clearing only BM-DMA fixes the issue (boot time) on
i440fx, and does not regress q35/AHCI.
Cc: Aleksei Kovura <alex3kov@zoho.com>
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Dann Frazier <dannf@ubuntu.com>
Cc: Eric Dong <eric.dong@intel.com>
Cc: Star Zeng <star.zeng@intel.com>
Reported-by: Aleksei Kovura <alex3kov@zoho.com>
Reported-by: Dann Frazier <dannf@ubuntu.com>
Reported-by: https://launchpad.net/~cjkrupp
Bisected-by: Dann Frazier <dannf@ubuntu.com>
Bisected-by: https://launchpad.net/~cjkrupp
Suggested-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Suggested-by: Star Zeng <star.zeng@intel.com>
Ref: https://bugs.launchpad.net/ubuntu/+source/edk2/+bug/1725560
Fixes: 6fb8ddd36b
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Tested-by: dann frazier <dann.frazier@canonical.com>
Reviewed-by: Star Zeng <star.zeng@intel.com>
Tested-by: Aleksei Kovura <alex3kov@zoho.com>
According the UEFI spec 2.7 A:
In section 28.3.2 for the IpConfigData.UseDefaultAddress, "While set to
TRUE, Configure() will trigger the EFI_IP4_CONFIG2_PROTOCOL to retrieve
the default IPv4 address if it is not available yet."
In section 28.5 for the Ip4Config2PolicyDhcp, "...All of these configurations
are retrieved from DHCP server or other auto-configuration mechanism."
This patch is to align with the above description. When the default IPv4
address is not available and IpConfigData.UseDefaultAddress is set to TRUE,
Ip4Config2 protocol will be called to retrieve the default address by setting
the policy to Ip4Config2PolicyDhcp.
Cc: Ye Ting <ting.ye@intel.com>
Cc: Fu Siyuan <siyuan.fu@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Wu Jiaxin <jiaxin.wu@intel.com>
Reviewed-by: Fu Siyuan <siyuan.fu@intel.com>
VariableRuntimeDxe deletes and locks the MorLock variable in
MorLockInit(), with the argument that any protection provided by MorLock
can be circumvented if MorLock can be overwritten by unprivileged code
(i.e., outside of SMM).
Extend the argument and the logic to the MOR variable, which is supposed
to be protected by MorLock. Pass Attributes=0 when deleting MorLock and
MOR both.
This change was suggested by Star; it is inspired by earlier VariableSmm
commit fda8f631ed ("MdeModulePkg/Variable/RuntimeDxe: delete and lock
OS-created MOR variable", 2017-10-03).
Cc: Eric Dong <eric.dong@intel.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Star Zeng <star.zeng@intel.com>
Suggested-by: Star Zeng <star.zeng@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Star Zeng <star.zeng@intel.com>
Make the comments align with the function.
And add some missing function comments.
Cc: Jian J Wang <jian.j.wang@intel.com>
Cc: Eric Dong <eric.dong@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Dandan Bi <dandan.bi@intel.com>
Reviewed-by: Jian J Wang <jian.j.wang@intel.com>
The bug was caused by 728d74973c
"MdeModulePkg/PciBus: Count multiple hotplug resource paddings".
The patch firstly updated the Bridge->Alignment to the maximum
alignment of all devices under the bridge, then aligned the
Bridge->Length to Bridge->Alignment.
It caused too much resources were claimed.
The new patch firstly aligns Bridge->Length to Bridge->Alignment,
then updates the Bridge->Alignment to the maximum alignment of all
devices under the bridge.
Because the step to update the Bridge->Alignment is to make sure
the resource allocated to the bus under the Bridge meets all
devices alignment. But the Bridge->Length doesn't have to align
to the maximum alignment.
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ruiyu Ni <ruiyu.ni@intel.com>
Reviewed-by: Eric Dong <eric.dong@intel.com>
Existing code forgot to set Descriptor->SpecificFlag to 0 when
the resource type is non-prefetchable MMIO.
The patch adds the missing assignment.
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ruiyu Ni <ruiyu.ni@intel.com>
Reviewed-by: Eric Dong <eric.dong@intel.com>
This is the regression issue. After apply CalculateCrc32(), the parameter
check is missing.
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Liming Gao <liming.gao@intel.com>
Reviewed-by: Wu Hao A <hao.a.wu@intel.com>
Install EdkiiSmmEndOfS3ResumeProtocol when S3 resume finished.
S3ResumePei will send S3 resume finished event to SmmCore through
communication buffer.
V2 change:
None.
V3 change:
1. Uninstall the protocol right after install it to avoid run out of memory.
Cc: Ruiyu Ni <ruiyu.ni@intel.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Eric Dong <eric.dong@intel.com>
Reviewed-by: Ruiyu Ni <ruiyu.ni@intel.com>
Reviewed-by: Jiewen Yao <jiewen.yao@intel.com>
Add gEdkiiSmmEndOfS3ResumeProtocolGuid which used by SmmCore to
notify smm drives that S3 resume has finished.
Cc: Ruiyu Ni <ruiyu.ni@intel.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Eric Dong <eric.dong@intel.com>
Reviewed-by: Ruiyu Ni <ruiyu.ni@intel.com>
Reviewed-by: Jiewen Yao <jiewen.yao@intel.com>
Current implementation skips to check whether the last four
characters are digits when the OptionNumber is NULL.
Even worse, it may incorrectly return FALSE when OptionNumber is
NULL.
The patch fixes it to always check the variable name even
OptionNumber is NULL.
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ruiyu Ni <ruiyu.ni@intel.com>
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
BmCharToUint() could be called using external data and it
already contains logic to return -1 when data is invalid,
so removing unnecessary assertion to avoid system hang.
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ruiyu Ni <ruiyu.ni@intel.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Star Zeng <star.zeng@intel.com>
Acked-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
One of issue caused by enabling NULL pointer detection is that some PCI
device OptionROM, binary drivers and binary OS boot loaders may have NULL
pointer access bugs, which will prevent BIOS from booting and is almost
impossible to fix. BIT7 of PCD PcdNullPointerDetectionPropertyMask is used
as a workaround to indicate BIOS to disable NULL pointer detection right
after event gEfiEndOfDxeEventGroupGuid, and then let boot continue.
Cc: Star Zeng <star.zeng@intel.com>
Cc: Eric Dong <eric.dong@intel.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Michael Kinney <michael.d.kinney@intel.com>
Cc: Ayellet Wolman <ayellet.wolman@intel.com>
Suggested-by: Ayellet Wolman <ayellet.wolman@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Jian J Wang <jian.j.wang@intel.com>
Reviewed-by: Star Zeng <star.zeng@intel.com>
Reviewed-by: Jiewen Yao <jiewen.yao@intel.com>
NULL pointer detection is done by making use of paging mechanism of CPU.
During page table setup, if enabled, the first 4-K page (0-4095) will be
marked as NOT PRESENT. Any code which unintentionally access memory between
0-4095 will trigger a Page Fault exception which warns users that there's
potential illegal code in BIOS.
This also means that legacy code which has to access memory between 0-4095
should be cautious to temporarily disable this feature before the access
and re-enable it afterwards; or disalbe this feature at all.
Cc: Star Zeng <star.zeng@intel.com>
Cc: Eric Dong <eric.dong@intel.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Michael Kinney <michael.d.kinney@intel.com>
Cc: Ayellet Wolman <ayellet.wolman@intel.com>
Suggested-by: Ayellet Wolman <ayellet.wolman@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Jian J Wang <jian.j.wang@intel.com>
Reviewed-by: Star Zeng <star.zeng@intel.com>
Reviewed-by: Jiewen Yao <jiewen.yao@intel.com>
PCD PcdNullPointerDetectionPropertyMask is a bitmask used to control the
NULL address detection functionality in code for different phases.
If enabled, accessing NULL address in UEFI or SMM code can be caught
as a page fault exception.
BIT0 - Enable NULL pointer detection for UEFI.
BIT1 - Enable NULL pointer detection for SMM.
BIT2..6 - Reserved for future uses.
BIT7 - Disable NULL pointer detection just after EndOfDxe. This is a
workaround for those unsolvable NULL access issues in
OptionROM, boot loader, etc. It can also help to avoid
unnecessary exception caused by legacy memory (0-4095) access
after EndOfDxe, such as Windows 7 boot on Qemu.
Cc: Star Zeng <star.zeng@intel.com>
Cc: Eric Dong <eric.dong@intel.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Michael Kinney <michael.d.kinney@intel.com>
Cc: Ayellet Wolman <ayellet.wolman@intel.com>
Suggested-by: Ayellet Wolman <ayellet.wolman@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Jian J Wang <jian.j.wang@intel.com>
Reviewed-by: Star Zeng <star.zeng@intel.com>
Reviewed-by: Jiewen Yao <jiewen.yao@intel.com>
FV3 HOB was introduced by new (>= 1.5) PI spec, it is intended to
be used to propagate PEI-phase FV authentication status to DXE.
This patch is to update PeiCore to build FV3 HOB with the
authentication status and DxeCore to get the authentication
status from FV3 HOB when producing FVB Protocol.
Cc: Liming Gao <liming.gao@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Star Zeng <star.zeng@intel.com>
Reviewed-by: Liming Gao <liming.gao@intel.com>
According to the TCG Platform Reset Attack Mitigation Specification (May
15, 2008):
> 5 Interface for UEFI
> 5.1 UEFI Variable
> 5.1.1 The MemoryOverwriteRequestControl
>
> Start of informative comment:
>
> [...] The OS loader should not create the variable. Rather, the firmware
> is required to create it and must support the semantics described here.
>
> End of informative comment.
However, some OS kernels create the MOR variable even if the platform
firmware does not support it (see one Bugzilla reference below). This OS
issue breaks the logic added in the last patch.
Strengthen the MOR check by searching for the TCG or TCG2 protocols, as
edk2's implementation of MOR depends on (one of) those protocols.
The protocols are defined under MdePkg, thus there's no inter-package
dependency issue. In addition, calling UEFI services in
MorLockInitAtEndOfDxe() is safe, due to the following order of events /
actions:
- platform BDS signals the EndOfDxe event group,
- the SMM core installs the SmmEndOfDxe protocol,
- MorLockInitAtEndOfDxe() is invoked, and it calls UEFI services,
- some time later, platform BDS installs the DxeSmmReadyToLock protocol,
- SMM / SMRAM is locked down and UEFI services become unavailable to SMM
drivers.
Cc: Eric Dong <eric.dong@intel.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Ladi Prosek <lprosek@redhat.com>
Cc: Star Zeng <star.zeng@intel.com>
Ref: https://bugzilla.redhat.com/show_bug.cgi?id=1498159
Suggested-by: Jiewen Yao <jiewen.yao@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Jiewen Yao <jiewen.yao@intel.com>
Tested-by: Ladi Prosek <lprosek@redhat.com>
The "MemoryOverwriteRequestControl" (a.k.a. MOR) variable comes from the
"TCG Platform Reset Attack Mitigation Specification":
https://www.trustedcomputinggroup.org/wp-content/uploads/Platform-Reset-Attack-Mitigation-Specification.pdf
The "MemoryOverwriteRequestControlLock" variable (a.k.a. MORL) is a
Microsoft extension, called "Secure MOR implementation":
https://docs.microsoft.com/en-us/windows-hardware/drivers/bringup/device-guard-requirements
Currently the VariableSmm driver creates MORL without regard to MOR. This
can lead to a situation where a platform does not support MOR from the
prerequisite spec (because it does not include the
"SecurityPkg/Tcg/MemoryOverwriteControl/TcgMor.inf" driver), but appears
to support MORL from the dependent Microsoft spec.
"winload.efi" notices this inconsistency, and disables the Device Guard
Virtualization Based Security in Windows Server 2016 and Windows 10 64-bit
Enterprise.
If the platform includes
"SecurityPkg/Tcg/MemoryOverwriteControl/TcgMor.inf", then MOR will exist
by the time EndOfDxe is reached, and VariableSmm can safely create MORL.
Otherwise, do not create MORL (delete it if it exists), and also prevent
other modules from creating it.
Cc: Eric Dong <eric.dong@intel.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Ladi Prosek <lprosek@redhat.com>
Cc: Star Zeng <star.zeng@intel.com>
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=727
Ref: https://bugzilla.redhat.com/show_bug.cgi?id=1496170
Reported-by: Ladi Prosek <lprosek@redhat.com>
Suggested-by: Jiewen Yao <jiewen.yao@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Jiewen Yao <jiewen.yao@intel.com>
Tested-by: Ladi Prosek <lprosek@redhat.com>
The SetMorLockVariable() function sets "mMorLockPassThru" to TRUE
temporarily, so that it can set the MOR Control Lock variable to
well-formed values without permission checks.
In the next patch, we'll need the same override for deleting the MOR
Control Lock variable; hence obey "mMorLockPassThru" in the deletion
branch of SetVariableCheckHandlerMorLock() as well.
Cc: Eric Dong <eric.dong@intel.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Ladi Prosek <lprosek@redhat.com>
Cc: Star Zeng <star.zeng@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Jiewen Yao <jiewen.yao@intel.com>
Tested-by: Ladi Prosek <lprosek@redhat.com>
Introduce the MorLockInitAtEndOfDxe() hook, in order to allow
MorLockInit() to delay / queue operations until EndOfDxe. (Or, if the
platform never signals EndOfDxe, until ReadyToBoot.)
Call MorLockInitAtEndOfDxe() whenever we set "mEndOfDxe" to TRUE:
- in VariableRuntimeDxe:
- in the OnReadyToBoot() function,
- in the OnEndOfDxe() function;
- in VariableSmm:
- on the SMM_VARIABLE_FUNCTION_READY_TO_BOOT SMI request,
- in the SmmEndOfDxeCallback() function.
For now, implement MorLockInitAtEndOfDxe() as a no-op in both
VariableRuntimeDxe and VariableSmm.
Cc: Eric Dong <eric.dong@intel.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Ladi Prosek <lprosek@redhat.com>
Cc: Star Zeng <star.zeng@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Jiewen Yao <jiewen.yao@intel.com>
Tested-by: Ladi Prosek <lprosek@redhat.com>
The MorLockInit() and SetVariableCheckHandlerMor() functions have separate
implementations for VariableRuntimeDxe (= unprivileged, unified
DXE_RUNTIME driver) and VariableSmm (= privileged, DXE_SMM back-end of the
split variable driver).
Move their declarations from "Variable.c" to "PrivilegePolymorphic.h", so
that the compiler enforce that the declarations and the definitions match.
(All C source files with the call sites and the function definitions
already include "PrivilegePolymorphic.h" via "Variable.h".)
At the same time:
- replace two typos in the MorLockInit() description:
- replace "EFI_SUCEESS" with "EFI_SUCCESS",
- replace "MOR Lock Control" with "MOR Control Lock";
- in the SetVariableCheckHandlerMor() description:
- replace @param with @param[in],
- correct the description of the Attributes parameter (suggested by Star
Zeng <star.zeng@intel.com>),
- rewrap the comment to 80 columns.
This change cleans up commit 2f6aa774fe ("MdeModulePkg: Add MorLock to
variable driver.", 2016-01-19).
Cc: Eric Dong <eric.dong@intel.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Ladi Prosek <lprosek@redhat.com>
Cc: Star Zeng <star.zeng@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Jiewen Yao <jiewen.yao@intel.com>
Tested-by: Ladi Prosek <lprosek@redhat.com>
If the platform supports SMM, a gRT->SetVariable() call at boot time
results in the following call tree to SecureBootHook():
RuntimeServiceSetVariable() [VariableSmmRuntimeDxe.c, unprivileged]
SmmVariableHandler() [VariableSmm.c, PRIVILEGED]
VariableServiceSetVariable() [Variable.c, PRIVILEGED]
SecureBootHook() [VariableSmm.c, PRIVILEGED]
//
// do nothing
//
SecureBootHook() [Measurement.c, unprivileged]
//
// measure variable if it
// is related to SB policy
//
And if the platform does not support SMM:
VariableServiceSetVariable() [Variable.c, unprivileged]
SecureBootHook() [Measurement.c, unprivileged]
//
// measure variable if it
// is related to SB policy
//
In other words, the measurement always happens outside of SMM.
Because there are two implementations of the SecureBootHook() API, one
that is called from SMM and does nothing, and another that is called
outside of SMM and measures variables, the function declaration should be
in a header file. This way the compiler can enforce that the function
declaration and all function definitions match.
"Variable.h" is used for "including common header files, defining internal
structures and functions used by Variable modules". Technically, we could
declare SecureBootHook() in "Variable.h". However, "Measurement.c" and
"VariableSmmRuntimeDxe.c" themselves do not include "Variable.h", and that
is likely intentional -- "Variable.h" exposes so much of the privileged
variable implementation that it is likely excluded from these C source
files on purpose.
Therefore introduce a new header file called "PrivilegePolymorphic.h".
"Variable.h" includes this header (so that all C source files that have
been allowed to see the variable internals learn about the new
SecureBootHook() declaration immediately). In "Measurement.c" and
"VariableSmmRuntimeDxe.c", include *only* the new header.
This change cleans up commit fa0737a839 ("MdeModulePkg Variable: Merge
from Auth Variable driver in SecurityPkg", 2015-07-01).
Cc: Eric Dong <eric.dong@intel.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Ladi Prosek <lprosek@redhat.com>
Cc: Star Zeng <star.zeng@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Jiewen Yao <jiewen.yao@intel.com>
Tested-by: Ladi Prosek <lprosek@redhat.com>
EFI_BOOT_SCRIPT_WRITE() interface is a var-arg interface.
Spec defines the order of parameters for
EFI_BOOT_SCRIPT_PCI_CONFIG2_WRITE_OPCODE as below:
typedef
EFI_STATUS
(EFIAPI *EFI_BOOT_SCRIPT_WRITE) (
IN CONST EFI_S3_SAVE_STATE_PROTOCOL *This,
IN UINT16 OpCode,
IN EFI_BOOT_SCRIPT_WIDTH Width,
IN UINT16 Segment,
IN UINT64 Address,
IN UINTN Count,
IN VOID *Buffer
);
But implementation assumes Segment is in the very end, after Buffer.
Similar spec/implementation gaps are also found for
EFI_BOOT_SCRIPT_PCI_CONFIG2_READ_WRITE_OPCODE.
The patch fixes the implementation to extract the arguments in
correct order.
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ruiyu Ni <ruiyu.ni@intel.com>
Reviewed-by: Star Zeng <star.zeng@intel.com>
Commit 8932679df5 adds an ASSERT for
checking NULL pointer dereference.
The ASSERT added here is for addressing a false positive NULL pointer
dereference issue raised from static analysis.
This commit adds comments to clarify the reason for using ASSERT as the
check.
Cc: Star Zeng <star.zeng@intel.com>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Hao Wu <hao.a.wu@intel.com>
Reviewed-by: Jiewen Yao <jiewen.yao@intel.com>
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=720
The current implementation assumes there is only one hotplug resource
padding for each resource type. It's not true considering
DegradeResource(): MEM64 resource could be degraded to MEM32
resource.
The patch treat the resource paddings using the same logic as
treating typical/actual resources and the total resource of a bridge
is set to the MAX of typical/actual resources and resource paddings.
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ruiyu Ni <ruiyu.ni@intel.com>
Tested-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Star Zeng <star.zeng@intel.com>
The debug messages can help developer to know the pre-memory
allocation usage.
Cc: Liming Gao <liming.gao@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Star Zeng <star.zeng@intel.com>
Reviewed-by: Liming Gao <liming.gao@intel.com>
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=699
Within function AhciModeInitialization(), left shift operations of 'BIT0'
in the following statements:
"if ((PortImplementBitMap & (BIT0 << Port)) != 0) {"
will incur possible out of range left shift when Port is 31, since
"1 << 31" is possible to exceed the range of type 'int' (signed).
According to the C11 spec, Section 6.5.7:
> 4 The result of E1 << E2 is E1 left-shifted E2 bit positions; vacated
> bits are filled with zeros. If E1 has an unsigned type, the value
> of the result is E1 * 2^E2 , reduced modulo one more than the
> maximum value representable in the result type. If E1 has a signed
> type and nonnegative value, and E1 * 2^E2 is representable in the
> result type, then that is the resulting value; otherwise, the
> behavior is undefined.
This commit explicitly cast 'BIT0' with UINT32 to resolve this issue.
Cc: Steven Shi <steven.shi@intel.com>
Cc: Eric Dong <eric.dong@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Hao Wu <hao.a.wu@intel.com>
Reviewed-by: Star Zeng <star.zeng@intel.com>
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=698
Within function NetRandomInitSeed(), left shift a negative value is used
in:
"~Time.Hour << 24"
which involves undefined behavior.
Since Time.Hour is of type UINT8 (range from 0 to 23), hence ~Time.Hour
will be a negative value (of type int, signed).
According to the C11 spec, Section 6.5.7:
> 4 The result of E1 << E2 is E1 left-shifted E2 bit positions; vacated
> bits are filled with zeros. If E1 has an unsigned type, the value
> of the result is E1 * 2^E2 , reduced modulo one more than the
> maximum value representable in the result type. If E1 has a signed
> type and nonnegative value, and E1 * 2^E2 is representable in the
> result type, then that is the resulting value; otherwise, the
> behavior is undefined.
This commit will remove the '~' operator before 'Time.Hour', since it
seems like an implementation choice for generating the seed.
Cc: Steven Shi <steven.shi@intel.com>
Cc: Qin Long <qin.long@intel.com>
Cc: Star Zeng <star.zeng@intel.com>
Cc: Eric Dong <eric.dong@intel.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Hao Wu <hao.a.wu@intel.com>
Reviewed-by: Wu Jiaxin <jiaxin.wu@intel.com>
Reviewed-by: Fu Siyuan <siyuan.fu@intel.com>
Reviewed-by: Ye Ting <ting.ye@intel.com>
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=695
Within function CoreRestoreTpl(), left shift a negative value -2 is used
in:
"while (((-2 << NewTpl) & gEventPending) != 0) {"
which involves undefined behavior.
According to the C11 spec, Section 6.5.7:
> 4 The result of E1 << E2 is E1 left-shifted E2 bit positions; vacated
> bits are filled with zeros. If E1 has an unsigned type, the value
> of the result is E1 * 2^E2 , reduced modulo one more than the
> maximum value representable in the result type. If E1 has a signed
> type and nonnegative value, and E1 * 2^E2 is representable in the
> result type, then that is the resulting value; otherwise, the
> behavior is undefined.
This commit refines the code logic to avoid left shifting the negative
value.
Cc: Steven Shi <steven.shi@intel.com>
Cc: Eric Dong <eric.dong@intel.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Michael Kinney <michael.d.kinney@intel.com>
Cc: Liming Gao <liming.gao@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Hao Wu <hao.a.wu@intel.com>
Reviewed-by: Star Zeng <star.zeng@intel.com>
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=702
Within function InternalPrintLibSPrintMarker(), possible left shift of a
negative value is found in:
"(*(ArgumentString + 1) << 8)"
which involves undefined behavior.
Since '*(ArgumentString + 1)' is of type CONST CHAR8 (signed), it will be
promoted to type int (signed) during the left shift operation. If
'*(ArgumentString + 1)' is a negative value, the behavior will be
undefined.
According to the C11 spec, Section 6.5.7:
> 4 The result of E1 << E2 is E1 left-shifted E2 bit positions; vacated
> bits are filled with zeros. If E1 has an unsigned type, the value
> of the result is E1 * 2^E2 , reduced modulo one more than the
> maximum value representable in the result type. If E1 has a signed
> type and nonnegative value, and E1 * 2^E2 is representable in the
> result type, then that is the resulting value; otherwise, the
> behavior is undefined.
This commit explicitly cast '*(ArgumentString + 1)' with UINT8 to resolve
this issue.
Cc: Steven Shi <steven.shi@intel.com>
Cc: Michael Kinney <michael.d.kinney@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Hao Wu <hao.a.wu@intel.com>
Reviewed-by: Liming Gao <liming.gao@intel.com>
Current implementation deletes the "BootNext" before calling
any PlatformBootManagerLib APIs, but if system resets in
PlatformBootManagerLib APIs, "BootNext" is not consumed but lost.
The patch defers the deletion of "BootNext" to before booting it.
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ruiyu Ni <ruiyu.ni@intel.com>
Cc: Eric Dong <eric.dong@intel.com>
Reviewed-by: Star Zeng <star.zeng@intel.com>
Reviewed-by: Sunny Wang <sunnywang@hpe.com>
In NetbufTrim() function, the NetBuf TotalSize should be checked with 0 before
making the trim operation, otherwise the function will fall into infinite loop.
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Fu Siyuan <siyuan.fu@intel.com>
Reviewed-by: Wu Jiaxin <jiaxin.wu@intel.com>
Fu Siyuan
Star Zeng
Jian J Wang
Heyi Guo
Ruiyu Ni
Dandan Bi
Ard Biesheuvel
Liming Gao
Zhang, Chao B
Laszlo Ersek
Jiaxin Wu
Eric Dong
Hao Wu