On Wed, Oct 18, 2023 at 12:20 PM Laszlo Ersek <ler...@redhat.com> wrote:
>
> On 10/18/23 12:33, Gerd Hoffmann wrote:
> > VirtiofsDxe throws an error in case the caller tries to open a file or
> > directory using an handle with is not a directory, claiming that opening
> > something relative to a file does not make sense.
> >
> > The claim is correct, but the code throws errors for both relative and
> > absolute paths. Add a check to fix that.
> >
> > Signed-off-by: Gerd Hoffmann <kra...@redhat.com>
> > ---
> > OvmfPkg/VirtioFsDxe/SimpleFsOpen.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/OvmfPkg/VirtioFsDxe/SimpleFsOpen.c
> > b/OvmfPkg/VirtioFsDxe/SimpleFsOpen.c
> > index a13d4f6a1e2d..1729ea2f5cf2 100644
> > --- a/OvmfPkg/VirtioFsDxe/SimpleFsOpen.c
> > +++ b/OvmfPkg/VirtioFsDxe/SimpleFsOpen.c
> > @@ -397,7 +397,7 @@ VirtioFsSimpleFileOpen (
> > // it cannot be implemented consistently with how a file is referred to
> > // relative to a directory).
> > //
> > - if (!VirtioFsFile->IsDirectory) {
> > + if (!VirtioFsFile->IsDirectory && FileName[0] != '\\') {
> > DEBUG ((
> > DEBUG_ERROR,
> > ("%a: Label=\"%s\" CanonicalPathname=\"%a\" FileName=\"%s\": "
>
> It's nice to see this topic pop up on edk2-devel; apparently you started
> testing shim on top of virtio-fs. :)
>
> I have had the following patch in my local repo, on a separate branch,
> since April this year:
>
> > commit cb4a6d1664ea6cabd14d2af0e5d9abb114973870
> > Author: Laszlo Ersek <ler...@redhat.com>
> > Date: Sat Apr 8 22:50:50 2023 +0200
> >
> > 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
What does the mantis ticket say/conclude? Yay for private bug trackers
that need corporate buy-in...
FWIW, Ext4Dxe does
[...]
if (!Ext4FileIsDir (Current)) {
return EFI_INVALID_PARAMETER;
}
// If the path starts with a backslash, we treat the root directory
as the base directory
if (FileName[0] == L'\\') {
FileName++;
Current = Partition->Root;
}
so if shim/other important UEFI apps have a bug, I may need to fix
this as well...
--
Pedro
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#109732): https://edk2.groups.io/g/devel/message/109732
Mute This Topic: https://groups.io/mt/102036263/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
