Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Compilation fails with smm #1

Open
jirislaby opened this issue Nov 26, 2019 · 1 comment
Open

Compilation fails with smm #1

jirislaby opened this issue Nov 26, 2019 · 1 comment

Comments

@jirislaby
Copy link

With the current patchset, OVMF build fails for the ovmf-x86_64-smm flavor (which is by default built in SUSE). The errors are like:

OvmfPkg/QemuFlashFvbServicesRuntimeDxe/QemuFlash.c:14:10: fatal error: Library/VmgExitLib.h: No such file or directory
#include <Library/VmgExitLib.h>
         ^~~~~~~~~~~~~~~~~~~~~~
...
build.py...
 : error F002: Failed to build module
       OvmfPkg/QemuFlashFvbServicesRuntimeDxe/FvbServicesSmm.inf [X64, GCC5, DEBUG]

Another one is:

/tmp/cc7KG5f7.ltrans0.ltrans.o: In function `CommonExceptionHandlerWorker':
UefiCpuPkg/Library/CpuExceptionHandlerLib/PeiDxeSmmCpuException.c:95: undefined reference to `DoVcException'

It seems to be expected as e.g. UefiCpuPkg/Library/CpuExceptionHandlerLib/SmmCpuExceptionHandlerLib.inf does not include any of the implementations of DoVcException. So is SMM ignored in this submission?

@tlendacky
Copy link
Collaborator

tlendacky commented Dec 2, 2019 via email

@jirislaby jirislaby changed the title Compilarion fails with smm Compilation fails with smm Dec 4, 2019
tlendacky pushed a commit that referenced this issue Aug 13, 2020
The unit test app supports running in 3 mode:
1. MtrrLibUnitTest generate-random-numbers
     <path to MtrrLib/UnitTest/RandomNumber.c> <random-number count>
   It generates random numbers and writes to RandomNumber.c.

2. MtrrLibUnitTest [<iterations>]
   It tests MtrrLib APIs using configurations generated from static
   numbers generated by mode #1.
   This is the default execution mode running in CI environment.

3. MtrrLibUnitTest <iterations> random
   It tests MtrrLib APIs using configurations generated from random
   numbers.
   This is what developers can use to test MtrrLib for regressions.

Signed-off-by: Ray Ni <[email protected]>
Cc: Michael D Kinney <[email protected]>
Cc: Eric Dong <[email protected]>
Cc: Laszlo Ersek <[email protected]>
Cc: Ming Shao <[email protected]>
Cc: Sean Brogan <[email protected]>
Cc: Bret Barkelew <[email protected]>
Cc: Jiewen Yao <[email protected]>
tlendacky pushed a commit that referenced this issue Jan 26, 2024
Root cause:
1. Before DisableReadonlyPageWriteProtect() is called, the return
address (#1) is pushed in shadow stack.
2. CET is disabled.
3. DisableReadonlyPageWriteProtect() returns to #1.
4. Page table is modified.
5. EnableReadonlyPageWriteProtect() is called, but the return
address (#2) is not pushed in shadow stack.
6. CET is enabled.
7. EnableReadonlyPageWriteProtect() returns to #2.
#CP exception happens because the actual return address (#2)
doesn't match the return address stored in shadow stack (#1).

Analysis:
Shadow stack will stop update after CET disable (DisableCet() in
DisableReadOnlyPageWriteProtect), but normal smi stack will be
continue updated with the function called and return
(DisableReadOnlyPageWriteProtect & EnableReadOnlyPageWriteProtect),
thus leading stack mismatch after CET re-enabled (EnableCet() in
EnableReadOnlyPageWriteProtect).

According SDM Vol 3, 6.15-Control Protection Exception:
Normal smi stack and shadow stack must be matched when CET enable,
otherwise CP Exception will happen, which is caused by a near RET
instruction.

CET is disabled in DisableCet(), while can be enabled in
EnableCet(). This way won't cause the problem because they are
implemented in a way that return address of DisableCet() is
poped out from shadow stack (Incsspq performs a pop to increases
the shadow stack) and EnableCet() doesn't use "RET" but "JMP" to
return to caller. So calling EnableCet() and DisableCet() doesn't
have the same issue as calling DisableReadonlyPageWriteProtect()
and EnableReadonlyPageWriteProtect().

With above root cause & analysis, define below 2 macros instead of
functions for WP & CET operation:
WRITE_UNPROTECT_RO_PAGES (Wp, Cet)
WRITE_PROTECT_RO_PAGES (Wp, Cet)
Because DisableCet() & EnableCet() must be in the same function
to avoid shadow stack and normal SMI stack mismatch.

Note: WRITE_UNPROTECT_RO_PAGES () must be called pair with
WRITE_PROTECT_RO_PAGES () in same function.

Change-Id: I4e126697efcd8dbfb4887da034d8691bfca969e3
Cc: Eric Dong <[email protected]>
Cc: Ray Ni <[email protected]>
Cc: Zeng Star <[email protected]>
Cc: Gerd Hoffmann <[email protected]>
Cc: Rahul Kumar <[email protected]>
Cc: Laszlo Ersek <[email protected]>
Signed-off-by: Jiaxin Wu <[email protected]>
Reviewed-by: Laszlo Ersek <[email protected]>
Reviewed-by: Ray Ni <[email protected]>
Reviewed-by: Eric Dong <[email protected]>
mdroth pushed a commit that referenced this issue Mar 29, 2024
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4534

Bug Details:
PixieFail Bug #1
CVE-2023-45229
CVSS 6.5 : CVSS:3.1/AV:A/AC:L/PR:N/UI:N/S:U/C:H/I:N/A:N
CWE-125 Out-of-bounds Read

Change Overview:

Introduce Dhcp6SeekInnerOptionSafe which performs checks before seeking
the Inner Option from a DHCP6 Option.

>
> EFI_STATUS
> Dhcp6SeekInnerOptionSafe (
>  IN  UINT16  IaType,
>  IN  UINT8   *Option,
>  IN  UINT32  OptionLen,
>  OUT UINT8   **IaInnerOpt,
>  OUT UINT16  *IaInnerLen
>  );
>

Lots of code cleanup to improve code readability.

Cc: Saloni Kasbekar <[email protected]>
Cc: Zachary Clark-williams <[email protected]>

Signed-off-by: Doug Flick [MSFT] <[email protected]>
Reviewed-by: Saloni Kasbekar <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants