Commit Graph

31482 Commits

Author SHA1 Message Date
Chris Johnson caa389625f .pytool/CISettings.py: Add subhook submodule
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4389

Add submodule for https://github.com/Zeex/subhook

This submodule is required to hook internal functions
when using gmock.

Cc: Sean Brogan <sean.brogan@microsoft.com>
Cc: Michael Kubacki <mikuback@linux.microsoft.com>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Cc: Liming Gao <gaoliming@byosoft.com.cn>
Signed-off-by: Chris Johnson <chris.n.johnson@intel.com>
Reviewed-by: Leif Lindholm <quic_llindhol@quicinc.com>
Reviewed-by: Michael Kubacki <michael.kubacki@microsoft.com>
Reviewed-by: Oliver Smith-Denny <osde@linux.microsoft.com>
Reviewed-by: Michael D Kinney <michael.d.kinney@intel.com>
2023-04-10 05:59:02 +00:00
Chris Johnson 0c6d851d25 UnitTestFrameworkPkg: Add subhook submodule required for gmock
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4389

Add subhook submodule that is required to hook internal functions
when using gmock.

    https://github.com/Zeex/subhook

Add SubhookLib library class and SubhookLib library instance.
Include the SUBHOOK_STATIC define in the SubhookLib INF file so
it builds as a static library. Also include the SUBHOOK_STATIC
define in SubhookLib.h so all modules using SubhookLib properly
link SubhookLib as a static library.

Cc: Andrew Fish <afish@apple.com>
Cc: Leif Lindholm <quic_llindhol@quicinc.com>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Cc: Michael Kubacki <mikuback@linux.microsoft.com>
Cc: Sean Brogan <sean.brogan@microsoft.com>
Signed-off-by: Chris Johnson <chris.n.johnson@intel.com>
Reviewed-by: Leif Lindholm <quic_llindhol@quicinc.com>
Reviewed-by: Michael Kubacki <michael.kubacki@microsoft.com>
Reviewed-by: Oliver Smith-Denny <osde@linux.microsoft.com>
Reviewed-by: Michael D Kinney <michael.d.kinney@intel.com>
2023-04-10 05:59:02 +00:00
Ard Biesheuvel 6405cd0304 MdeModulePkg: Enable forward edge CFI in mem attributes table
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4405

The memory attributes table has been extended with a flag that indicates
whether or not the OS is permitted to map the EFI runtime code regions
with strict enforcement for IBT/BTI landing pad instructions.

Given that the PE/COFF spec now defines a DllCharacteristicsEx flag that
indicates whether or not a loaded image is compatible with this, we can
wire this up to the flag in the memory attributes table, and set it if
all loaded runtime image are compatible with it.

Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
Reviewed-by: Leif Lindholm <quic_llindhol@quicinc.com>
Reviewed-by: Oliver Smith-Denny <osde@linux.microsoft.com>
Reviewed-by: Michael Kubacki <michael.kubacki@microsoft.com>
Reviewed-by: Liming Gao <gaoliming@byosoft.com.cn>
2023-04-07 13:18:38 +00:00
Ard Biesheuvel d6457b3090 MdePkg/PeCoffLib: Capture DLL characteristics fields in image context
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4405

When loading a PE/COFF image, capture the DLL characteristics fields of
the header into our image context structure so we can refer to them when
mapping the image.

Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
Reviewed-by: Leif Lindholm <quic_llindhol@quicinc.com>
Reviewed-by: Oliver Smith-Denny <osde@linux.microsoft.com>
Reviewed-by: Michael Kubacki <michael.kubacki@microsoft.com>
Reviewed-by: Liming Gao <gaoliming@byosoft.com.cn>
2023-04-07 13:18:38 +00:00
Ard Biesheuvel b62d7ac97b BaseTools/GenFw: Add DllCharacteristicsEx field to debug data
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4405

The PE/COFF spec describes an additional DllCharacteristics field
implemented as a debug directory entry, which carries flags related to
which control flow integrity (CFI) features are supported by the binary.

So let's add this entry when doing ELF to PE/COFF conversion - we will
add support for setting the flags in a subsequent patch.

Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
Reviewed-by: Leif Lindholm <quic_llindhol@quicinc.com>
Reviewed-by: Oliver Smith-Denny <osde@linux.microsoft.com>
Reviewed-by: Michael Kubacki <michael.kubacki@microsoft.com>
Reviewed-by: Liming Gao <gaoliming@byosoft.com.cn>
2023-04-07 13:18:38 +00:00
Ard Biesheuvel 6c299acf48 BaseTools/GenFw: Parse IBT/BTI support status from ELF note
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4405

When performing ELF to PE/COFF conversion, parse any notes sections to
decide whether the image supports forward CFI landing pads. This will be
used to set the associated DllCharacteristicsEx flag in a subsequent
patch.

Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
Reviewed-by: Leif Lindholm <quic_llindhol@quicinc.com>
Reviewed-by: Oliver Smith-Denny <osde@linux.microsoft.com>
Acked-by: Michael Kubacki <michael.kubacki@microsoft.com>
Reviewed-by: Liming Gao <gaoliming@byosoft.com.cn>
2023-04-07 13:18:38 +00:00
KasimX Liu cdf6ff1719 UefiPayloadPkg:Add new build commands for UniversalPayload
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4375

add '-pb' command of build bios with exist PreUniversalPayload file.
    '-e'  command of olny build the UniversalPayloadentry file.

Cc: Guo Dong <guo.dong@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Reviewed-by: James Lu <james.lu@intel.com>
Reviewed-by: Gua Guo <gua.guo@intel.com>
Signed-off-by: KasimX Liu <kasimx.liu@intel.com>
2023-04-07 03:47:30 +00:00
Ard Biesheuvel 3e3be2cbc2 UefiCpuPkg/CpuExceptionHandlerLib: Drop special XCODE5 version
This library is no longer used or needed, so let's remove it.

Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
Reviewed-by: Ray Ni <ray.ni@intel.com>
2023-04-06 06:52:55 +00:00
Ard Biesheuvel 68c89b987f OvmfPkg: Drop special Xcode5 version of exception handler library
The generic and XCODE5 versions of this library are now identical, so
drop the special case. The library will be removed entirely in a
subsequent patch.

Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
Acked-by: Jiewen Yao <jiewen.yao@intel.com>
2023-04-06 06:52:55 +00:00
Ard Biesheuvel e5fcaeb133 UefiCpuPkg/CpuExceptionHandlerLib: Make runtime fixups XCODE-only
The CPU exception handler library code was rewritten at some point to
populate the vector code templates with absolute references at runtime,
given that the XCODE linker does not permit absolute references in
executable code when creating PIE executables.

This is rather unfortunate, as this prevents us from using strict
permissions on the memory mappings, given that the .text section needs
to be writable at runtime for this arrangement to work.

So let's make this hack XCODE-only, by setting a preprocessor #define
from the command line when using the XCODE toolchain, and only including
the runtime fixup code when the macro is defined.

While at it, rename the Xcode5ExceptionHandlerAsm.nasm source file and
drop the Xcode5 prefix: this code is used by other toolchains too.

Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
Reviewed-by: Ray Ni <ray.ni@intel.com>
2023-04-06 06:52:55 +00:00
Ard Biesheuvel 4764aa50ef UefiCpuPkg/PeiCpuExceptionHandlerLib: Use SEC/PEI specific asm component
The PEI flavor of CpuExceptionHandlerLib never populates more than 32
IDT vectors, and there is no CET shadow stack support in the PEI phase.
So there is no need to use the generic ExceptionHandler NASM source,
which carries a 256-entry template and CET support, and writes to its
own .text section when built using XCODE, which is not permitted in the
PEI phase. So let's switch to the reduced SEC/PEI version of this
component, which is sufficient for PEI and doesn't suffer from the same
issue.

Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
Reviewed-by: Ray Ni <ray.ni@intel.com>
2023-04-06 06:52:55 +00:00
Ard Biesheuvel 95f0330953 UefiCpuPkg/CpuExceptionHandlerLib: Use single SEC/PEI version
Currently, we use the non-Xcode5 version of ExceptionHandlerAsm.nasm
only for the SEC and PEI phases, and this version was not compatible
with the XCODE or LLD linkers, which do not permit absolute relocations
in read-only sections.

Given that SEC and PEI code typically executes in place from flash and
does not use page alignment for sections, we can simply emit the code
carrying the absolute symbol references into the .data segment instead.
This works around the linker's objections, and the resulting image will
be mapped executable in its entirety anyway. Since this is only needed
for XCODE, let's make this change conditionally using a preprocessor
macro.

Let's rename the .nasm file to reflect the fact that is used for the
SecPei flavor of this library only, and while at it, remove some
unnecessary absolute references.

Also update the Xcode specific version of this library, and use this
source file instead. This is necesessary, as the Xcode specific version
modifies its own code at runtime, which is not permitted in SEC or PEI.
Note that this also removes CET support from the Xcode5 specific build
of the SEC/PEI version of this library, but this is not needed this
early in any case, and this aligns it with other toolchains, which use
this version of the library, which does not have CET support either.

1. Change for non-XCODE SecPeiCpuExceptionHandlerLib:
. Use SecPeiExceptionHandlerAsm.nasm (renamed from
  ExceptionHandlerAsm.nasm)
. Removed some unnecessary absolute references
  (32 IDT stubs are still in .text.)

2. Change for XCODE SecPeiCpuExceptionHandlerLib:
. Use SecPeiExceptionHandlerAsm.nasm instead of
  Xcode5ExceptionHandlerAsm.nasm
. CET logic is not in SecPeiExceptionHandlerAsm.nasm (but aligns to
  non-XCODE lib instance)
. Fixed a bug that does runtime fixup in TEXT section in SPI flash.
. Emitted the code carrying the absolute symbol references into the
  .data which XCODE or LLD linkers allow.
. Then fixup can be done by other build tools such as GenFv if the code
  runs in SPI flash, or by PE coff loader if the code is loaded to
  memory.

Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
Reviewed-by: Ray Ni <ray.ni@intel.com>
2023-04-06 06:52:55 +00:00
Ard Biesheuvel a257988f59 BaseTools/tools_def CLANGDWARF: Permit text relocations
We rely on PIE executables to get the codegen that is suitable for
PE/COFF conversion where the resulting executables can be loaded
anywhere in the address space.

However, ELF linkers may default to disallowing text relocations in PIE
executables, as this would require text segments to be updated at
runtime, which is bad for security and increases the copy-on-write
footprint of ELF executables and shared libraries.

However, none of those concerns apply to PE/COFF executables in the
context of EFI, which are copied into memory rather than mmap()'ed, and
fixed up by the loader before launch.

So pass -z notext to the LLD linker to permit runtime relocations in
read-only sections.

Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
Reviewed-by: Rebecca Cran <rebecca@bsdio.com>
2023-04-06 06:52:55 +00:00
Rebecca Cran 8d185dfb66 BaseTools: Update antlr makefile to use cc by default
Update the antlr makefile to remove the explicit setting of CC to either
clang or gcc. This causes it to use /usr/bin/cc or whatever the user has
set $(CC) to.

This removes the last dependency on gcc for BaseTools.

Signed-off-by: Rebecca Cran <rebecca@bsdio.com>
Reviewed-by: Liming Gao <gaoliming@byosoft.com.cn>
2023-04-06 01:32:09 +00:00
Rebecca Cran 2bb6938949 BaseTools/Conf/tools_def: Fix linking using CLANGDWARF_IA32
The clang toolchain might default to fPIE/fPIC, which prevents
lld from linking the objects into a binary.

Specify -fno-pie -fno-pic as done on GCC to fix linking.

Test:
Building the Universal Payload using the command
'python UefiPayloadPkg/UniversalPayloadBuild.py -a IA32' actually
works.

Signed-off-by: Patrick Rudolph <patrick.rudolph@9elements.com>
Reviewed-by: Rebecca Cran <rebecca@bsdio.com>
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=4356
2023-04-05 20:39:36 +00:00
Rebecca Cran a56ee36c49 BaseTools: Build against C++14 when building with clang
clang 17 defaults to C++17, where the 'register' keyword is deprecated
and the warning changed to an error. To avoid build errors, compile
against C++14 by specifying '-std=c++14' in CXXFLAGS.

Signed-off-by: Rebecca Cran <rebecca@quicinc.com>
Reviewed-by: Liming Gao <gaoliming@byosoft.com.cn>
2023-04-05 15:44:48 +00:00
Rebecca Cran 728ff1da33 BaseTools: Allow users to build with clang using CC=clang CXX=clang++
In https://bugzilla.tianocore.org/show_bug.cgi?id=2842 clang support was
added by having users specify "make CXX=llvm" when building BaseTools.

The Makefile then sees that and sets CC=$(CLANG_BIN)clang and
CXX=$(CLANG_BIN)clang++. That requires that the executables 'clang' and
'clang++' exist and for example aren't named 'clang-17' and
'clang++-17'. Also, it's an unusual way of specifying the compiler,
since many users will expect to be able to override CC and CXX on the
make command line.

Rework the BaseTools Makefiles removing the 'BUILD_' prefix (BUILD_CC
and BUILD_CXX) and using the standard name 'LDFLAGS' instead of
'LFLAGS'. This allows clang to be used by running
'make -C BaseTools CC=clang CXX=clang++'.

Signed-off-by: Rebecca Cran <rebecca@quicinc.com>
Reviewed-by: Liming Gao <gaoliming@byosoft.com.cn>
2023-04-05 15:44:48 +00:00
Rebecca Cran 206168e83f BaseTools: Allow users to specify compiler to use with make CC= CXX=
In https://bugzilla.tianocore.org/show_bug.cgi?id=2842 clang support was
added by having users specify "make CXX=llvm" when building BaseTools.

The Makefile then sees that and sets CC=$(CLANG_BIN)clang and
CXX=$(CLANG_BIN)clang++. That requires that the executables 'clang' and
'clang++' exist and for example aren't named 'clang-17' and
'clang++-17'. Also, it's an unusual way of specifying the compiler,
since many users will expect to be able to override CC and CXX on the
make command line.

Rework the BaseTools Makefiles removing the 'BUILD_' prefix (BUILD_CC
and BUILD_CXX) and using the standard name 'LDFLAGS' instead of
'LFLAGS'. This allows clang to be used by running
'make -C BaseTools CC=clang CXX=clang++'.

Signed-off-by: Rebecca Cran <rebecca@quicinc.com>
Reviewed-by: Liming Gao <gaoliming@byosoft.com.cn>
2023-04-05 15:44:48 +00:00
Rebecca Cran cdd79996c2 pip-requirements.txt: bump edk2-basetools to 0.1.43
Bump the version of edk2-basetools in pip-requirements.txt to 0.1.43.
This version contains the update to generate makefiles with both
CFLAGS and BUILD_CFLAGS.

Signed-off-by: Rebecca Cran <rebecca@bsdio.com>
Reviewed-by: Michael D Kinney <michael.d.kinney@intel.com>
2023-04-05 09:40:32 +00:00
Chasel Chiu 7df447930c IntelFsp2Pkg: LoadMicrocodeDefault() causing unnecessary delay.
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4391

FSP should support the scenario that CPU microcode already loaded
before calling LoadMicrocodeDefault(), in this case it should return
directly without spending more time.
Also the LoadMicrocodeDefault() should only attempt to load one version
of the microcode for current CPU and return directly without parsing
rest of the microcode in FV.

This patch also removed unnecessary LoadCheck code after supporting
CPU microcode already loaded scenario.

Cc: Nate DeSimone <nathaniel.l.desimone@intel.com>
Cc: Star Zeng <star.zeng@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Signed-off-by: Chasel Chiu <chasel.chiu@intel.com>
Reviewed-by: Ted Kuo <ted.kuo@intel.com>
Reviewed-by: Nate DeSimone <nathaniel.l.desimone@intel.com>
Reviewed-by: Ray Ni <ray.ni@intel.com>
2023-04-04 17:18:20 +00:00
Chasel Chiu af98f1fb03 IntelFsp2Pkg: TempRamInit API should preserve EBX/RBX register.
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4395

FSP specification defines the TempRamInit API preserved register list
which including EBX/RBX, however current implementation unexpectedly
overriding EBX/RBX register that should be fixed.

Cc: Nate DeSimone <nathaniel.l.desimone@intel.com>
Cc: Star Zeng <star.zeng@intel.com>
Signed-off-by: Chasel Chiu <chasel.chiu@intel.com>
Reviewed-by: Nate DeSimone <nathaniel.l.desimone@intel.com>
2023-04-04 16:39:41 +00:00
Michael D Kinney fb89f62d27 .azurepilelines/templates: Split CryptoPkg builds
CryptoPkg has double the build time of all other packages.
Split CryptoPkg up matching the style applied to MdeModulePkg.

Cc: Sean Brogan <sean.brogan@microsoft.com>
Cc: Michael Kubacki <mikuback@linux.microsoft.com>
Cc: Liming Gao <gaoliming@byosoft.com.cn>
Signed-off-by: Michael D Kinney <michael.d.kinney@intel.com>
Reviewed-by: Michael Kubacki <mikuback@linux.microsoft.com>
2023-04-04 15:06:31 +00:00
Michael D Kinney 56e70968e9 .github.workflows: Split MdeModulePkg and CryptoPkg analysis
MdeModulePkg and CryptoPkg have double the CodeQL analysis
time of all other packages.  Split these packages up to perform
separate analysis of IA32 and X64.

Cc: Sean Brogan <sean.brogan@microsoft.com>
Cc: Michael Kubacki <mikuback@linux.microsoft.com>
Signed-off-by: Michael D Kinney <michael.d.kinney@intel.com>
Reviewed-by: Michael Kubacki <mikuback@linux.microsoft.com>
2023-04-04 15:06:31 +00:00
Michael Kubacki 26997800c9 .github/codeql/edk2.qls: Enable CWE 120, 787, and 805 queries
As recommended by CodeQL this change replaces
cpp/potential-buffer-overflow with cpp/overrunning-write-with-float
and cpp/overrunning-write.

Enables:

1. cpp/overrunning-write
   - @name Likely overrunning write
   - @description Buffer write operations that do not control the length
                  data written may overflow
   - @kind problem
   - @problem.severity error
   - @security-severity 9.3
   - @precision high
   - @id cpp/very-likely-overrunning-write
   - @tags reliability
     - security
     - external/cwe/cwe-120
     - external/cwe/cwe-787
     - external/cwe/cwe-805
2. cpp/overrunning-write-with-float
   - @name Potentially overrunning write with float to string conversion
   - @description Buffer write operations that do not control the length
                  of data written may overflow when floating point inputs
                  take extreme values.
   - @kind problem
   - @problem.severity error
   - @security-severity 9.3
   - @precision medium
   - @id cpp/overrunning-write-with-float
   - @tags reliability
     - security
     - external/cwe/cwe-120
     - external/cwe/cwe-787
     - external/cwe/cwe-805
3. cpp/very-likely-overrunning-write
   - @name Likely overrunning write
   - @description Buffer write operations that do not control the length
                  of data written may overflow
   - @kind problem
   - @problem.severity error
   - @security-severity 9.3
   - @precision high
   - @id cpp/very-likely-overrunning-write
   - @tags reliability
     - security
     - external/cwe/cwe-120
     - external/cwe/cwe-787
     - external/cwe/cwe-805

- CWEs:
  - https://cwe.mitre.org/data/definitions/120.html
  - https://cwe.mitre.org/data/definitions/787.html
  - https://cwe.mitre.org/data/definitions/805.html

Cc: Sean Brogan <sean.brogan@microsoft.com>
Cc: Michael Kubacki <mikuback@linux.microsoft.com>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Signed-off-by: Michael Kubacki <michael.kubacki@microsoft.com>
Reviewed-by: Michael D Kinney <michael.d.kinney@intel.com>
Reviewed-by: Oliver Smith-Denny <osd@smith-denny.com>
2023-04-03 15:29:08 +00:00
Michael Kubacki 0734975824 .github/codeql/edk2.qls: Enable CWE 457, 676, and 758 queries
The previous commits fixed issues with these queries across various
packages. Now that those are resolved, enable the queries in the
edk2 query set so regressions can be found in the future.

Enables:

1. cpp/conditionallyuninitializedvariable
   - CWE: https://cwe.mitre.org/data/definitions/457.html
   - @name Conditionally uninitialized variable
   - @description An initialization function is used to initialize a
                  local variable, but the returned status code is
                  not checked. The variable may be left in an
                  uninitialized state, and reading the variable may
                  result in undefined behavior.
   - @kind problem
   - @problem.severity warning
   - @security-severity 7.8
   - @id cpp/conditionally-uninitialized-variable
   - @tags security
     - external/cwe/cwe-457
2. cpp/pointer-overflow-check
   - CWE: https://cwe.mitre.org/data/definitions/758.html
   - @name Pointer overflow check
   - @description Adding a value to a pointer to check if it
                  overflows relies on undefined behavior and
                  may lead to memory corruption.
   - @kind problem
   - @problem.severity error
   - @security-severity 2.1
   - @precision high
   - @id cpp/pointer-overflow-check
   - @tags reliability
     - security
     - external/cwe/cwe-758
3. cpp/potential-buffer-overflow
   - CWE: https://cwe.mitre.org/data/definitions/676.html
   - @name Potential buffer overflow
   - @description Using a library function that does not check
                  buffer bounds requires the surrounding program
                  to be very carefully written to avoid buffer
                  overflows.
   - @kind problem
   - @id cpp/potential-buffer-overflow
   - @problem.severity warning
   - @security-severity 10.0
   - @tags reliability
     - security
     - external/cwe/cwe-676
   - @deprecated This query is deprecated, use
                 Potentially overrunning write
                 (`cpp/overrunning-write`) and
                 Potentially overrunning write with float to string
                 conversion
                 (`cpp/overrunning-write-with-float`) instead.

Note that cpp/potential-buffer-overflow is deprecated. This query
will be updated to the succeeding queries in the next commit. The
query is used in this commit to show that we considered and tested
the query in history.

Cc: Sean Brogan <sean.brogan@microsoft.com>
Cc: Michael Kubacki <mikuback@linux.microsoft.com>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Signed-off-by: Michael Kubacki <michael.kubacki@microsoft.com>
Reviewed-by: Michael D Kinney <michael.d.kinney@intel.com>
Reviewed-by: Oliver Smith-Denny <osd@smith-denny.com>
2023-04-03 15:29:08 +00:00
Michael Kubacki e03657dca8 UefiCpuPkg: Fix conditionally uninitialized variables
Fixes CodeQL alerts for CWE-457:
https://cwe.mitre.org/data/definitions/457.html

Cc: Eric Dong <eric.dong@intel.com>
Cc: Erich McMillan <emcmillan@microsoft.com>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Cc: Michael Kubacki <mikuback@linux.microsoft.com>
Cc: Rahul Kumar <rahul1.kumar@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Co-authored-by: Erich McMillan <emcmillan@microsoft.com>
Signed-off-by: Michael Kubacki <michael.kubacki@microsoft.com>
Reviewed-by: Michael D Kinney <michael.d.kinney@intel.com>
Reviewed-by: Oliver Smith-Denny <osd@smith-denny.com>
2023-04-03 15:29:08 +00:00
Michael Kubacki 11dd44dfbe ShellPkg: Fix conditionally uninitialized variables
Fixes CodeQL alerts for CWE-457:
https://cwe.mitre.org/data/definitions/457.html

Cc: Erich McMillan <emcmillan@microsoft.com>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Cc: Michael Kubacki <mikuback@linux.microsoft.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Zhichao Gao <zhichao.gao@intel.com>
Co-authored-by: Erich McMillan <emcmillan@microsoft.com>
Signed-off-by: Michael Kubacki <michael.kubacki@microsoft.com>
Reviewed-by: Zhichao Gao <zhichao.gao@intel.com>
Reviewed-by: Oliver Smith-Denny <osd@smith-denny.com>
2023-04-03 15:29:08 +00:00
Michael Kubacki 7dc182ed1e PcAtChipsetPkg: Fix conditionally uninitialized variables
Fixes CodeQL alerts for CWE-457:
https://cwe.mitre.org/data/definitions/457.html

Cc: Erich McMillan <emcmillan@microsoft.com>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Cc: Michael Kubacki <mikuback@linux.microsoft.com>
Cc: Ray Ni <ray.ni@intel.com>
Co-authored-by: Erich McMillan <emcmillan@microsoft.com>
Signed-off-by: Michael Kubacki <michael.kubacki@microsoft.com>
Reviewed-by: Michael D Kinney <michael.d.kinney@intel.com>
Reviewed-by: Ray Ni <ray.ni@intel.com>
Reviewed-by: Oliver Smith-Denny <osd@smith-denny.com>
2023-04-03 15:29:08 +00:00
Michael Kubacki 3fab32d41d NetworkPkg: Fix conditionally uninitialized variables
Fixes CodeQL alerts for CWE-457:
https://cwe.mitre.org/data/definitions/457.html

Cc: Erich McMillan <emcmillan@microsoft.com>
Cc: Jiaxin Wu <jiaxin.wu@intel.com>
Cc: Maciej Rabeda <maciej.rabeda@linux.intel.com>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Cc: Michael Kubacki <mikuback@linux.microsoft.com>
Cc: Siyuan Fu <siyuan.fu@intel.com>
Co-authored-by: Erich McMillan <emcmillan@microsoft.com>
Signed-off-by: Michael Kubacki <michael.kubacki@microsoft.com>
Reviewed-by: Michael D Kinney <michael.d.kinney@intel.com>
Reviewed-by: Oliver Smith-Denny <osd@smith-denny.com>
2023-04-03 15:29:08 +00:00
Michael Kubacki 321240b135 MdePkg: Fix conditionally uninitialized variables
Fixes CodeQL alerts for CWE-457:
https://cwe.mitre.org/data/definitions/457.html

Note that this change affects the actual return value from the
following functions. The functions documented that if an integer
overflow occurred, MAX_UINTN would be returned. They were
implemented to actually return an undefined value from the stack.

This change makes the function follow its description. However, this
is technically different than what callers may have previously
expected.

MdePkg/Library/BaseLib/String.c:
  - StrDecimalToUintn()
  - StrDecimalToUint64()
  - StrHexToUintn()
  - StrHexToUint64()
  - AsciiStrDecimalToUintn()
  - AsciiStrDecimalToUint64()
  - AsciiStrHexToUintn()
  - AsciiStrHexToUint64()

Cc: Erich McMillan <emcmillan@microsoft.com>
Cc: Liming Gao <gaoliming@byosoft.com.cn>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Cc: Michael Kubacki <mikuback@linux.microsoft.com>
Cc: Zhiguang Liu <zhiguang.liu@intel.com>
Co-authored-by: Erich McMillan <emcmillan@microsoft.com>
Signed-off-by: Michael Kubacki <michael.kubacki@microsoft.com>
Reviewed-by: Liming Gao <gaoliming@byosoft.com.cn>
Reviewed-by: Oliver Smith-Denny <osd@smith-denny.com>
2023-04-03 15:29:08 +00:00
Michael Kubacki 07251f3c6a MdeModulePkg: Fix conditionally uninitialized variables
Fixes CodeQL alerts for CWE-457:
https://cwe.mitre.org/data/definitions/457.html

Cc: Dandan Bi <dandan.bi@intel.com>
Cc: Eric Dong <eric.dong@intel.com>
Cc: Erich McMillan <emcmillan@microsoft.com>
Cc: Guomin Jiang <guomin.jiang@intel.com>
Cc: Jian J Wang <jian.j.wang@intel.com>
Cc: Liming Gao <gaoliming@byosoft.com.cn>
Cc: Michael Kubacki <mikuback@linux.microsoft.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Zhichao Gao <zhichao.gao@intel.com>
Co-authored-by: Erich McMillan <emcmillan@microsoft.com>
Signed-off-by: Michael Kubacki <michael.kubacki@microsoft.com>
Reviewed-by: Liming Gao <gaoliming@byosoft.com.cn>
Reviewed-by: Oliver Smith-Denny <osd@smith-denny.com>
2023-04-03 15:29:08 +00:00
Michael Kubacki 84d77d9bf5 CryptoPkg: Fix conditionally uninitialized variable
Fixes CodeQL alerts for CWE-457:
https://cwe.mitre.org/data/definitions/457.html

Checks the return value from `ASN1_get_object()` to verify values
set by the function are valid.

Note that the function returns literal `0x80`:
    `return (0x80);`

That is used to check the return value is as the case in other areas
of the code.

Cc: Erich McMillan <emcmillan@microsoft.com>
Cc: Guomin Jiang <guomin.jiang@intel.com>
Cc: Jian J Wang <jian.j.wang@intel.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Michael Kubacki <mikuback@linux.microsoft.com>
Cc: Xiaoyu Lu <xiaoyu1.lu@intel.com>
Co-authored-by: Erich McMillan <emcmillan@microsoft.com>
Signed-off-by: Michael Kubacki <michael.kubacki@microsoft.com>
Reviewed-by: Jiewen Yao <Jiewen.yao@intel.com>
Reviewed-by: Oliver Smith-Denny <osd@smith-denny.com>
2023-04-03 15:29:08 +00:00
Michael Kubacki dbe820d5fa BaseTools/VfrCompile: Fix potential buffer overwrites
While more portable methods exist to handle these cases, this change
does not attempt to do more than fix the immediate problem and
follow the conventions already established in this code.

`snprintf()` is introduced as the minimum improvement apart from
making the buffers larger.

Fixes the following CodeQL alerts:

1. Failure on line 2339 in
   BaseTools/Source/C/VfrCompile/Pccts/antlr/gen.c

   - Type: Potentially overrunning write
   - Severity: Critical
   - Problem: This 'call to sprintf' operation requires 17 bytes but
     the destination is only 16 bytes.

2. Failure on line 2341 in
   BaseTools/Source/C/VfrCompile/Pccts/antlr/gen.c

   - Type: Potentially overrunning write
   - Severity: Critical
   - Problem: This 'call to sprintf' operation requires 17 bytes but
     the destination is only 16 bytes.

3. Failure on line 1309 in
   BaseTools/Source/C/VfrCompile/Pccts/antlr/main.c

   - Type: Potentially overrunning write
   - Severity: Critical
   - Problem: This 'call to sprintf' operation requires 25 bytes but
     the destination is only 20 bytes.

Cc: Bob Feng <bob.c.feng@intel.com>
Cc: Liming Gao <gaoliming@byosoft.com.cn>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Cc: Sean Brogan <sean.brogan@microsoft.com>
Cc: Yuwei Chen <yuwei.chen@intel.com>
Signed-off-by: Michael Kubacki <michael.kubacki@microsoft.com>
Reviewed-by: Liming Gao <gaoliming@byosoft.com.cn>
Reviewed-by: Michael D Kinney <michael.d.kinney@intel.com>
Reviewed-by: Oliver Smith-Denny <osd@smith-denny.com>
2023-04-03 15:29:08 +00:00
Michael Kubacki 4693b325e8 BaseTools/PatchCheck.py: Add PCCTS to tab exemption list
Purdue Compiler Construction Tool Set (PCCTS) source code was copied/
pasted into BaseTools/Source/C/VfrCompile/Pccts/.

The code contains tab characters instead of spaces.

PatchCheck.py gives an error on modifications to files that
contain tabs.

The goal of my upcoming change there is not to mix tabs and spaces
but to fix a bug while preserving its current formatting characters.

This change adds that directory to the pre-existing list of
directories in which tab checks are ignored in PatchCheck.py
and also updates the check for makefiles to check for *.makefile:
this allows {header,footer,app,lib}.makefile in
BaseTools/Source/C/Makefiles to be detected and avoid having
PatchCheck.py complain about tab characters.

The check for "Makefile" is updated to be case-insensitive since
there are some Makefiles named 'makefile' instead of 'Makefile'.

Co-authored-by: Rebecca Cran <rebecca@bsdio.com>
Cc: Bob Feng <bob.c.feng@intel.com>
Cc: Liming Gao <gaoliming@byosoft.com.cn>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Cc: Sean Brogan <sean.brogan@microsoft.com>
Cc: Yuwei Chen <yuwei.chen@intel.com>
Signed-off-by: Michael Kubacki <michael.kubacki@microsoft.com>
Reviewed-by: Liming Gao <gaoliming@byosoft.com.cn>
Reviewed-by: Michael D Kinney <michael.d.kinney@intel.com>
Reviewed-by: Rebecca Cran <rebecca@bsdio.com>
Reviewed-by: Oliver Smith-Denny <osd@smith-denny.com>
2023-04-03 15:29:08 +00:00
Erich McMillan 33f517445b MdeModulePkg/SmbiosDxe: Fix pointer and buffer overflow CodeQL alerts
Details for these CodeQL alerts can be found here:

- Pointer overflow check (cpp/pointer-overflow-check):
  - https://cwe.mitre.org/data/definitions/758.html

- Potential buffer overflow check (cpp/potential-buffer-overflow):
  - https://cwe.mitre.org/data/definitions/676.html

CodeQL alert:

  - Line 1612 in MdeModulePkg/Universal/SmbiosDxe/SmbiosDxe.c
    - Type: Pointer overflow check
    - Severity: Low
    - Problem: Range check relying on pointer overflow

Cc: Dandan Bi <dandan.bi@intel.com>
Cc: Erich McMillan <emcmillan@microsoft.com>
Cc: Jian J Wang <jian.j.wang@intel.com>
Cc: Liming Gao <gaoliming@byosoft.com.cn>
Cc: Michael Kubacki <mikuback@linux.microsoft.com>
Cc: Star Zeng <star.zeng@intel.com>
Cc: Zhichao Gao <zhichao.gao@intel.com>
Cc: Zhiguang Liu <zhiguang.liu@intel.com>
Co-authored-by: Michael Kubacki <michael.kubacki@microsoft.com>
Signed-off-by: Erich McMillan <emcmillan@microsoft.com>
Reviewed-by: Liming Gao <gaoliming@byosoft.com.cn>
Reviewed-by: Oliver Smith-Denny <osd@smith-denny.com>
2023-04-03 15:29:08 +00:00
Michael Kubacki fc00ff286a .github/dependabot.yml: Disable automatic rebasing
Sets the rebase-strategy to "disabled" to prevent automatic
rebasing.

Rebasing can be done manually in the dependabot PR either through
the GitHub UI or the dependabot command.

Cc: Sean Brogan <sean.brogan@microsoft.com>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Signed-off-by: Michael Kubacki <michael.kubacki@microsoft.com>
Reviewed-by: Sean Brogan <sean.brogan@microsoft.com>
Reviewed-by: Michael D Kinney <michael.d.kinney@intel.com>
2023-04-02 02:26:01 +00:00
Michael Kubacki b4af23aaab .github/workflows/codeql-analysis.yml: Add PIP caching
Adds caching of PIP dependencies. This reduces overall execution time
and decreases likelihood of a network error reaching out pypi to get
the dependencies.

Caching happens based on modules specified in pip-requirements.txt.

Cc: Sean Brogan <sean.brogan@microsoft.com>
Cc: Michael Kubacki <mikuback@linux.microsoft.com>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Signed-off-by: Michael Kubacki <michael.kubacki@microsoft.com>
Reviewed-by: Michael D Kinney <michael.d.kinney@intel.com>
2023-04-02 00:24:13 +00:00
Gerd Hoffmann 4ca4d2b9df UefiCpuPkg/MtrrLib: use new IS_POW2() macro.
Drop MtrrLibIsPowerOfTwo function, use the new IS_POW2() macro instead.

The ASSERT() removed (inside MtrrLibIsPowerOfTwo) is superfluous,
another ASSERT() a few lines up in MtrrLibCalculateMtrrs() already
guarantees that Length can not be zero at this point.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
Reviewed-by: Ray Ni <ray.ni@intel.com>
2023-04-01 23:11:44 +00:00
Gerd Hoffmann 12d3d60f51 OvmfPkg: Consume new alignment-related macros
This patch substitutes the macros that were renamed in the second
patch with the new, shared alignment macros.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
Reviewed-by: Michael D Kinney <michael.d.kinney@intel.com>
Reviewed-by: Jiewen Yao <Jiewen.yao@intel.com>
Acked-by: Tom Lendacky <thomas.lendacky@amd.com>
2023-04-01 23:11:44 +00:00
Marvin Häuser 495809a614 MdeModulePkg: Consume new alignment-related macros
This patch substitutes the macros that were renamed in the first
patch with the new, shared alignment macros.

Cc: Jian J Wang <jian.j.wang@intel.com>
Cc: Hao A Wu <hao.a.wu@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Vitaly Cheptsov <vit9696@protonmail.com>
Signed-off-by: Marvin Häuser <mhaeuser@posteo.de>
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
Reviewed-by: Hao A Wu <hao.a.wu@intel.com>
Reviewed-by: Michael D Kinney <michael.d.kinney@intel.com>
2023-04-01 23:11:44 +00:00
Marvin Häuser 836042ffd8 MdePkg/Base.h: Introduce various alignment-related macros
ALIGNOF: Determining the alignment requirement of data types is
crucial to ensure safe memory accesses when parsing untrusted data.

IS_POW2: Determining whether a value is a power of two is important
to verify whether untrusted values are valid alignment values.

IS_ALIGNED: In combination with ALIGNOF data offsets can be verified.
A more general version of the IS_ALIGNED macro previously defined by
several modules.

ADDRESS_IS_ALIGNED: Variant of IS_ALIGNED for pointers and addresses.
Replaces module-specific definitions throughout the code base.

ALIGN_VALUE_ADDEND: The addend to align up can be used to directly
determine the required offset for data alignment.

Cc: Michael D Kinney <michael.d.kinney@intel.com>
Cc: Liming Gao <gaoliming@byosoft.com.cn>
Cc: Zhiguang Liu <zhiguang.liu@intel.com>
Cc: Vitaly Cheptsov <vit9696@protonmail.com>
Signed-off-by: Marvin Häuser <mhaeuser@posteo.de>
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
Reviewed-by: Michael D Kinney <michael.d.kinney@intel.com>
2023-04-01 23:11:44 +00:00
Gerd Hoffmann 24e6daa2bc OvmfPkg: Rename IS_ALIGNED macros to avoid name collisions
This patch is a preparation for the patches that follow. The
subsequent patches will introduce and integrate new alignment-related
macros, which collide with existing definitions in OvmfPkg.
Temporarily rename them to avoid build failure, till they can be
substituted with the new, shared definitions.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
Reviewed-by: Michael D Kinney <michael.d.kinney@intel.com>
Reviewed-by: Jiewen Yao <Jiewen.yao@intel.com>
Acked-by: Tom Lendacky <thomas.lendacky@amd.com>
2023-04-01 23:11:44 +00:00
Marvin Häuser 583f1aba8b MdeModulePkg: Rename IS_ALIGNED macros to avoid name collisions
This patch is a preparation for the patches that follow. The
subsequent patches will introduce and integrate new alignment-related
macros, which collide with existing definitions in MdeModulePkg.
Temporarily rename them to avoid build failure, till they can be
substituted with the new, shared definitions.

Cc: Jian J Wang <jian.j.wang@intel.com>
Cc: Hao A Wu <hao.a.wu@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Signed-off-by: Marvin Häuser <mhaeuser@posteo.de>
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
Reviewed-by: Hao A Wu <hao.a.wu@intel.com>
Reviewed-by: Michael D Kinney <michael.d.kinney@intel.com>
2023-04-01 23:11:44 +00:00
Lendacky, Thomas via groups.io 67a6f414aa UefiCpuPkg/MpInitLib: Ensure SEV-SNP VMSA allocations are not 2MB aligned
BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=4353

Due to AMD erratum #1467, an SEV-SNP VMSA should not be 2MB aligned. To
work around this issue, allocate two pages instead of one. Because of the
way that page allocation is implemented, always try to use the second
page. If the second page is not 2MB aligned, free the first page and use
the second page. If the second page is 2MB aligned, free the second page
and use the first page. Freeing in this way reduces holes in the memory
map.

Fixes: 06544455d0 ("UefiCpuPkg/MpInitLib: Use SEV-SNP AP Creation ...")
Signed-off-by: Tom Lendacky <thomas.lendacky@amd.com>

Acked-by: Gerd Hoffmann <kraxel@redhat.com>
Acked-by: Ray Ni <ray.ni@intel.com>
2023-04-01 03:15:51 +00:00
Lendacky, Thomas via groups.io 3323359a81 UefiCpuPkg/MpInitLib: Reuse VMSA allocation to avoid unreserved allocation
https://bugzilla.tianocore.org/show_bug.cgi?id=4353

When parking the APs on exiting from UEFI, a new page allocation is made.
This allocation, however, does not end up being marked reserved in the
memory map supplied to the OS. To avoid this, re-use the VMSA by clearing
the VMSA RMP flag, updating the page contents and re-setting the VMSA RMP
flag.

Fixes: 06544455d0 ("UefiCpuPkg/MpInitLib: Use SEV-SNP AP Creation ...")
Signed-off-by: Tom Lendacky <thomas.lendacky@amd.com>
Acked-by: Gerd Hoffmann <kraxel@redhat.com>
Acked-by: Ray Ni <ray.ni@intel.com>
2023-04-01 03:15:51 +00:00
Gerd Hoffmann 54051768cd UefiCpuPkg/PiSmmCpuDxeSmm: fix format string
BufferPages is UINTN, so we need "%Lu" when printing it to avoid
it being truncated.  Also cast to UINT64 to make sure it works
for 32bit builds too.

Fixes: 4f441d024b ("UefiCpuPkg/PiSmmCpuDxeSmm: fix error handling")
Reported-by: Laszlo Ersek <lersek@redhat.com>
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Ray Ni <ray.ni@intel.com>
2023-04-01 03:15:51 +00:00
Ray Ni 71210053fa UefiCpuPkg/MtrrTest: Add test cases for TME-MK enable case
When TME-MK is enabled, the MtrrLib should substract the TME-MK
reserved bits from the max PA returned from CPUID instruction.

The new test case guarantees such behavior in MtrrLib.

Signed-off-by: Ray Ni <ray.ni@intel.com>
Cc: Eric Dong <eric.dong@intel.com>
Cc: Rahul Kumar <rahul1.kumar@intel.com>
Cc: Gerd Hoffmann <kraxel@redhat.com>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Cc: Ahmad Anadani <ahmad.anadani@intel.com>
Acked-by: Gerd Hoffmann <kraxel@redhat.com>
Reviewed-by: Michael D Kinney <michael.d.kinney@intel.com>
2023-04-01 01:08:34 +00:00
Ray Ni bb40c3836f UefiCpuPkg/CpuDxe: Substract TME-MK KEY_ID_BITS from CPU max PA
CPUID enumeration of MAX_PA is unaffected by TME-MK activation and
will continue to report the maximum physical address bits available
for software to use, irrespective of the number of KeyID bits.

So, we need to check if TME is enabled and adjust the PA size
accordingly.

Signed-off-by: Ray Ni <ray.ni@intel.com>
Cc: Eric Dong <eric.dong@intel.com>
Cc: Rahul Kumar <rahul1.kumar@intel.com>
Cc: Gerd Hoffmann <kraxel@redhat.com>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Cc: Ahmad Anadani <ahmad.anadani@intel.com>
Acked-by: Gerd Hoffmann <kraxel@redhat.com>
Reviewed-by: Michael D Kinney <michael.d.kinney@intel.com>
2023-04-01 01:08:34 +00:00
Ray Ni f2f526e074 UefiCpuPkg/CpuDxe: Refactor to use CPUID definitions
The patch does not change any code behavior but only refactors by:
* replaces the hardcode 0x80000000 with CPUID_EXTENDED_FUNCTION
* replaces the hardcode 0x80000008 with CPUID_VIR_PHY_ADDRESS_SIZE
* replace "UINT32 Eax" with
   "CPUID_VIR_PHY_ADDRESS_SIZE_EAX  VirPhyAddressSize"

Signed-off-by: Ray Ni <ray.ni@intel.com>
Cc: Eric Dong <eric.dong@intel.com>
Cc: Rahul Kumar <rahul1.kumar@intel.com>
Cc: Gerd Hoffmann <kraxel@redhat.com>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Cc: Ahmad Anadani <ahmad.anadani@intel.com>
Acked-by: Gerd Hoffmann <kraxel@redhat.com>
Reviewed-by: Michael D Kinney <michael.d.kinney@intel.com>
2023-04-01 01:08:34 +00:00
Ray Ni 263782f667 UefiCpuPkg/MtrrLib: Substract TME-MK KEY_ID_BITS from CPU max PA
CPUID enumeration of MAX_PA is unaffected by TME-MK activation and
will continue to report the maximum physical address bits available
for software to use, irrespective of the number of KeyID bits.

So, we need to check if TME is enabled and adjust the PA size
accordingly.

Signed-off-by: Ray Ni <ray.ni@intel.com>
Cc: Eric Dong <eric.dong@intel.com>
Cc: Rahul Kumar <rahul1.kumar@intel.com>
Cc: Gerd Hoffmann <kraxel@redhat.com>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Cc: Ahmad Anadani <ahmad.anadani@intel.com>
Acked-by: Gerd Hoffmann <kraxel@redhat.com>
Reviewed-by: Michael D Kinney <michael.d.kinney@intel.com>
2023-04-01 01:08:34 +00:00