From 8c805565614538863309ef79cc6c5bb63f5263e5 Mon Sep 17 00:00:00 2001 From: Laszlo Ersek Date: Wed, 18 Oct 2023 19:24:34 +0200 Subject: [PATCH] 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 Cc: Gerd Hoffmann Cc: Jiewen Yao Cc: Jordan Justen Signed-off-by: Laszlo Ersek Message-Id: <20231018172434.91280-1-lersek@redhat.com> Reviewed-by: Gerd Hoffmann Tested-by: Gerd Hoffmann --- 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 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; + } } //