Re: [edk2-devel] [PATCH] OvmfPkg: Harden #VC instruction emulation somewhat (CVE-2024-25742)

["Adam Dunlap via groups.io" <acdunlap=google.com@groups.io>]

On Thu, Apr 18, 2024 at 5:15 AM Gerd Hoffmann <kraxel@redhat.com> wrote:
>
> On Wed, Apr 17, 2024 at 09:54:00AM -0700, Adam Dunlap via groups.io wrote:
> > +  UINT8  OpCode;
>
> The linux kernel patch uses "unsigned int opcode" and apparently
> checks more than just the first byte for multi-byte opcodes.  Why
> do it differently here?

Good question. This patch does check for two-byte opcodes with this snippet:

+  OpCode = *(InstructionData->OpCodes);
+  if (OpCode == TWO_BYTE_OPCODE_ESCAPE) {
+    OpCode = *(InstructionData->OpCodes + 1);
+  }

This works because the first byte of two-byte opcodes is always 0x0f in the
cases that we're checking for. I was wary about blindly dereferencing two
bytes since that could cause a page fault if it was actually a 1 byte opcode
that was at the very end of an allocated region. This is also what is done in
the MmioExit function in this file. The linux kernel instruction decoder is much
more extensive than what is done here and I didn't want to duplicate the
whole thing.

> On the bigger picture:  I'm wondering why SNP allows external #VC
> injections in the first place?

Yup, I think it'd be better if it didn't.


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#117995): https://edk2.groups.io/g/devel/message/117995
Mute This Topic: https://groups.io/mt/105581633/7686176
Mute #vc:https://edk2.groups.io/g/devel/mutehashtag/vc
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-