[PATCH] Update FvLength to UINTN from UINT32 in FirmwareVolumeInfo #2367
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Pi/PiFirmwareVolume FvLength has been assigned as UINT64, and Ppi/FirmwareVolumeInfo FvLength has mentioned as UINT32 which will break the X64 build, updating the FvLenth to UINTN will support for both Cc: Rangasai V Chaganty <rangasai.v.chaganty@intel.com> Cc: Digant H Solanki <digant.h.solanki@intel.com> Cc: Sangeetha V <sangeetha.v@intel.com> Cc: Ray Ni <ray.ni@intel.com> Signed-off-by: Ashraf Ali S <ashraf.ali.s@intel.com>
lersek
added a commit
to lersek/edk2
that referenced
this pull request
Oct 19, 2023
… file Referring to a file relative to a regular file makes no sense (or at least it cannot be implemented consistently with how a file is referred to relative to a directory). VirtioFsSimpleFileOpen() has enforced this strictly since the beginning, and a few months ago I reported USWG Mantis ticket tianocore#2367 [1] too, for clearing up the related confusion in the UEFI spec. Unfortunately, the shim boot loader contains such a bug [2] [3]. I don't believe the shim bug is ever going to be fixed. We can however relax the check in VirtioFsSimpleFileOpen() a bit: if the pathname that's being opened relative to a regular file is absolute, then the base file is going to be ignored anyway, so we can let the caller's bug slide. This happens to make shim work. Why this matters: UEFI-bootable Linux installer ISOs tend to come with shim and grub in the embedded (ElTorito) FAT image (ESP). Sometimes you want to build upstream shim/grub binaries, but boot the same ISO otherwise. The fastest way for overriding the ESP for this purpose is to copy its original contents to a virtio filesystem, then overwrite the shim and grub binaries from the host side. Note that this is different from direct-booting a kernel (via fw_cfg); the point is to check whether the just-built shim and grub are able to boot the rest of the ISO. [1] https://mantis.uefi.org/mantis/view.php?id=2367 [2] https://bugzilla.redhat.com/show_bug.cgi?id=1966973 [3] rhboot/shim#382 Cc: Ard Biesheuvel <ardb+tianocore@kernel.org> Cc: Gerd Hoffmann <kraxel@redhat.com> Cc: Jiewen Yao <jiewen.yao@intel.com> Cc: Jordan Justen <jordan.l.justen@intel.com> Signed-off-by: Laszlo Ersek <lersek@redhat.com> Message-Id: <20231018172434.91280-1-lersek@redhat.com> Reviewed-by: Gerd Hoffmann <kraxel@redhat.com> Tested-by: Gerd Hoffmann <kraxel@redhat.com>
mergify bot
pushed a commit
that referenced
this pull request
Oct 19, 2023
… file Referring to a file relative to a regular file makes no sense (or at least it cannot be implemented consistently with how a file is referred to relative to a directory). VirtioFsSimpleFileOpen() has enforced this strictly since the beginning, and a few months ago I reported USWG Mantis ticket #2367 [1] too, for clearing up the related confusion in the UEFI spec. Unfortunately, the shim boot loader contains such a bug [2] [3]. I don't believe the shim bug is ever going to be fixed. We can however relax the check in VirtioFsSimpleFileOpen() a bit: if the pathname that's being opened relative to a regular file is absolute, then the base file is going to be ignored anyway, so we can let the caller's bug slide. This happens to make shim work. Why this matters: UEFI-bootable Linux installer ISOs tend to come with shim and grub in the embedded (ElTorito) FAT image (ESP). Sometimes you want to build upstream shim/grub binaries, but boot the same ISO otherwise. The fastest way for overriding the ESP for this purpose is to copy its original contents to a virtio filesystem, then overwrite the shim and grub binaries from the host side. Note that this is different from direct-booting a kernel (via fw_cfg); the point is to check whether the just-built shim and grub are able to boot the rest of the ISO. [1] https://mantis.uefi.org/mantis/view.php?id=2367 [2] https://bugzilla.redhat.com/show_bug.cgi?id=1966973 [3] rhboot/shim#382 Cc: Ard Biesheuvel <ardb+tianocore@kernel.org> Cc: Gerd Hoffmann <kraxel@redhat.com> Cc: Jiewen Yao <jiewen.yao@intel.com> Cc: Jordan Justen <jordan.l.justen@intel.com> Signed-off-by: Laszlo Ersek <lersek@redhat.com> Message-Id: <20231018172434.91280-1-lersek@redhat.com> Reviewed-by: Gerd Hoffmann <kraxel@redhat.com> Tested-by: Gerd Hoffmann <kraxel@redhat.com>
mmisono
pushed a commit
to mmisono/edk2
that referenced
this pull request
Aug 30, 2024
… file Referring to a file relative to a regular file makes no sense (or at least it cannot be implemented consistently with how a file is referred to relative to a directory). VirtioFsSimpleFileOpen() has enforced this strictly since the beginning, and a few months ago I reported USWG Mantis ticket tianocore#2367 [1] too, for clearing up the related confusion in the UEFI spec. Unfortunately, the shim boot loader contains such a bug [2] [3]. I don't believe the shim bug is ever going to be fixed. We can however relax the check in VirtioFsSimpleFileOpen() a bit: if the pathname that's being opened relative to a regular file is absolute, then the base file is going to be ignored anyway, so we can let the caller's bug slide. This happens to make shim work. Why this matters: UEFI-bootable Linux installer ISOs tend to come with shim and grub in the embedded (ElTorito) FAT image (ESP). Sometimes you want to build upstream shim/grub binaries, but boot the same ISO otherwise. The fastest way for overriding the ESP for this purpose is to copy its original contents to a virtio filesystem, then overwrite the shim and grub binaries from the host side. Note that this is different from direct-booting a kernel (via fw_cfg); the point is to check whether the just-built shim and grub are able to boot the rest of the ISO. [1] https://mantis.uefi.org/mantis/view.php?id=2367 [2] https://bugzilla.redhat.com/show_bug.cgi?id=1966973 [3] rhboot/shim#382 Cc: Ard Biesheuvel <ardb+tianocore@kernel.org> Cc: Gerd Hoffmann <kraxel@redhat.com> Cc: Jiewen Yao <jiewen.yao@intel.com> Cc: Jordan Justen <jordan.l.justen@intel.com> Signed-off-by: Laszlo Ersek <lersek@redhat.com> Message-Id: <20231018172434.91280-1-lersek@redhat.com> Reviewed-by: Gerd Hoffmann <kraxel@redhat.com> Tested-by: Gerd Hoffmann <kraxel@redhat.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Pi/PiFirmwareVolume FvLength has been assigned as UINT64, and
Ppi/FirmwareVolumeInfo FvLength has mentioned as UINT32 which will
break the X64 build, updating the FvLenth to UINTN will support for both
Cc: Rangasai V Chaganty rangasai.v.chaganty@intel.com
Cc: Digant H Solanki digant.h.solanki@intel.com
Cc: Sangeetha V sangeetha.v@intel.com
Cc: Ray Ni ray.ni@intel.com
Signed-off-by: Ashraf Ali S ashraf.ali.s@intel.com