OvmfPkg/VirtioFsDxe: tolerate opening an abs. pathname rel. to a reg. 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] https://github.com/rhboot/shim/issues/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>

1 files changed, 14 insertions, 3 deletions

diff --git a/OvmfPkg/VirtioFsDxe/SimpleFsOpen.c b/OvmfPkg/VirtioFsDxe/SimpleFsOpen.c
index a13d4f6a1e..2ecf3d6c23 100644
--- a/OvmfPkg/VirtioFsDxe/SimpleFsOpen.c
+++ b/OvmfPkg/VirtioFsDxe/SimpleFsOpen.c
@@ -395,11 +395,20 @@ 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"),

@@ -408,7 +417,9 @@ VirtioFsSimpleFileOpen (
       VirtioFsFile->CanonicalPathname,

       FileName

       ));

-    return EFI_INVALID_PARAMETER;

+    if (!BugCompat) {

+      return EFI_INVALID_PARAMETER;

+    }

   }

 

   //