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

OVMF doesn't respect -acpitable QEMU parameter #5

Closed
alexfv opened this issue Jan 12, 2015 · 31 comments
Closed

OVMF doesn't respect -acpitable QEMU parameter #5

alexfv opened this issue Jan 12, 2015 · 31 comments

Comments

@alexfv
Copy link

alexfv commented Jan 12, 2015

QEMU allows to insert various vendor-specific ACPI tables, such as SLIC and MSDM, by -acpitable command-line parameter. SeaBIOS adds this tables to ACPI information, but OVMF doesn't.

vchong pushed a commit to linaro-swg/edk2 that referenced this issue Jun 9, 2015
@lersek
Copy link
Member

lersek commented Nov 25, 2015

OVMF's ACPI linker/loader is certainly supposed to support ACPI tables that are passed to QEMU explicitly on the command line. For example, here's an independent report that it worked for the same exact purpose as yours: http://forums.fedoraforum.org/showthread.php?t=298236

Considering how old this report is, I'm closing it.

If you can still reproduce the issue, please reopen the item, and also upload your SLIC and MSDM tables, plus paste the exact QEMU command line and/or libvirt domain XML you are using. (You are certainly encouraged to hex-edit the actual license key out of the ACPI blobs, but I'll need structurally identical tables.)

@lersek
Copy link
Member

lersek commented Nov 25, 2015

Sigh, obviously I have no permission to close this issue. Second, why is this issue tracker view jumping around the page when I scroll to the bottom in Firefox? Extremely annoying and broken. Is github incompatible with the "It's All Text!" firefox extension? If it is, that's a deal breaker for me.

@alex3kov
Copy link

I'm having the same issue (with OVMF, that is).
Qemu command is

qemu-system-x86_64 -nodefaults -nodefconfig -name Windows7 -enable-kvm \
-machine type=pc,accel=kvm -m 3072 -vga std \
-serial none -parallel none -rtc base=localtime \
-cpu host -smp sockets=1,cores=4,threads=2 \
-smbios file=/vms/dmidecode.bin \
-acpitable file=/vms/acpi_slic.bin \
-acpitable file=/vms/acpi_msdm.bin \
-object rng-random,id=rng0 -device virtio-rng-pci,rng=rng0 \
-usb -usbdevice tablet \
-drive if=pflash,file=/vms/ovmf_x64.bin,format=raw \
-drive if=virtio,file=/vms/disk_diff.qcow2,format=qcow2,media=disk \

Where dmidecode.bin file is generated by dmidecode --dump-bin dmidecode.bin, acpi_slic.bin and acpi_msdm.bin are simply copied from /sys/firmware/acpi/tables/.
I need to present hw information as close to real hardware as possible, including SLIC/MSDM tables. To verify the visibility of ACPI tables to a vm I use ACPICA tools https://www.acpica.org/downloads/binary-tools - inside qemu VM it reports only BOCHS tables.
On the other hand, Windows 7, virtualized with VirtualBox (there's an internal command to present 1 ACPI table to a VM), successfully reads provided SLIC table.
I'm ready to provide additional info and tests. Qemu 2.4.1, OVMF 18419.
Can we please reopen this issue?

@lersek
Copy link
Member

lersek commented Jan 11, 2016

I agree that this should work.

Please try to reproduce the issue with a fresh OVMF build.

Please upload the MSDM and SLIC binary files (with the identifying information in them overwritten with whatever you'd like, e.g. zeroes). Please also upload the smbios binary file.

Thanks
Laszlo

@alex3kov
Copy link

@lersek
Reproduced the issue with ovmf-git 17551.d764d59.
SLIC file fails digital signature (checked with Slic Toolkit) after replacing all IDs with zeroes. Github doesn't take binary files, here they are - http://www.4shared.com/zip/1JBDp9d6ba/slic_bios.html

@lersek
Copy link
Member

lersek commented Jan 11, 2016

@alex3kov

Sorry, that download link doesn't work for me. It seems to want me to disable my adblocker, which I won't do. Here's some alternatives:

  • Please look up my email in the edk2 commit log or the Maintainers.txt file in edk2, and send me the files directly.
  • Please uuencode (with the --base64 option) the zip file, and paste it here as a comment (formatted as "code"). I can uudecode it.
  • Please upload the zip file to your google drive (or some such, if you have one of these fashionable things) and share the link to the file.

Thanks.

@alex3kov
Copy link

@lersek here we go:
http://pastebin.com/pgx1eNXm

@jljusten
Copy link
Member

@lersek, @alex3kov: Seems like your can attach files directly to issues. (See text below the comment box.)

@alex3kov
Copy link

As I mentioned before - Github doesn't take binary files.

@jljusten
Copy link
Member

@alex3kov: Ah, yep. Sorry, I missed that.

@jljusten
Copy link
Member

@lersek: Let me know if I should re-open this issue. By the way, after the svn=>git transition all of the package maintainers should be able to close/re-open.

@lersek
Copy link
Member

lersek commented Jan 13, 2016

@jljusten: please reopen this one, yes. Thanks!

@lersek
Copy link
Member

lersek commented Jan 13, 2016

re: attaching files, maybe we can combine base64 encoding and attachments. That is, we should do the base64 uuencode, then attach the output as an attachment, instead of cluttering comment space.

Sorry that I didn't think of this earlier, but honestly... is this the eighties where we have to manually ascii-armor our binaries? I guess github wants to prevent "warez" and viruses by disabling binary attachments. I'm sure they happy to pay the price of the 4/3 size factor that's inherent in base64...

@alex3kov
Copy link

@lersek Just tried putting base64 text in a text file and attaching it - believe it or not, github detected my trickery and didn't let me attach it either! I put it on pastebin though.

@lersek
Copy link
Member

lersek commented Jan 13, 2016

@alex3kov

In your earlier comment you said:

SLIC file fails digital signature (checked with Slic Toolkit) after replacing all IDs with zeroes.

Therefore I'm now assuming that the base64 encoded, zipped SLIC that you attached here is actually the real deal, with the key material intact. Is that correct?

I do think so, because I ran iasl -d on the slic.bin file, in order to decompile it, and as far as I can see, the "Public Key Structure" and "Windows Marker Structure" sub-tables have not been zeroed out:

/*
 * Intel ACPI Component Architecture
 * AML/ASL+ Disassembler version 20141107-64 [Dec 27 2014]
 * Copyright (c) 2000 - 2014 Intel Corporation
 * 
 * Disassembling to symbolic ASL+ operators
 *
 * Disassembly of slic.bin, Wed Jan 13 11:56:37 2016
 *
 * ACPI Data Table [SLIC]
 *
 * Format: [HexOffset DecimalOffset ByteLength]  FieldName : FieldValue
 */

[000h 0000   4]                    Signature : "SLIC"    [Software Licensing Description Table]
[004h 0004   4]                 Table Length : 00000176
[008h 0008   1]                     Revision : 03
[009h 0009   1]                     Checksum : 6A     /* Incorrect checksum, should be CC */
[00Ah 0010   6]                       Oem ID : "0000  "
[010h 0016   8]                 Oem Table ID : "0000   "
[018h 0024   4]                 Oem Revision : 01072009
[01Ch 0028   4]              Asl Compiler ID : "MSFT"
[020h 0032   4]        Asl Compiler Revision : 00010013


[024h 0036   4]                Subtable Type : 00000000 [Public Key Structure]
[028h 0040   4]                       Length : 0000009C

[02Ch 0044   1]                     Key Type : 06
[02Dh 0045   1]                      Version : 02
[02Eh 0046   2]                     Reserved : 0000
[030h 0048   4]                    Algorithm : 00002400
[034h 0052   4]                        Magic : "RSA1"
[038h 0056   4]                    BitLength : 00000400
[03Ch 0060   4]                     Exponent : 00010001
[040h 0064 128]                      Modulus : 7F F6 C1 05 BE 5C 57 63 A5 8A 68 F3 6E 8F 06 FA \
                                               AF B4 9F 68 82 23 EC 50 40 5A 73 7F EC E4 07 CB \
                                               DC 25 1A 9C E3 E3 66 11 E0 A5 98 06 C5 80 0A FA \
                                               42 93 86 98 E7 D5 1B D4 D7 3A A4 0B EE E2 7D BE \
                                               5F 5B 15 0C AB D0 21 DE BF E9 B5 6E A4 57 B9 8C \
                                               0C D2 BA 3A 69 30 76 94 71 A2 64 D7 4C D8 85 BF \
                                               DF A5 6A C8 DC 45 D5 4D 8C B8 8C 05 2F FC 2E 23 \
                                               C4 29 C5 6F 3F 29 6C 6D 57 79 0E B6 75 ED 21 95

[0C0h 0192   4]                Subtable Type : 00000001 [Windows Marker Structure]
[0C4h 0196   4]                       Length : 000000B6

[0C8h 0200   4]                      Version : 00020000
[0CCh 0204   6]                       Oem ID : "0000  "
[0D2h 0210   8]                 Oem Table ID : "0000   "
[0DAh 0218   8]                 Windows Flag : "WINDOWS "
[0E2h 0226   4]                 SLIC Version : 00020001
[0E6h 0230  16]                     Reserved : 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
[0F6h 0246 128]                    Signature : 7A AB 2D D4 C8 27 C3 2F CB 4C 40 7E 26 1A 0B EE \
                                               49 FC 48 09 7F F8 12 CC 64 18 35 B3 63 EC 48 C2 \
                                               25 47 53 92 31 30 E1 18 0C 55 ED 57 6A F5 E7 79 \
                                               01 34 2D 7C FA 3C 30 A1 41 C8 84 31 6F 45 FB B4 \
                                               CC ED 13 E0 9D 25 78 7C 37 45 CE 29 A2 CC 65 84 \
                                               2E 5F B6 65 F7 EE 27 EE E6 2C B7 41 1D 20 16 AC \
                                               9C 7C 95 E3 36 92 19 30 7B B6 A1 75 EA A2 40 4F \
                                               EA BE 71 92 E5 20 FE C4 70 10 1A 9D DF 78 BA C3

But, looking at the decompiled file, I can actually say at once why it doesn't work with OVMF. Notice the Incorrect checksum, should be CC comment emitted by iasl during decompilation? Perhaps you overwrote the Oem ID and Oem Table ID fields manually -- but you forgot to update the ACPI table checksum.

OVMF verifies the checksum before deciding that it is likely looking at an ACPI table. See the CalculateSum8() call in Process2ndPassCmdAddPointer(), file OvmfPkg/AcpiPlatformDxe/QemuFwCfgAcpi.c.

After I fixed up the incorrect checksum in your file with a hex editor, it worked well with OVMF.

@alex3kov
Copy link

@lersek

Therefore I'm now assuming that the base64 encoded, zipped SLIC that you attached here is actually the real deal, with the key material intact. Is that correct?

No, attached (well, you know) slic table - the one you're working with - is edited one (I replaced vendor IDs with zeroes). What I'm passing to qemu/OVMF is unedited one of course. Just tried iasl -d on unedited one - it doesn't complain about checksum. Can you post your qemu command line please?

@lersek
Copy link
Member

lersek commented Jan 13, 2016

@alex3kov

I just took an earlier libvirt guest of mine, and added

<qemu:arg value='-acpitable'/>
<qemu:arg value='file=/tmp/slic2.bin'/>

to the end of the domain XML. These XML elements append -acpitable file=/tmp/slic2.bin directly to the QEMU command line.

One other thing comes to mind though. In your command line I notice:

-drive if=pflash,file=/vms/ovmf_x64.bin,format=raw \

You seem to be using a unified pflash file, which contains both the live varstore and the firmware binary. Are you sure you replaced this file after rebuilding OVMF?

@alex3kov
Copy link

I just took an earlier libvirt guest of mine, and added

I don't use libvirt, just running qemu-system-x86_64 from shell. Can you try the same, to eliminate libvirt from the equation?

You seem to be using a unified pflash file, which contains both the live varstore and the firmware binary. Are you sure you replaced this file after rebuilding OVMF?

Yes, I replaced it.

@lersek
Copy link
Member

lersek commented Jan 13, 2016

@alex3kov

Also, your smbios usage seems incorrect. The output of dmidecode --dump-bin is a set of SMBIOS tables, in some dmidecode-specific representation (probably just concatenated). Whereas QEMU's -smbios file=zzzz option takes one SMBIOS table.

So, if you want to pass in multiple SMBIOS tables, you should pass each with a separate -smbios file=... option. I'm unsure if dmidecode can split up its binary output like this, especially for table types for which there are several table instances. You could try getting the individual tables from /sys/firmware/dmi/entries/*/raw.

@alex3kov
Copy link

@lersek

Also, your smbios usage seems incorrect. The output of dmidecode --dump-bin is a set of SMBIOS tables, in some dmidecode-specific representation (probably just concatenated). Whereas QEMU's -smbios file=zzzz option takes one SMBIOS table.

I knew it couldn't be that easy :) Ok, I just replaced my -smbios parameters to this (with real values taken from dmidecode -t0 and dmidecode -t1 instead of 00):

-smbios type=0,uefi=on,vendor="00",version="00",date="00",release=0.0 \
-smbios type=1,manufacturer="00",product="00",serial="00",uuid=00-00,sku="00" \

And ran the VM - still, acpidump.exe -s shows only BOCHS tables.

@lersek
Copy link
Member

lersek commented Jan 13, 2016

@alex3kov

Wait a second, ACPI and SMBIOS are two separate things. I commented on your SMBIOS usage independently of ACPI. If you'd like to verify SMBIOS in the guest, then (for example) boot some GNU/Linux livecd, and run dmidecode.

Anyway, my command line is the following, for the -acpitable option -- I verified it with "dmesg" and acpidump in a Linux guest:

ISO=/mnt/data/isos/fedora/22/Fedora-Live-Workstation-x86_64-22-3.iso
CODE=/usr/share/edk2.git/ovmf-x64/OVMF_CODE-pure-efi.fd
TMPL=/usr/share/edk2.git/ovmf-x64/OVMF_VARS-pure-efi.fd

if ! [ -e vars.fd ]; then
  cp -- "$TMPL" vars.fd
fi

/opt/qemu-installed/bin/qemu-system-x86_64 \
  -enable-kvm \
  -m 2048 \
  -drive if=pflash,readonly,format=raw,file=$CODE \
  -drive if=pflash,format=raw,file=vars.fd \
  -drive id=cdrom,if=none,readonly,format=raw,cache=writethrough,file=$ISO \
  -device virtio-scsi-pci,id=scsi0 \
  -device scsi-cd,bus=scsi0.0,drive=cdrom,bootindex=0 \
  -debugcon file:debug.log \
  -global isa-debugcon.iobase=0x402 \
  -monitor stdio \
  -acpitable file=/tmp/slic2.bin

@alex3kov
Copy link

@lersek

Wait a second, ACPI and SMBIOS are two separate things. I commented on your SMBIOS usage independently of ACPI. If you'd like to verify SMBIOS in the guest, then (for example) boot some GNU/Linux livecd, and run "dmidecode".

I never said they weren't :) But you mentioned it, so I thought -smbios can interfere with acpi tables. Btw, windows is pretty OK with smbios file, dumped by dmidecode --dump-bin - it reads BIOS info like vendor, SN and so on.
I replicated your experiment on my host and Fedora indeed sees the provided acpi table, but Windows 7 guest doesn't! I booted into Windows 7 installation CD to try and rule out problems with installed Windows and ran acpidump from repair command prompt, but it's not working - says "The subsystem needed to support the image type is not present".
Windows 7 guest was P2V-ed with disk2vhd. VirtualBox successfully presents ACPI table to Windows 7 guest, migrated in the same way - but VirtualBox boots from BIOS, not UEFI (I'm not sure if that's related or not).

@lersek
Copy link
Member

lersek commented Jan 13, 2016

If VirtualBox boots from BIOS, not UEFI, then that's a big difference. Under "legacy" BIOS, ACPI tables are presented to the OS differently -- the RSD PTR structure (the root of the tree of tables) is not linked into the UEFI system configuration table then, but placed into a well-known low address area, and the OS searches that range for the RSD PTR signature. (See "5.2.5.1 Finding the RSDP on IA-PC Systems" in ACPI 6.0.)

So yeah it's quite different.

@lersek
Copy link
Member

lersek commented Jan 13, 2016

@alex3kov

BTW, you can't P2V a Windows installation from a legacy BIOS machine and then expect it to work in a virtual machine that has UEFI firmware. The firmware types must match.

We should confirm with @rwmjones to be sure.

@alex3kov
Copy link

@lersek No, it's not like that - I P2V-ed BIOS Windows 7 installation and successfully ran it in VirtualBox in BIOS mode, with ACPI table.
Now I've P2V-ed GPT/UEFI Windows 7 installation and trying to run it in Qemu/OVMF, with ACPI table.
I'm still confused about how Qemu interprets/works with both -smbios <parameters> and -drive if=pflash,file=/vms/ovmf_code_x64.bin parameters passed to it. Does it present both BIOS and UEFI firmwares to a guest? Can/does this arrangement confuse the guest?

Anyway, we seem to have narrowed the problem to "All else being equal, Fedora Live recognizes ACPI table passed to it and installed Windows guest doesn't". Any chance of fixing that? :)

@lersek
Copy link
Member

lersek commented Jan 13, 2016

@alex3kov

QEMU exposes the ACPI payload that it generates through "fw_cfg". This is a simple communications protocol between QEMU and guest firmware (although there is work underway to access fw_cfg from guest Linux too). You can read about fw_cfg in QEMU's docs/specs/fw_cfg.txt file.

QEMU generates most (well, all) of the ACPI payload. However, if you use the -acpitable file=... switch as well (as many times as you feel like), then QEMU will include those tables as well in the ACPI stuff that is exposed to the guest firmware over fw_cfg.

SMBIOS is very similar. The tables are much simpler, but the fact remains that QEMU generates the SMBIOS payload too, and exposes it to the guest fw over fw_cfg. The user can control and/or extend the exact SMBIOS data exposed to the guest fw.

ACPI and SMBIOS are independent, as I noted earlier.

Now, the guest firmware has to process and install these tables for the runtime guest OS to consume. For ACPI this is very complex. There is an "ACPI linker/loader" protocol -- a command script -- that QEMU generates and the guest firmware has to consume and process. This script instructs the firmware to download tables and to patch them in various ways. This way the firmware need not have any table-specific knowledge. user-provided ACPI tables are not patched, just linked into the RSDT.

For SMBIOS, we don't have a linker/loader, but the procedure is more or less the same. Guest fw downloads stuff, exposes it to runtime OS, done.

Now, legacy BIOS and UEFI differ in the way they expose these tables to guest OSes. For that reason, both SeaBIOS and OVMF have separate, independent "fw_cfg client" implementations, for the above exchanges.

Don't be confused about -drive if=pflash,..., -smbios file=..., and -acpitable file=.... They are all mutually independent / orthogonal. The pflash option selects OVMF (binary and varstore) over SeaBIOS, practically speaking. The smbios option influences the SMBIOS payload generation, which (in QEMU) is independent of the firmware. The acpitable option does the same for the ACPI payload generation. Once the guest runs, it is the specific guest firmware's (OVMF's vs. SeaBIOS's) responsibility to download the exact same fw_cfg payload (both SMBIOS and ACPI), and to install them sensibly for the runtime OS, in a way that conforms to the given firmware type (UEFI vs. legacy BIOS).

Anyway, we seem to have narrowed the problem to "All else being equal, Fedora Live recognizes ACPI table passed to it and installed Windows guest doesn't". Any chance of fixing that? :)

What is there to fix? OVMF calls the EFI_ACPI_TABLE_PROTOCOL.InstallAcpiTable() protocol member function to install the table. The -acpitable switch is only special as far as QEMU is concerned (i.e. the way it gets added to the ACPI payload); for OVMF it's all the same.

The Fedora guest confirms that the table is installed just fine, so this is certainly a Windows 7 problem. Windows 7's UEFI support is half-assed anyway.

Can you perhaps try with a Windows 8+ guest?

And/or, can you check in Windows 7 (UEFI) if you see any other ACPI tables? (Most likely, auto-generated tables.)

If you can, can you try hexediting the signature (SLIC) and perhaps the other OEM bits of the manually passed-in table, to TEMP or MINE? If that suddenly makes the table appear in the WIndows 7 UEFI guest, you can thank Redmond for the foul play (they are obviously hiding SLIC for whatever reason then).

@lersek
Copy link
Member

lersek commented Jan 13, 2016

Note: don't forget to recalculate the checksum if you modify the table. You can hexedit the signature / OEM bits in the file, then run iasl -d, and it will tell you (in the warning comment seen above) what checksum byte to put in.

@alex3kov
Copy link

@lersek
Thanks for the pointers about all this internal Qemu/OVMF stuff. I'll have to set aside some hours to dig into it (not a system programmer here), maybe there's some way to jam that table down Windows 7's throat after all.
Let me check if I got your overall point: OVMF presents ACPI table correctly to a guest OS; Windows 7 is not reading that ACPI table, because Windows 7's way of reading ACPI tables from UEFI firmwares in general is wrong/flawed somehow. Correct?

And/or, can you check in Windows 7 (UEFI) if you see any other ACPI tables? (Most likely, auto-generated tables.)

Yes, as mentioned earlier, they are called BOCHS tables - I think they are auto-generated. Here:

scr
If it picks up those tables from OVMF - why not the -acpitable-passed one?

Can you perhaps try with a Windows 8+ guest?

Sure; when I'll have the time.

If you can, can you try hexediting the signature (SLIC) and perhaps the other OEM bits of the manually passed-in table, to TEMP or MINE? If that suddenly makes the table appear in the WIndows 7 UEFI guest, you can thank Redmond for the foul play (they are obviously hiding SLIC for whatever reason then).

I don't think this is likely - Windows 7 is very keen to read SLIC table, and it successfully reads the very same table on a real (the same) HW (at least that's what I remember... I'll test again to be sure). This is a dual-boot setup.

@lersek
Copy link
Member

lersek commented Jan 13, 2016

@alex3kov

Let me check if I got your overall point: OVMF presents ACPI table correctly to a guest OS;

Yes.

Windows 7 is not reading that ACPI table, because the way Windows 7's way of reading ACPI tables from UEFI firmwares in general is wrong/flawed somehow. Correct?

Much more likely there's some special handling done for SLIC. If Windows 7 (UEFI) handled ACPI tables completely wrong, then it wouldn't run on any hardware at all, including QEMU-with-OVMF.

You are using acpidump.exe for checking ACPI tables in the Windows 7 (UEFI) guest. I have no clue what that program is. Is that the Windows build of the ACPI CA? What we should verify is the Windows kernel's view of the ACPI tables. I have no idea what restrictions acpidump.exe could be subject to.

... Hm, I've just remembered something. In edk2's implementation of EFI_ACPI_TABLE_PROTOCOL, we have several instances of a comment:

AcpiTableProtocol.c:576:      // RSDP OEM information is updated to match the FADT OEM information
AcpiTableProtocol.c:585:      // RSDT OEM information is updated to match the FADT OEM information.
AcpiTableProtocol.c:633:      // RSDP OEM information is updated to match the FADT OEM information
AcpiTableProtocol.c:642:      // RSDT OEM information is updated to match FADT OEM information.
AcpiTableProtocol.c:657:      // XSDT OEM information is updated to match FADT OEM information.

I vaguely recall some emails about Windows insisting on an OEM info match between these tables. (The commit log for this directory in edk2, or the rest of this source file, don't seem to finger Windows in any way though. This is probably something to ask on edk2-devel.) Perhaps the problem is that the OEM bits in your SLIC don't match the OEM bits in the main tables that QEMU generates.

Let me dig into my mailbox...

groan That is it exactly! See this email thread:

http://thread.gmane.org/gmane.comp.emulators.qemu/358854

Please read the entire thread (it's not that long). I don't understand why I didn't remember this from last Septemer :) In any case we have an open RHBZ for it too:

https://bugzilla.redhat.com/show_bug.cgi?id=1248758

So, this looks like a Windows 7 peculiarity. In fact the SLIC specification from Microsoft spells out that the OEMID and OEM Table ID in the SLIC must match the corresponding fields in the RSDT/XSDT. That's why your Windows 7 guest ignores the SLIC.

Patches for accommodating this requirement in QEMU exist in the wild, but (AFAICS) none of those are in upstream QEMU.

For further complication, OVMF doesn't actually take the RSDT and the XSDT from QEMU -- EFI_ACPI_TABLE_PROTOCOL handles those internally. So the OEM bits that QEMU places in the RSDT and XSDT don't matter: the MdeModulePkg/Universal/Acpi/AcpiTableDxe driver in edk2 fills in those from build-time configured constant values at initialization, and then updates the RSDT/XSDT OEM bits from the same OEM fields in the FADT (when the FADT is installed). This means that the Debian-originated QEMU patch linked in the discussion above wouldn't actually help, when the guest firmware is OVMF.

Instead, since QEMU produces a FADT, and OVMF does install said FADT with EFI_ACPI_TABLE_PROTOCOL, ultimately the FADT's OEM fields have to match your SLIC fields. You could try grabbing @rwmjones's patches from the qemu-devel thread I linked above.

Sorry it took so long to figure this out.

@jljusten, apparently this isn't an OVMF issue after all. A QEMU patch would be needed that lets the user configure the OEM fields in the auto-generated tables; most importantly among them the OEM fields in the FADT.

@alex3kov
Copy link

@lersek Tried the patches - one of them isn't working on a freshly fetched qemu, I made a comment at RHBZ; to be continued over there.
This issue should probably be renamed to something more accurate like "Windows 7 guest doesn't read SLIC table when OVMF is used with Qemu" to guide other users with the same problem.

You are using acpidump.exe for checking ACPI tables in the Windows 7 (UEFI) guest. I have no clue what that program is. Is that the Windows build of the ACPI CA? What we should verify is the Windows kernel's view of the ACPI tables. I have no idea what restrictions acpidump.exe could be subject to.

Not that it matters anymore, but yes - downloaded them from here

@lersek
Copy link
Member

lersek commented Jul 21, 2016

This (closed) issue has been manually migrated to
https://tianocore.acgmultimedia.com/show_bug.cgi?id=71

Flickdm added a commit to Flickdm/edk2 that referenced this issue Jan 25, 2024
REF:https://bugzilla.tianocore.org/show_bug.cgi?id=4537
REF:https://bugzilla.tianocore.org/show_bug.cgi?id=4538

Bug Details:
PixieFail Bug tianocore#4
CVE-2023-45232
CVSS 7.5 : CVSS:3.1/AV:N/AC:L/PR:N/UI:N/S:U/C:N/I:N/A:H
CWE-835 Loop with Unreachable Exit Condition ('Infinite Loop')

Infinite loop when parsing unknown options in the Destination Options
header

PixieFail Bug tianocore#5
CVE-2023-45233
CVSS 7.5 : CVSS:3.1/AV:N/AC:L/PR:N/UI:N/S:U/C:N/I:N/A:H
CWE-835 Loop with Unreachable Exit Condition ('Infinite Loop')

Infinite loop when parsing a PadN option in the Destination Options
header

Change Overview:

Most importantly this change corrects the following incorrect math
and cleans up the code.

>   // It is a PadN option
>   //
> - Offset = (UINT8)(Offset + *(Option + Offset + 1) + 2);
> + OptDataLen = *(IP6_OFFSET_OF_OPT_LEN (Option + Offset));
> + Offset     = IP6_NEXT_OPTION_OFFSET (Offset, OptDataLen);

> case Ip6OptionSkip:
> - Offset = (UINT8)(Offset + *(Option + Offset + 1));
>   OptDataLen = *(IP6_OFFSET_OF_OPT_LEN (Option + Offset));
>   Offset     = IP6_NEXT_OPTION_OFFSET (Offset, OptDataLen);

Additionally, this change also corrects incorrect math where the calling
function was calculating the HDR EXT optionLen as a uint8 instead of a
uint16

> - OptionLen = (UINT8)((*Option + 1) * 8 - 2);
> + OptionLen = IP6_HDR_EXT_LEN (*Option) -
IP6_COMBINED_SIZE_OF_NEXT_HDR_AND_LEN;

Additionally this check adds additional logic to santize the incoming
data

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

Signed-off-by: Doug Flick [MSFT] <[email protected]>
Flickdm added a commit to Flickdm/edk2 that referenced this issue Jan 25, 2024
REF:https://bugzilla.tianocore.org/show_bug.cgi?id=4537
REF:https://bugzilla.tianocore.org/show_bug.cgi?id=4538

Bug Details:
PixieFail Bug tianocore#4
CVE-2023-45232
CVSS 7.5 : CVSS:3.1/AV:N/AC:L/PR:N/UI:N/S:U/C:N/I:N/A:H
CWE-835 Loop with Unreachable Exit Condition ('Infinite Loop')

Infinite loop when parsing unknown options in the Destination Options
header

PixieFail Bug tianocore#5
CVE-2023-45233
CVSS 7.5 : CVSS:3.1/AV:N/AC:L/PR:N/UI:N/S:U/C:N/I:N/A:H
CWE-835 Loop with Unreachable Exit Condition ('Infinite Loop')

Infinite loop when parsing a PadN option in the Destination Options
header

Change Overview:

Most importantly this change corrects the following incorrect math
and cleans up the code.

>   // It is a PadN option
>   //
> - Offset = (UINT8)(Offset + *(Option + Offset + 1) + 2);
> + OptDataLen = *(IP6_OFFSET_OF_OPT_LEN (Option + Offset));
> + Offset     = IP6_NEXT_OPTION_OFFSET (Offset, OptDataLen);

> case Ip6OptionSkip:
> - Offset = (UINT8)(Offset + *(Option + Offset + 1));
>   OptDataLen = *(IP6_OFFSET_OF_OPT_LEN (Option + Offset));
>   Offset     = IP6_NEXT_OPTION_OFFSET (Offset, OptDataLen);

Additionally, this change also corrects incorrect math where the calling
function was calculating the HDR EXT optionLen as a uint8 instead of a
uint16

> - OptionLen = (UINT8)((*Option + 1) * 8 - 2);
> + OptionLen = IP6_HDR_EXT_LEN (*Option) -
IP6_COMBINED_SIZE_OF_NEXT_HDR_AND_LEN;

Additionally this check adds additional logic to santize the incoming
data

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

Signed-off-by: Doug Flick [MSFT] <[email protected]>
Flickdm added a commit to Flickdm/edk2 that referenced this issue Jan 25, 2024
REF:https://bugzilla.tianocore.org/show_bug.cgi?id=4537
REF:https://bugzilla.tianocore.org/show_bug.cgi?id=4538

Bug Details:
PixieFail Bug tianocore#4
CVE-2023-45232
CVSS 7.5 : CVSS:3.1/AV:N/AC:L/PR:N/UI:N/S:U/C:N/I:N/A:H
CWE-835 Loop with Unreachable Exit Condition ('Infinite Loop')

Infinite loop when parsing unknown options in the Destination Options
header

PixieFail Bug tianocore#5
CVE-2023-45233
CVSS 7.5 : CVSS:3.1/AV:N/AC:L/PR:N/UI:N/S:U/C:N/I:N/A:H
CWE-835 Loop with Unreachable Exit Condition ('Infinite Loop')

Infinite loop when parsing a PadN option in the Destination Options
header

Change Overview:

Most importantly this change corrects the following incorrect math
and cleans up the code.

>   // It is a PadN option
>   //
> - Offset = (UINT8)(Offset + *(Option + Offset + 1) + 2);
> + OptDataLen = *(IP6_OFFSET_OF_OPT_LEN (Option + Offset));
> + Offset     = IP6_NEXT_OPTION_OFFSET (Offset, OptDataLen);

> case Ip6OptionSkip:
> - Offset = (UINT8)(Offset + *(Option + Offset + 1));
>   OptDataLen = *(IP6_OFFSET_OF_OPT_LEN (Option + Offset));
>   Offset     = IP6_NEXT_OPTION_OFFSET (Offset, OptDataLen);

Additionally, this change also corrects incorrect math where the calling
function was calculating the HDR EXT optionLen as a uint8 instead of a
uint16

> - OptionLen = (UINT8)((*Option + 1) * 8 - 2);
> + OptionLen = IP6_HDR_EXT_LEN (*Option) -
IP6_COMBINED_SIZE_OF_NEXT_HDR_AND_LEN;

Additionally this check adds additional logic to santize the incoming
data

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

Signed-off-by: Doug Flick [MSFT] <[email protected]>
Flickdm added a commit to Flickdm/edk2 that referenced this issue Jan 25, 2024
REF:https://bugzilla.tianocore.org/show_bug.cgi?id=4537
REF:https://bugzilla.tianocore.org/show_bug.cgi?id=4538

Bug Details:
PixieFail Bug tianocore#4
CVE-2023-45232
CVSS 7.5 : CVSS:3.1/AV:N/AC:L/PR:N/UI:N/S:U/C:N/I:N/A:H
CWE-835 Loop with Unreachable Exit Condition ('Infinite Loop')

Infinite loop when parsing unknown options in the Destination Options
header

PixieFail Bug tianocore#5
CVE-2023-45233
CVSS 7.5 : CVSS:3.1/AV:N/AC:L/PR:N/UI:N/S:U/C:N/I:N/A:H
CWE-835 Loop with Unreachable Exit Condition ('Infinite Loop')

Infinite loop when parsing a PadN option in the Destination Options
header

Change Overview:

Most importantly this change corrects the following incorrect math
and cleans up the code.

>   // It is a PadN option
>   //
> - Offset = (UINT8)(Offset + *(Option + Offset + 1) + 2);
> + OptDataLen = *(IP6_OFFSET_OF_OPT_LEN (Option + Offset));
> + Offset     = IP6_NEXT_OPTION_OFFSET (Offset, OptDataLen);

> case Ip6OptionSkip:
> - Offset = (UINT8)(Offset + *(Option + Offset + 1));
>   OptDataLen = *(IP6_OFFSET_OF_OPT_LEN (Option + Offset));
>   Offset     = IP6_NEXT_OPTION_OFFSET (Offset, OptDataLen);

Additionally, this change also corrects incorrect math where the calling
function was calculating the HDR EXT optionLen as a uint8 instead of a
uint16

> - OptionLen = (UINT8)((*Option + 1) * 8 - 2);
> + OptionLen = IP6_HDR_EXT_LEN (*Option) -
IP6_COMBINED_SIZE_OF_NEXT_HDR_AND_LEN;

Additionally this check adds additional logic to santize the incoming
data

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

Signed-off-by: Doug Flick [MSFT] <[email protected]>
Flickdm added a commit to Flickdm/edk2 that referenced this issue Jan 25, 2024
REF:https://bugzilla.tianocore.org/show_bug.cgi?id=4537
REF:https://bugzilla.tianocore.org/show_bug.cgi?id=4538

Bug Details:
PixieFail Bug tianocore#4
CVE-2023-45232
CVSS 7.5 : CVSS:3.1/AV:N/AC:L/PR:N/UI:N/S:U/C:N/I:N/A:H
CWE-835 Loop with Unreachable Exit Condition ('Infinite Loop')

Infinite loop when parsing unknown options in the Destination Options
header

PixieFail Bug tianocore#5
CVE-2023-45233
CVSS 7.5 : CVSS:3.1/AV:N/AC:L/PR:N/UI:N/S:U/C:N/I:N/A:H
CWE-835 Loop with Unreachable Exit Condition ('Infinite Loop')

Infinite loop when parsing a PadN option in the Destination Options
header

Change Overview:

Most importantly this change corrects the following incorrect math
and cleans up the code.

>   // It is a PadN option
>   //
> - Offset = (UINT8)(Offset + *(Option + Offset + 1) + 2);
> + OptDataLen = *(IP6_OFFSET_OF_OPT_LEN (Option + Offset));
> + Offset     = IP6_NEXT_OPTION_OFFSET (Offset, OptDataLen);

> case Ip6OptionSkip:
> - Offset = (UINT8)(Offset + *(Option + Offset + 1));
>   OptDataLen = *(IP6_OFFSET_OF_OPT_LEN (Option + Offset));
>   Offset     = IP6_NEXT_OPTION_OFFSET (Offset, OptDataLen);

Additionally, this change also corrects incorrect math where the calling
function was calculating the HDR EXT optionLen as a uint8 instead of a
uint16

> - OptionLen = (UINT8)((*Option + 1) * 8 - 2);
> + OptionLen = IP6_HDR_EXT_LEN (*Option) -
IP6_COMBINED_SIZE_OF_NEXT_HDR_AND_LEN;

Additionally this check adds additional logic to santize the incoming
data

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

Signed-off-by: Doug Flick [MSFT] <[email protected]>
Flickdm added a commit to Flickdm/edk2 that referenced this issue Jan 25, 2024
REF:https://bugzilla.tianocore.org/show_bug.cgi?id=4537
REF:https://bugzilla.tianocore.org/show_bug.cgi?id=4538

Bug Details:
PixieFail Bug tianocore#4
CVE-2023-45232
CVSS 7.5 : CVSS:3.1/AV:N/AC:L/PR:N/UI:N/S:U/C:N/I:N/A:H
CWE-835 Loop with Unreachable Exit Condition ('Infinite Loop')

Infinite loop when parsing unknown options in the Destination Options
header

PixieFail Bug tianocore#5
CVE-2023-45233
CVSS 7.5 : CVSS:3.1/AV:N/AC:L/PR:N/UI:N/S:U/C:N/I:N/A:H
CWE-835 Loop with Unreachable Exit Condition ('Infinite Loop')

Infinite loop when parsing a PadN option in the Destination Options
header

Change Overview:

Most importantly this change corrects the following incorrect math
and cleans up the code.

>   // It is a PadN option
>   //
> - Offset = (UINT8)(Offset + *(Option + Offset + 1) + 2);
> + OptDataLen = *(IP6_OFFSET_OF_OPT_LEN (Option + Offset));
> + Offset     = IP6_NEXT_OPTION_OFFSET (Offset, OptDataLen);

> case Ip6OptionSkip:
> - Offset = (UINT8)(Offset + *(Option + Offset + 1));
>   OptDataLen = *(IP6_OFFSET_OF_OPT_LEN (Option + Offset));
>   Offset     = IP6_NEXT_OPTION_OFFSET (Offset, OptDataLen);

Additionally, this change also corrects incorrect math where the calling
function was calculating the HDR EXT optionLen as a uint8 instead of a
uint16

> - OptionLen = (UINT8)((*Option + 1) * 8 - 2);
> + OptionLen = IP6_HDR_EXT_LEN (*Option) -
IP6_COMBINED_SIZE_OF_NEXT_HDR_AND_LEN;

Additionally this check adds additional logic to santize the incoming
data

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

Signed-off-by: Doug Flick [MSFT] <[email protected]>
Flickdm added a commit to Flickdm/edk2 that referenced this issue Jan 25, 2024
REF:https://bugzilla.tianocore.org/show_bug.cgi?id=4537
REF:https://bugzilla.tianocore.org/show_bug.cgi?id=4538

Bug Details:
PixieFail Bug tianocore#4
CVE-2023-45232
CVSS 7.5 : CVSS:3.1/AV:N/AC:L/PR:N/UI:N/S:U/C:N/I:N/A:H
CWE-835 Loop with Unreachable Exit Condition ('Infinite Loop')

Infinite loop when parsing unknown options in the Destination Options
header

PixieFail Bug tianocore#5
CVE-2023-45233
CVSS 7.5 : CVSS:3.1/AV:N/AC:L/PR:N/UI:N/S:U/C:N/I:N/A:H
CWE-835 Loop with Unreachable Exit Condition ('Infinite Loop')

Infinite loop when parsing a PadN option in the Destination Options
header

Change Overview:

Most importantly this change corrects the following incorrect math
and cleans up the code.

>   // It is a PadN option
>   //
> - Offset = (UINT8)(Offset + *(Option + Offset + 1) + 2);
> + OptDataLen = *(IP6_OFFSET_OF_OPT_LEN (Option + Offset));
> + Offset     = IP6_NEXT_OPTION_OFFSET (Offset, OptDataLen);

> case Ip6OptionSkip:
> - Offset = (UINT8)(Offset + *(Option + Offset + 1));
>   OptDataLen = *(IP6_OFFSET_OF_OPT_LEN (Option + Offset));
>   Offset     = IP6_NEXT_OPTION_OFFSET (Offset, OptDataLen);

Additionally, this change also corrects incorrect math where the calling
function was calculating the HDR EXT optionLen as a uint8 instead of a
uint16

> - OptionLen = (UINT8)((*Option + 1) * 8 - 2);
> + OptionLen = IP6_HDR_EXT_LEN (*Option) -
IP6_COMBINED_SIZE_OF_NEXT_HDR_AND_LEN;

Additionally this check adds additional logic to santize the incoming
data

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

Signed-off-by: Doug Flick [MSFT] <[email protected]>
Flickdm added a commit to Flickdm/edk2 that referenced this issue Jan 25, 2024
REF:https://bugzilla.tianocore.org/show_bug.cgi?id=4537
REF:https://bugzilla.tianocore.org/show_bug.cgi?id=4538

Bug Details:
PixieFail Bug tianocore#4
CVE-2023-45232
CVSS 7.5 : CVSS:3.1/AV:N/AC:L/PR:N/UI:N/S:U/C:N/I:N/A:H
CWE-835 Loop with Unreachable Exit Condition ('Infinite Loop')

Infinite loop when parsing unknown options in the Destination Options
header

PixieFail Bug tianocore#5
CVE-2023-45233
CVSS 7.5 : CVSS:3.1/AV:N/AC:L/PR:N/UI:N/S:U/C:N/I:N/A:H
CWE-835 Loop with Unreachable Exit Condition ('Infinite Loop')

Infinite loop when parsing a PadN option in the Destination Options
header

Change Overview:

Most importantly this change corrects the following incorrect math
and cleans up the code.

>   // It is a PadN option
>   //
> - Offset = (UINT8)(Offset + *(Option + Offset + 1) + 2);
> + OptDataLen = *(IP6_OFFSET_OF_OPT_LEN (Option + Offset));
> + Offset     = IP6_NEXT_OPTION_OFFSET (Offset, OptDataLen);

> case Ip6OptionSkip:
> - Offset = (UINT8)(Offset + *(Option + Offset + 1));
>   OptDataLen = *(IP6_OFFSET_OF_OPT_LEN (Option + Offset));
>   Offset     = IP6_NEXT_OPTION_OFFSET (Offset, OptDataLen);

Additionally, this change also corrects incorrect math where the calling
function was calculating the HDR EXT optionLen as a uint8 instead of a
uint16

> - OptionLen = (UINT8)((*Option + 1) * 8 - 2);
> + OptionLen = IP6_HDR_EXT_LEN (*Option) -
IP6_COMBINED_SIZE_OF_NEXT_HDR_AND_LEN;

Additionally this check adds additional logic to santize the incoming
data

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

Signed-off-by: Doug Flick [MSFT] <[email protected]>
Flickdm added a commit to Flickdm/edk2 that referenced this issue Jan 25, 2024
REF:https://bugzilla.tianocore.org/show_bug.cgi?id=4537
REF:https://bugzilla.tianocore.org/show_bug.cgi?id=4538

Bug Details:
PixieFail Bug tianocore#4
CVE-2023-45232
CVSS 7.5 : CVSS:3.1/AV:N/AC:L/PR:N/UI:N/S:U/C:N/I:N/A:H
CWE-835 Loop with Unreachable Exit Condition ('Infinite Loop')

Infinite loop when parsing unknown options in the Destination Options
header

PixieFail Bug tianocore#5
CVE-2023-45233
CVSS 7.5 : CVSS:3.1/AV:N/AC:L/PR:N/UI:N/S:U/C:N/I:N/A:H
CWE-835 Loop with Unreachable Exit Condition ('Infinite Loop')

Infinite loop when parsing a PadN option in the Destination Options
header

Change Overview:

Most importantly this change corrects the following incorrect math
and cleans up the code.

>   // It is a PadN option
>   //
> - Offset = (UINT8)(Offset + *(Option + Offset + 1) + 2);
> + OptDataLen = ((EFI_IP6_OPTION *)(Option + Offset))->Length;
> + Offset     = IP6_NEXT_OPTION_OFFSET (Offset, OptDataLen);

> case Ip6OptionSkip:
> - Offset = (UINT8)(Offset + *(Option + Offset + 1));
>   OptDataLen = ((EFI_IP6_OPTION *)(Option + Offset))->Length;
>   Offset     = IP6_NEXT_OPTION_OFFSET (Offset, OptDataLen);

Additionally, this change also corrects incorrect math where the calling
function was calculating the HDR EXT optionLen as a uint8 instead of a
uint16

> - OptionLen = (UINT8)((*Option + 1) * 8 - 2);
> + OptionLen = IP6_HDR_EXT_LEN (*Option) -
IP6_COMBINED_SIZE_OF_NEXT_HDR_AND_LEN;

Additionally this check adds additional logic to santize the incoming
data

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

Signed-off-by: Doug Flick [MSFT] <[email protected]>
Flickdm added a commit to Flickdm/edk2 that referenced this issue Jan 25, 2024
REF:https://bugzilla.tianocore.org/show_bug.cgi?id=4537
REF:https://bugzilla.tianocore.org/show_bug.cgi?id=4538

Bug Details:
PixieFail Bug tianocore#4
CVE-2023-45232
CVSS 7.5 : CVSS:3.1/AV:N/AC:L/PR:N/UI:N/S:U/C:N/I:N/A:H
CWE-835 Loop with Unreachable Exit Condition ('Infinite Loop')

Infinite loop when parsing unknown options in the Destination Options
header

PixieFail Bug tianocore#5
CVE-2023-45233
CVSS 7.5 : CVSS:3.1/AV:N/AC:L/PR:N/UI:N/S:U/C:N/I:N/A:H
CWE-835 Loop with Unreachable Exit Condition ('Infinite Loop')

Infinite loop when parsing a PadN option in the Destination Options
header

Change Overview:

Most importantly this change corrects the following incorrect math
and cleans up the code.

>   // It is a PadN option
>   //
> - Offset = (UINT8)(Offset + *(Option + Offset + 1) + 2);
> + OptDataLen = ((EFI_IP6_OPTION *)(Option + Offset))->Length;
> + Offset     = IP6_NEXT_OPTION_OFFSET (Offset, OptDataLen);

> case Ip6OptionSkip:
> - Offset = (UINT8)(Offset + *(Option + Offset + 1));
>   OptDataLen = ((EFI_IP6_OPTION *)(Option + Offset))->Length;
>   Offset     = IP6_NEXT_OPTION_OFFSET (Offset, OptDataLen);

Additionally, this change also corrects incorrect math where the calling
function was calculating the HDR EXT optionLen as a uint8 instead of a
uint16

> - OptionLen = (UINT8)((*Option + 1) * 8 - 2);
> + OptionLen = IP6_HDR_EXT_LEN (*Option) -
IP6_COMBINED_SIZE_OF_NEXT_HDR_AND_LEN;

Additionally this check adds additional logic to santize the incoming
data

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

Signed-off-by: Doug Flick [MSFT] <[email protected]>
Flickdm added a commit to Flickdm/edk2 that referenced this issue Jan 25, 2024
REF:https://bugzilla.tianocore.org/show_bug.cgi?id=4537
REF:https://bugzilla.tianocore.org/show_bug.cgi?id=4538

Bug Details:
PixieFail Bug tianocore#4
CVE-2023-45232
CVSS 7.5 : CVSS:3.1/AV:N/AC:L/PR:N/UI:N/S:U/C:N/I:N/A:H
CWE-835 Loop with Unreachable Exit Condition ('Infinite Loop')

Infinite loop when parsing unknown options in the Destination Options
header

PixieFail Bug tianocore#5
CVE-2023-45233
CVSS 7.5 : CVSS:3.1/AV:N/AC:L/PR:N/UI:N/S:U/C:N/I:N/A:H
CWE-835 Loop with Unreachable Exit Condition ('Infinite Loop')

Infinite loop when parsing a PadN option in the Destination Options
header

Change Overview:

Most importantly this change corrects the following incorrect math
and cleans up the code.

>   // It is a PadN option
>   //
> - Offset = (UINT8)(Offset + *(Option + Offset + 1) + 2);
> + OptDataLen = ((EFI_IP6_OPTION *)(Option + Offset))->Length;
> + Offset     = IP6_NEXT_OPTION_OFFSET (Offset, OptDataLen);

> case Ip6OptionSkip:
> - Offset = (UINT8)(Offset + *(Option + Offset + 1));
>   OptDataLen = ((EFI_IP6_OPTION *)(Option + Offset))->Length;
>   Offset     = IP6_NEXT_OPTION_OFFSET (Offset, OptDataLen);

Additionally, this change also corrects incorrect math where the calling
function was calculating the HDR EXT optionLen as a uint8 instead of a
uint16

> - OptionLen = (UINT8)((*Option + 1) * 8 - 2);
> + OptionLen = IP6_HDR_EXT_LEN (*Option) -
IP6_COMBINED_SIZE_OF_NEXT_HDR_AND_LEN;

Additionally this check adds additional logic to santize the incoming
data

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

Signed-off-by: Doug Flick [MSFT] <[email protected]>
Flickdm added a commit to Flickdm/edk2 that referenced this issue Jan 25, 2024
REF:https://bugzilla.tianocore.org/show_bug.cgi?id=4537
REF:https://bugzilla.tianocore.org/show_bug.cgi?id=4538

Bug Details:
PixieFail Bug tianocore#4
CVE-2023-45232
CVSS 7.5 : CVSS:3.1/AV:N/AC:L/PR:N/UI:N/S:U/C:N/I:N/A:H
CWE-835 Loop with Unreachable Exit Condition ('Infinite Loop')

Infinite loop when parsing unknown options in the Destination Options
header

PixieFail Bug tianocore#5
CVE-2023-45233
CVSS 7.5 : CVSS:3.1/AV:N/AC:L/PR:N/UI:N/S:U/C:N/I:N/A:H
CWE-835 Loop with Unreachable Exit Condition ('Infinite Loop')

Infinite loop when parsing a PadN option in the Destination Options
header

Change Overview:

Most importantly this change corrects the following incorrect math
and cleans up the code.

>   // It is a PadN option
>   //
> - Offset = (UINT8)(Offset + *(Option + Offset + 1) + 2);
> + OptDataLen = ((EFI_IP6_OPTION *)(Option + Offset))->Length;
> + Offset     = IP6_NEXT_OPTION_OFFSET (Offset, OptDataLen);

> case Ip6OptionSkip:
> - Offset = (UINT8)(Offset + *(Option + Offset + 1));
>   OptDataLen = ((EFI_IP6_OPTION *)(Option + Offset))->Length;
>   Offset     = IP6_NEXT_OPTION_OFFSET (Offset, OptDataLen);

Additionally, this change also corrects incorrect math where the calling
function was calculating the HDR EXT optionLen as a uint8 instead of a
uint16

> - OptionLen = (UINT8)((*Option + 1) * 8 - 2);
> + OptionLen = IP6_HDR_EXT_LEN (*Option) -
IP6_COMBINED_SIZE_OF_NEXT_HDR_AND_LEN;

Additionally this check adds additional logic to santize the incoming
data

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

Signed-off-by: Doug Flick [MSFT] <[email protected]>
lgao4 pushed a commit to lgao4/edk2 that referenced this issue Feb 6, 2024
REF:https://bugzilla.tianocore.org/show_bug.cgi?id=4537
REF:https://bugzilla.tianocore.org/show_bug.cgi?id=4538

Bug Details:
PixieFail Bug tianocore#4
CVE-2023-45232
CVSS 7.5 : CVSS:3.1/AV:N/AC:L/PR:N/UI:N/S:U/C:N/I:N/A:H
CWE-835 Loop with Unreachable Exit Condition ('Infinite Loop')

Infinite loop when parsing unknown options in the Destination Options
header

PixieFail Bug tianocore#5
CVE-2023-45233
CVSS 7.5 : CVSS:3.1/AV:N/AC:L/PR:N/UI:N/S:U/C:N/I:N/A:H
CWE-835 Loop with Unreachable Exit Condition ('Infinite Loop')

Infinite loop when parsing a PadN option in the Destination Options
header

Change Overview:

Most importantly this change corrects the following incorrect math
and cleans up the code.

>   // It is a PadN option
>   //
> - Offset = (UINT8)(Offset + *(Option + Offset + 1) + 2);
> + OptDataLen = ((EFI_IP6_OPTION *)(Option + Offset))->Length;
> + Offset     = IP6_NEXT_OPTION_OFFSET (Offset, OptDataLen);

> case Ip6OptionSkip:
> - Offset = (UINT8)(Offset + *(Option + Offset + 1));
>   OptDataLen = ((EFI_IP6_OPTION *)(Option + Offset))->Length;
>   Offset     = IP6_NEXT_OPTION_OFFSET (Offset, OptDataLen);

Additionally, this change also corrects incorrect math where the calling
function was calculating the HDR EXT optionLen as a uint8 instead of a
uint16

> - OptionLen = (UINT8)((*Option + 1) * 8 - 2);
> + OptionLen = IP6_HDR_EXT_LEN (*Option) -
IP6_COMBINED_SIZE_OF_NEXT_HDR_AND_LEN;

Additionally this check adds additional logic to santize the incoming
data

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]>
mergify bot pushed a commit that referenced this issue Feb 6, 2024
REF:https://bugzilla.tianocore.org/show_bug.cgi?id=4537
REF:https://bugzilla.tianocore.org/show_bug.cgi?id=4538

Bug Details:
PixieFail Bug #4
CVE-2023-45232
CVSS 7.5 : CVSS:3.1/AV:N/AC:L/PR:N/UI:N/S:U/C:N/I:N/A:H
CWE-835 Loop with Unreachable Exit Condition ('Infinite Loop')

Infinite loop when parsing unknown options in the Destination Options
header

PixieFail Bug #5
CVE-2023-45233
CVSS 7.5 : CVSS:3.1/AV:N/AC:L/PR:N/UI:N/S:U/C:N/I:N/A:H
CWE-835 Loop with Unreachable Exit Condition ('Infinite Loop')

Infinite loop when parsing a PadN option in the Destination Options
header

Change Overview:

Most importantly this change corrects the following incorrect math
and cleans up the code.

>   // It is a PadN option
>   //
> - Offset = (UINT8)(Offset + *(Option + Offset + 1) + 2);
> + OptDataLen = ((EFI_IP6_OPTION *)(Option + Offset))->Length;
> + Offset     = IP6_NEXT_OPTION_OFFSET (Offset, OptDataLen);

> case Ip6OptionSkip:
> - Offset = (UINT8)(Offset + *(Option + Offset + 1));
>   OptDataLen = ((EFI_IP6_OPTION *)(Option + Offset))->Length;
>   Offset     = IP6_NEXT_OPTION_OFFSET (Offset, OptDataLen);

Additionally, this change also corrects incorrect math where the calling
function was calculating the HDR EXT optionLen as a uint8 instead of a
uint16

> - OptionLen = (UINT8)((*Option + 1) * 8 - 2);
> + OptionLen = IP6_HDR_EXT_LEN (*Option) -
IP6_COMBINED_SIZE_OF_NEXT_HDR_AND_LEN;

Additionally this check adds additional logic to santize the incoming
data

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]>
nicklela pushed a commit to changab/edk2 that referenced this issue Mar 25, 2024
REF:https://bugzilla.tianocore.org/show_bug.cgi?id=4537
REF:https://bugzilla.tianocore.org/show_bug.cgi?id=4538

Bug Details:
PixieFail Bug tianocore#4
CVE-2023-45232
CVSS 7.5 : CVSS:3.1/AV:N/AC:L/PR:N/UI:N/S:U/C:N/I:N/A:H
CWE-835 Loop with Unreachable Exit Condition ('Infinite Loop')

Infinite loop when parsing unknown options in the Destination Options
header

PixieFail Bug tianocore#5
CVE-2023-45233
CVSS 7.5 : CVSS:3.1/AV:N/AC:L/PR:N/UI:N/S:U/C:N/I:N/A:H
CWE-835 Loop with Unreachable Exit Condition ('Infinite Loop')

Infinite loop when parsing a PadN option in the Destination Options
header

Change Overview:

Most importantly this change corrects the following incorrect math
and cleans up the code.

>   // It is a PadN option
>   //
> - Offset = (UINT8)(Offset + *(Option + Offset + 1) + 2);
> + OptDataLen = ((EFI_IP6_OPTION *)(Option + Offset))->Length;
> + Offset     = IP6_NEXT_OPTION_OFFSET (Offset, OptDataLen);

> case Ip6OptionSkip:
> - Offset = (UINT8)(Offset + *(Option + Offset + 1));
>   OptDataLen = ((EFI_IP6_OPTION *)(Option + Offset))->Length;
>   Offset     = IP6_NEXT_OPTION_OFFSET (Offset, OptDataLen);

Additionally, this change also corrects incorrect math where the calling
function was calculating the HDR EXT optionLen as a uint8 instead of a
uint16

> - OptionLen = (UINT8)((*Option + 1) * 8 - 2);
> + OptionLen = IP6_HDR_EXT_LEN (*Option) -
IP6_COMBINED_SIZE_OF_NEXT_HDR_AND_LEN;

Additionally this check adds additional logic to santize the incoming
data

Bug 4457168

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]>
Change-Id: Ice9de525a1908fa268456148260d9a8c8229bcab
Reviewed-on: https://git-master.nvidia.com/r/c/3rdparty/edk2/+/3073486
Reviewed-by: svcacv <[email protected]>
Reviewed-by: svc-sw-mobile-l4t <[email protected]>
Reviewed-by: Jeff Brasen <[email protected]>
GVS: Gerrit_Virtual_Submit <[email protected]>
Tested-by: Jeff Brasen <[email protected]>
jiaxinwu added a commit to jiaxinwu/edk2 that referenced this issue Oct 12, 2024
This patch does not impact functionality. It aims to clarify the
synchronization flow between the BSP and APs to enhance code
readability and understanding:

Steps tianocore#6 and tianocore#11 are the basic synchronization requirements for all
cases.

Steps tianocore#1 and tianocore#2 are additional requirements if the
MmCpuSyncModeTradition mode is selected.

Steps tianocore#1, tianocore#2, tianocore#3, tianocore#4, tianocore#5, tianocore#7, tianocore#8, tianocore#9, and tianocore#10 are additional
requirements if the system needs to configure the MTRR.

Steps tianocore#9 and tianocore#10 are additional requirements if the system needs to
support the mSmmDebugAgentSupport.

Signed-off-by: Jiaxin Wu <[email protected]>
jiaxinwu added a commit to jiaxinwu/edk2 that referenced this issue Oct 12, 2024
This patch does not impact functionality. It aims to clarify the
synchronization flow between the BSP and APs to enhance code
readability and understanding:

Steps tianocore#6 and tianocore#11 are the basic synchronization requirements for all
cases.

Steps tianocore#1 and tianocore#2 are additional requirements if the
MmCpuSyncModeTradition mode is selected.

Steps tianocore#1, tianocore#2, tianocore#3, tianocore#4, tianocore#5, tianocore#7, tianocore#8, tianocore#9, and tianocore#10 are additional
requirements if the system needs to configure the MTRR.

Steps tianocore#9 and tianocore#10 are additional requirements if the system needs to
support the mSmmDebugAgentSupport.

Signed-off-by: Jiaxin Wu <[email protected]>
jiaxinwu added a commit to jiaxinwu/edk2 that referenced this issue Oct 12, 2024
This patch does not impact functionality. It aims to clarify the
synchronization flow between the BSP and APs to enhance code
readability and understanding:

Steps tianocore#6 and tianocore#11 are the basic synchronization requirements for all
cases.

Steps tianocore#1 is additional requirements if the MmCpuSyncModeTradition
mode is selected.

Steps tianocore#1, tianocore#2, tianocore#3, tianocore#4, tianocore#5, tianocore#7, tianocore#8, tianocore#9, and tianocore#10 are additional
requirements if the system needs to configure the MTRR.

Steps tianocore#9 and tianocore#10 are additional requirements if the system needs to
support the mSmmDebugAgentSupport.

Signed-off-by: Jiaxin Wu <[email protected]>
jiaxinwu added a commit to jiaxinwu/edk2 that referenced this issue Oct 12, 2024
This patch does not impact functionality. It aims to clarify the
synchronization flow between the BSP and APs to enhance code
readability and understanding:

Steps tianocore#6 and tianocore#11 are the basic synchronization requirements for all
cases.

Steps tianocore#1 is additional requirements if the MmCpuSyncModeTradition
mode is selected.

Steps tianocore#1, tianocore#2, tianocore#3, tianocore#4, tianocore#5, tianocore#7, tianocore#8, tianocore#9, and tianocore#10 are additional
requirements if the system needs to configure the MTRR.

Steps tianocore#9 and tianocore#10 are additional requirements if the system needs to
support the mSmmDebugAgentSupport.

Signed-off-by: Jiaxin Wu <[email protected]>
mergify bot pushed a commit that referenced this issue Oct 12, 2024
This patch does not impact functionality. It aims to clarify the
synchronization flow between the BSP and APs to enhance code
readability and understanding:

Steps #6 and #11 are the basic synchronization requirements for all
cases.

Steps #1 is additional requirements if the MmCpuSyncModeTradition
mode is selected.

Steps #1, #2, #3, #4, #5, #7, #8, #9, and #10 are additional
requirements if the system needs to configure the MTRR.

Steps #9 and #10 are additional requirements if the system needs to
support the mSmmDebugAgentSupport.

Signed-off-by: Jiaxin Wu <[email protected]>
ishih1 pushed a commit to ishih1/edk2 that referenced this issue Nov 11, 2024
This patch does not impact functionality. It aims to clarify the
synchronization flow between the BSP and APs to enhance code
readability and understanding:

Steps tianocore#6 and tianocore#11 are the basic synchronization requirements for all
cases.

Steps tianocore#1 is additional requirements if the MmCpuSyncModeTradition
mode is selected.

Steps tianocore#1, tianocore#2, tianocore#3, tianocore#4, tianocore#5, tianocore#7, tianocore#8, tianocore#9, and tianocore#10 are additional
requirements if the system needs to configure the MTRR.

Steps tianocore#9 and tianocore#10 are additional requirements if the system needs to
support the mSmmDebugAgentSupport.

Signed-off-by: Jiaxin Wu <[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

4 participants