On Wed, Oct 18, 2023 at 6:24 PM Laszlo Ersek <ler...@redhat.com> wrote: > > 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] https://github.com/rhboot/shim/issues/382 > > Cc: Ard Biesheuvel <ardb+tianoc...@kernel.org> > Cc: Gerd Hoffmann <kra...@redhat.com> > Cc: Jiewen Yao <jiewen....@intel.com> > Cc: Jordan Justen <jordan.l.jus...@intel.com> > Signed-off-by: Laszlo Ersek <ler...@redhat.com> > --- > > Notes: > context:-U4 > > OvmfPkg/VirtioFsDxe/SimpleFsOpen.c | 17 ++++++++++++++--- > 1 file changed, 14 insertions(+), 3 deletions(-) > > diff --git a/OvmfPkg/VirtioFsDxe/SimpleFsOpen.c > b/OvmfPkg/VirtioFsDxe/SimpleFsOpen.c > index a13d4f6a1e2d..2ecf3d6c2325 100644 > --- a/OvmfPkg/VirtioFsDxe/SimpleFsOpen.c > +++ b/OvmfPkg/VirtioFsDxe/SimpleFsOpen.c > @@ -394,22 +394,33 @@ VirtioFsSimpleFileOpen ( > > // > // 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). > + // relative to a directory). See USWG Mantis ticket #2367. > // > if (!VirtioFsFile->IsDirectory) { > + BOOLEAN BugCompat; > + > + // > + // Tolerate this bug in the caller if FileName is absolute. If FileName > is > + // absolute, then VirtioFsAppendPath() below will disregard > + // VirtioFsFile->CanonicalPathname. > + // > + BugCompat = (FileName[0] == L'\\'); > + > DEBUG (( > - DEBUG_ERROR, > + BugCompat ? DEBUG_WARN : DEBUG_ERROR, > ("%a: Label=\"%s\" CanonicalPathname=\"%a\" FileName=\"%s\": " > "nonsensical request to open a file or directory relative to a > regular " > "file\n"), > __func__, > VirtioFs->Label, > VirtioFsFile->CanonicalPathname, > FileName > )); > - return EFI_INVALID_PARAMETER; > + if (!BugCompat) { > + return EFI_INVALID_PARAMETER; > + } > } > > // > // Allocate the new VIRTIO_FS_FILE object. >
Aww, you should've CC'd me. Anyway, retroactive Acked-by: Pedro Falcato <pedro.falc...@gmail.com> If this is the new pseudo-sanctioned behavior for filesystem drivers, I'll make sure to do the same adjustments for Ext4Dxe. -- Pedro -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#109805): https://edk2.groups.io/g/devel/message/109805 Mute This Topic: https://groups.io/mt/102044004/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
