* [edk2-devel] [PATCH 1/1] OvmfPkg/VirtioFsDxe: fix SimpleFileOpen @ 2023-10-18 10:33 Gerd Hoffmann 2023-10-18 11:20 ` Laszlo Ersek 0 siblings, 1 reply; 8+ messages in thread From: Gerd Hoffmann @ 2023-10-18 10:33 UTC (permalink / raw) To: devel Cc: Jordan Justen, Gerd Hoffmann, Ard Biesheuvel, László Érsek, Oliver Steffen, Jiewen Yao 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 <kraxel@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\": " -- 2.41.0 -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#109727): https://edk2.groups.io/g/devel/message/109727 Mute This Topic: https://groups.io/mt/102036263/7686176 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io] -=-=-=-=-=-=-=-=-=-=-=- ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [edk2-devel] [PATCH 1/1] OvmfPkg/VirtioFsDxe: fix SimpleFileOpen 2023-10-18 10:33 [edk2-devel] [PATCH 1/1] OvmfPkg/VirtioFsDxe: fix SimpleFileOpen Gerd Hoffmann @ 2023-10-18 11:20 ` Laszlo Ersek 2023-10-18 11:33 ` Pedro Falcato 2023-10-18 13:13 ` Gerd Hoffmann 0 siblings, 2 replies; 8+ messages in thread From: Laszlo Ersek @ 2023-10-18 11:20 UTC (permalink / raw) To: Gerd Hoffmann, devel Cc: Jordan Justen, Ard Biesheuvel, Oliver Steffen, Jiewen Yao 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 <kraxel@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 <lersek@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 > [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> > > Notes: > context:-U4 > > diff --git a/OvmfPkg/VirtioFsDxe/SimpleFsOpen.c b/OvmfPkg/VirtioFsDxe/SimpleFsOpen.c > index d479f76f5bc3..ec0521ac3703 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"), > __FUNCTION__, > VirtioFs->Label, > VirtioFsFile->CanonicalPathname, > FileName > )); > - return EFI_INVALID_PARAMETER; > + if (!BugCompat) { > + return EFI_INVALID_PARAMETER; > + } > } > > // > // Allocate the new VIRTIO_FS_FILE object. Note that I'm adamant that this is a shim (and UEFI spec) bug, and that the current upstream code is right, *regardless* of whether the pathname to open starts with a backslash or not. The spec bug is reference [1] above, and the original incarnation of my shim bug report is reference [2]. Reference [3] is just the original RHBZ [2] having been migrated / copied to the upstream tracker. In other words, the patch is expressly a bug-compat patch. There are two reasons why I never posted the patch: (1) The (non-)treatment I received from the shim maintainers in ticket <https://github.com/rhboot/shim/issues/382> discouraged me from doing anything with, or for, shim. (2) With this modification in place, shim is happy, but grub isn't. When I realized that, I looked relatively deeply into making grub work on top of virtio-fs as well -- and my findings were horrendous. I wrote up my findings in a private email to some colleagues; you were among the recipients. The Message-Id of that email is <244b4c0f-8c79-7cd6-193e-54046ecf323c@redhat.com>, and the date is "4/19/23, 15:18". My main statement in that email was that grub2 was *architecturally incompatible* with UEFI, and I added: > What I mean by architecturally incompatible: grub2 is designed from a > perspective where it thinks it is an operating system; in other words, > that it *owns* the computer. The problem is that UEFI thinks the exact > same thing of itself, and of course the two conflict. I elaborated a great detail on that, providing various examples, in particular in relation to how grub used EFI_SIMPLE_FILE_SYSTEM_PROTOCOL. Those findings were what made me deem my shim bugcompat patch futile, after all -- I didn't post the patch because grub was unsalvageable anyway, so I didn't see the point. If you have a use case where you rely on shim but *not* on Grub (UKIs?), then I'm OK relaxing the strictness of VirtioFsDxe. In that case, I'd prefer upstreaming my above patch, from April, rather than taking yours. What do you think about that? --*-- Here's a further (independent) caveat: if you are using VirtioFsDxe with the rust language virtiofsd, then you might experience hangs in VirtioFsInit. For fixing that, you need the following *qemu* patch set: [PATCH v3 0/7] vhost-user: call VHOST_USER_SET_VRING_ENABLE synchronously https://patchew.org/QEMU/20231002203221.17241-1-lersek@redhat.com/ (This patch set has been on qemu-devel for nearly 2 months now, counting from v1; I'm going to ping MST again. It's been ready for merging for weeks now!) Thanks! Laszlo -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#109729): https://edk2.groups.io/g/devel/message/109729 Mute This Topic: https://groups.io/mt/102036263/7686176 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/leave/12367111/7686176/1913456212/xyzzy [rebecca@openfw.io] -=-=-=-=-=-=-=-=-=-=-=- ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [edk2-devel] [PATCH 1/1] OvmfPkg/VirtioFsDxe: fix SimpleFileOpen 2023-10-18 11:20 ` Laszlo Ersek @ 2023-10-18 11:33 ` Pedro Falcato 2023-10-18 12:20 ` Laszlo Ersek 2023-10-18 13:13 ` Gerd Hoffmann 1 sibling, 1 reply; 8+ messages in thread From: Pedro Falcato @ 2023-10-18 11:33 UTC (permalink / raw) To: devel, lersek Cc: Gerd Hoffmann, Jordan Justen, Ard Biesheuvel, Oliver Steffen, Jiewen Yao, Marvin Häuser On Wed, Oct 18, 2023 at 12:20 PM Laszlo Ersek <lersek@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 <kraxel@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 <lersek@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/7686176 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io] -=-=-=-=-=-=-=-=-=-=-=- ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [edk2-devel] [PATCH 1/1] OvmfPkg/VirtioFsDxe: fix SimpleFileOpen 2023-10-18 11:33 ` Pedro Falcato @ 2023-10-18 12:20 ` Laszlo Ersek 2023-10-18 13:08 ` Pedro Falcato 0 siblings, 1 reply; 8+ messages in thread From: Laszlo Ersek @ 2023-10-18 12:20 UTC (permalink / raw) To: Pedro Falcato, devel Cc: Gerd Hoffmann, Jordan Justen, Ard Biesheuvel, Oliver Steffen, Jiewen Yao, Marvin Häuser On 10/18/23 13:33, Pedro Falcato wrote: > On Wed, Oct 18, 2023 at 12:20 PM Laszlo Ersek <lersek@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 <kraxel@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 <lersek@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... I posted patches for the UEFI spec. (In two formats -- as a pull request to the locked-down repository on github.com, and as attachments.) Kevin W Shaw started reviewing my patches, but he seemed to misunderstand the git patch format in general; so I commented on that, but then the thread petered out. So it's stuck at the moment. I guess I could try to join USWG meetings / calls and champion the issue there, but I had not had time for that for a decade, and I don't have it now. I'd hope we could communicate asynchronously, via bug trackers... > > 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... > I vaguely remember that I looked up both Ext4Dxe and EnhancedFatDxe regarding this question, but I don't remember what my take-away was at the time. :) Clearly, EnhancedFatDxe must be tolerant of this bug, otherwise shim would never boot off of "normal" (i.e., FAT32) ESPs. Laszlo -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#109741): https://edk2.groups.io/g/devel/message/109741 Mute This Topic: https://groups.io/mt/102036263/7686176 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/leave/12367111/7686176/1913456212/xyzzy [rebecca@openfw.io] -=-=-=-=-=-=-=-=-=-=-=- ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [edk2-devel] [PATCH 1/1] OvmfPkg/VirtioFsDxe: fix SimpleFileOpen 2023-10-18 12:20 ` Laszlo Ersek @ 2023-10-18 13:08 ` Pedro Falcato 2023-10-18 14:03 ` Laszlo Ersek 0 siblings, 1 reply; 8+ messages in thread From: Pedro Falcato @ 2023-10-18 13:08 UTC (permalink / raw) To: devel, lersek Cc: Gerd Hoffmann, Jordan Justen, Ard Biesheuvel, Oliver Steffen, Jiewen Yao, Marvin Häuser On Wed, Oct 18, 2023 at 1:20 PM Laszlo Ersek <lersek@redhat.com> wrote: > > On 10/18/23 13:33, Pedro Falcato wrote: > > On Wed, Oct 18, 2023 at 12:20 PM Laszlo Ersek <lersek@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 <kraxel@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 <lersek@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... > > I posted patches for the UEFI spec. (In two formats -- as a pull request > to the locked-down repository on github.com, and as attachments.) > > Kevin W Shaw started reviewing my patches, but he seemed to > misunderstand the git patch format in general; so I commented on that, > but then the thread petered out. So it's stuck at the moment. > > I guess I could try to join USWG meetings / calls and champion the issue > there, but I had not had time for that for a decade, and I don't have it > now. I'd hope we could communicate asynchronously, via bug trackers... > Ok so I took some time to re-read the context and the spec. I think you're going about things the wrong way. The act of tolerating bad base "directories" in open calls is nothing new. For instance, in POSIX: int openat(int fd, const char *path, int oflag, ...); [...] The openat() function shall fail if: [...] [ENOTDIR] The path argument is not an absolute path and fd is a file descriptor associated with a non-directory file. so there's some track record in POSIX (in fact, openat is extra tolerant and can also take bad file descriptors if the path is absolute). Given that there may be software out there that does rely on this, it sure sounds like a better idea to be looser here, both in-driver and in-spec. What problem did GRUB have with this? GRUB freaking out with your patch sure does seem like it would freak out with any run-of-the-mill EnhancedFatDxe? (FWIW, I've just checked and EnhancedFatDxe does not seem to even check if the EFI_FILE is a directory, ever :)) -- Pedro -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#109745): https://edk2.groups.io/g/devel/message/109745 Mute This Topic: https://groups.io/mt/102036263/7686176 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io] -=-=-=-=-=-=-=-=-=-=-=- ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [edk2-devel] [PATCH 1/1] OvmfPkg/VirtioFsDxe: fix SimpleFileOpen 2023-10-18 13:08 ` Pedro Falcato @ 2023-10-18 14:03 ` Laszlo Ersek 0 siblings, 0 replies; 8+ messages in thread From: Laszlo Ersek @ 2023-10-18 14:03 UTC (permalink / raw) To: Pedro Falcato, devel Cc: Gerd Hoffmann, Jordan Justen, Ard Biesheuvel, Oliver Steffen, Jiewen Yao, Marvin Häuser On 10/18/23 15:08, Pedro Falcato wrote: > On Wed, Oct 18, 2023 at 1:20 PM Laszlo Ersek <lersek@redhat.com> wrote: >> >> On 10/18/23 13:33, Pedro Falcato wrote: >>> On Wed, Oct 18, 2023 at 12:20 PM Laszlo Ersek <lersek@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 <kraxel@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 <lersek@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... >> >> I posted patches for the UEFI spec. (In two formats -- as a pull request >> to the locked-down repository on github.com, and as attachments.) >> >> Kevin W Shaw started reviewing my patches, but he seemed to >> misunderstand the git patch format in general; so I commented on that, >> but then the thread petered out. So it's stuck at the moment. >> >> I guess I could try to join USWG meetings / calls and champion the issue >> there, but I had not had time for that for a decade, and I don't have it >> now. I'd hope we could communicate asynchronously, via bug trackers... >> > > Ok so I took some time to re-read the context and the spec. I think > you're going about things the wrong way. > The act of tolerating bad base "directories" in open calls is nothing > new. For instance, in POSIX: > > int openat(int fd, const char *path, int oflag, ...); > > [...] > The openat() function shall fail if: > [...] > [ENOTDIR] The path argument is not an absolute path and fd is a file > descriptor associated with a non-directory file. > > so there's some track record in POSIX (in fact, openat is extra > tolerant and can also take bad file descriptors if the path is > absolute). > > Given that there may be software out there that does rely on this, it > sure sounds like a better idea to be looser here, both in-driver and > in-spec. I agree about being looser, as long as we label the change properly. And by properly, I mean "bug compat". I maintain that the UEFI spec is buggy -- it is inconsistent. The expression "the location of fh", where "fh" is a file handle, is ill-defined in the UEFI spec. My bug report for it (and my proposed fix for it) *restricts* the spec, turning the shim behavior into a clear-cut bug. (Note that the OpenEx protocol member spec already restricts "This" to be a directory! Which is actually the opposite of what you quote for openat() in POSIX.) We could further tweak the spec to add an exception for absolute pathnames, on top of my proposed restriction. But that's not the first step for fixing the spec. > What problem did GRUB have with this? GRUB freaking out with your > patch sure does seem like it would freak out with any run-of-the-mill > EnhancedFatDxe? Wait, you are conflating things. There is a bug in the UEFI spec and (in parallel) in shim. The bug compat patch makes shim work. Therefore, in a shim+grub guest setup, grub can be *reached*. Then, grub cannot do anything with virtio-fs because it has no direct support for EFI_SIMPLE_FILE_SYSTEM_PROTOCOL. It only has a (very questionable, IMO) block layer abstraction, on top of which it wants to use its own filesystem drivers (IIRC). I had looked into EFI_SIMPLE_FILE_SYSTEM_PROTOCOL enablement for grub, and my verdict was that grub's architecture makes that impossible to do *safely*. I don't want to rehash that investigation there; I did it half a year ago. > > (FWIW, I've just checked and EnhancedFatDxe does not seem to even > check if the EFI_FILE is a directory, ever :)) > EnhancedFatDxe is not scripture... For the record, here's the commit message for my (main) UEFI spec patch: EFI_FILE_PROTOCOL.Open(): restrict "This" to stand for a directory Make sure the Open() and OpenEx() languages consistently restrict "This" to be a directory file handle. Introduce the EFI_INVALID_PARAMETER return value to both functions, for when the restriction is not satisfied by the caller. Update the general description accordingly. [--*--] Consider the following pathname: \EFI\FOO Assume that we have an open file handle (an EFI_FILE_PROTOCOL) that refers to this filesystem object. Now further assume that we open the following pathname: BAR relative to the open file handle. What is the expectation for the full (absolute) pathname of the resultant object? Two choices: \EFI\FOO\BAR [1] \EFI\BAR [2] The first option makes sense if the original file handle, \EFI\FOO, stands for a directory. (That is, "FOO" is a subdirectory of "EFI".) And in that case, the second option is wrong. The second option makes sense, perhaps, if the original file handle, \EFI\FOO, stands for a regular file. (That is, "FOO" is a regular file in the "EFI" directory.) In that case, the first option is simply undefined -- \EFI\FOO identifies a regular file, so it has no child directory entries, such as \EFI\FOO\BAR. This means that when we attempt to open a pathname relative to a regular file, what we could mean in the best case would be a sibling, and not a child, of the last pathname component. Therefore, the UEFI spec language implies a child relationship for the relative pathname when the base file handle stands for an open directory, and *at best* a sibling relationship when the base file handle stands for an open regular file. This inconsistency is a bug. The language under EFI_FILE_PROTOCOL at <https://uefi.org/specs/UEFI/2.10/13_Protocols_Media_Access.html#efi-file-protocol>: "With any given file handle, other files may be opened relative to this file's location" is *ill-defined* for regular files. Even if we attempt to retrofit the intended meaning from directories to regular files, the behavior will not be consistent -- see the *child* vs. *sibling* interpretations, respectively. To put differently, given "fh->Open()", the expression "the location of fh" is currently ill-defined in the UEFI spec. If "fh" is a directory, then its "location" is defined as "itself", in the usual sense of pathnames. But if "fh" stands for a regular file, then its "location" is *at best* defined as its *parent* directory. Because of this, filesystem objects relative to the "location of fh" are also ill-defined. It only becomes well-defined if we exclude regular files as base (source) handles. For opening filesystem objects with the EFI_FILE_PROTOCOL.Open() member function, the base handle must refer to a directory; it must not refer to a regular file. This implicit requirement is already correctly reflected in the OpenEx() language -- the Summary there explains "This" must be a directory. https://mantis.uefi.org/mantis/view.php?id=2367 Signed-off-by: Laszlo Ersek <lersek@redhat.com> So I slightly wonder how EnhancedFatDxe resolves the *relative* pathname "BAR", when the base file handle stands for "\EFI\FOO", and "\EFI\FOO" is a regular file. Laszlo -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#109754): https://edk2.groups.io/g/devel/message/109754 Mute This Topic: https://groups.io/mt/102036263/7686176 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/leave/12367111/7686176/1913456212/xyzzy [rebecca@openfw.io] -=-=-=-=-=-=-=-=-=-=-=- ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [edk2-devel] [PATCH 1/1] OvmfPkg/VirtioFsDxe: fix SimpleFileOpen 2023-10-18 11:20 ` Laszlo Ersek 2023-10-18 11:33 ` Pedro Falcato @ 2023-10-18 13:13 ` Gerd Hoffmann 2023-10-18 15:13 ` Laszlo Ersek 1 sibling, 1 reply; 8+ messages in thread From: Gerd Hoffmann @ 2023-10-18 13:13 UTC (permalink / raw) To: Laszlo Ersek Cc: devel, Jordan Justen, Ard Biesheuvel, Oliver Steffen, Jiewen Yao Hi, > > - if (!VirtioFsFile->IsDirectory) { > > + if (!VirtioFsFile->IsDirectory && FileName[0] != '\\') { > It's nice to see this topic pop up on edk2-devel; apparently you started > testing shim on top of virtio-fs. :) Indeed. For starters just passed the host /boot/efi into the guest to see how far it goes. Amazingly ranking virtiofs first in bootorder JustWorks[tm]. Next was shim trapping into this when trying to read BOOTX64.CSV (i.e. probably it actually was fallback.efi) and a boot loop. > > 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. I have to admit I didn't check the what the spec says yet. Looking ... Hmm, EFI_FILE_PROTOCOL.Open() description says "This would typically be an open handle to a directory". I don't read that as "MUST be a directory". With an absolute path the only thing we need from the Handle passed in is the reference to the Filesystem root, the actual file/directory is not needed, so accepting anything and not only directories makes sense to me. And most existing implementations apparently agree, otherwise we would see much more fallout from this. Updating the spec to clearly document expected behavior in that corner case would be good. > (2) With this modification in place, shim is happy, but grub isn't. When > I realized that, I looked relatively deeply into making grub work on top > of virtio-fs as well -- and my findings were horrendous. > > I wrote up my findings in a private email to some colleagues; you were > among the recipients. Yes, I remember. Didn't try anything in grub yet. It boots to the grub shell prompt, but there is no kernel on the host ESP anyway. And no real grub.cfg either, only a stub pointing to the real grub.cfg on a different filesystem. But, yes, that is probably a dead end. IIRC grub completely ignores the efi filesystem drivers and uses its own exclusively (using its own interfaces, not registering them in efi). Trying to fix that opens the can of worms where you can have both grub and efi filesystem drivers being active and stomping on each others feet. > If you have a use case where you rely on shim but *not* on Grub > (UKIs?), then I'm OK relaxing the strictness of VirtioFsDxe. No concrete plans yet. There are ideas/plans to allow booting container images as VM. In that case the root fileystem would be virtiofs, and given that OVMF has a driver it IMHO makes sense to use that to boot the system. And, yes, using UKIs for that makes sense, although the exact workflow is far from being clear. I think one possible option would be to simply have an /EFI subdirectory on the root filesystem, place the files you would have on the ESP there, so the rootfs becomes EFI-bootable. No need to have a boot filesystem or directory, given that EFI can read the complete filesystem anyway you can slurp the UKIs directly from /lib/modules/$kernel/vmlinuz-virt.efi. > In that case, I'd prefer upstreaming my above patch, from April, > rather than taking yours. What do you think about that? Fine with me. The UEFI spec is not clear and I don't feel like bikeshedding whenever shim behavior is a bug or not. Once the UEFI spec has been fixed the one way or other we can revisit this. > Here's a further (independent) caveat: if you are using VirtioFsDxe with > the rust language virtiofsd, then you might experience hangs in > VirtioFsInit. For fixing that, you need the following *qemu* patch set: > > [PATCH v3 0/7] vhost-user: call VHOST_USER_SET_VRING_ENABLE synchronously > https://patchew.org/QEMU/20231002203221.17241-1-lersek@redhat.com/ > > (This patch set has been on qemu-devel for nearly 2 months now, counting > from v1; I'm going to ping MST again. It's been ready for merging for > weeks now!) Good to know, I think I tapped into this when I tried to pass the complete host filesystem into the guest. IIRC mst tends to do large pulls in large intervals, I wouldn't worry too much yet. Pinging doesn't hurt though. take care, Gerd -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#109746): https://edk2.groups.io/g/devel/message/109746 Mute This Topic: https://groups.io/mt/102036263/7686176 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io] -=-=-=-=-=-=-=-=-=-=-=- ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [edk2-devel] [PATCH 1/1] OvmfPkg/VirtioFsDxe: fix SimpleFileOpen 2023-10-18 13:13 ` Gerd Hoffmann @ 2023-10-18 15:13 ` Laszlo Ersek 0 siblings, 0 replies; 8+ messages in thread From: Laszlo Ersek @ 2023-10-18 15:13 UTC (permalink / raw) To: Gerd Hoffmann Cc: devel, Jordan Justen, Ard Biesheuvel, Oliver Steffen, Jiewen Yao On 10/18/23 15:13, Gerd Hoffmann wrote: > Hi, > >>> - if (!VirtioFsFile->IsDirectory) { >>> + if (!VirtioFsFile->IsDirectory && FileName[0] != '\\') { > >> It's nice to see this topic pop up on edk2-devel; apparently you started >> testing shim on top of virtio-fs. :) > > Indeed. For starters just passed the host /boot/efi into the guest to > see how far it goes. > > Amazingly ranking virtiofs first in bootorder JustWorks[tm]. Well, there's no magic :) On one side, in OVMF's QemuBootOrderLib, the TranslatePciOfwNodes() function contains a "Generic OpenFirmware device path for PCI devices" branch (a catch-all branch, if none of the more specific branches match). On the other side, I had changed QEMU to produce a "bootorder" fw_cfg entry for virtio-fs, in commit 6da32fe5efdd ("vhost-user-fs: add the "bootindex" property", 2021-01-13) -- specifically for this purpose. (The QEMU commit message shows an example OFW device path: /pci@i0cf8/pci-bridge@1,6/pci1af4,105a@0/filesystem@0 And, if I remember correctly, the "pci1af4,105a" part was so ugly that I decided not to add a specific branch for it in QemuBootOrderLib!) > > Next was shim trapping into this when trying to read BOOTX64.CSV (i.e. > probably it actually was fallback.efi) and a boot loop. > >>> 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. > > I have to admit I didn't check the what the spec says yet. Looking ... > > Hmm, EFI_FILE_PROTOCOL.Open() description says "This would typically be > an open handle to a directory". I don't read that as "MUST be a > directory". > > With an absolute path the only thing we need from the Handle passed in > is the reference to the Filesystem root, the actual file/directory is > not needed, so accepting anything and not only directories makes sense > to me. And most existing implementations apparently agree, otherwise we > would see much more fallout from this. > > Updating the spec to clearly document expected behavior in that corner > case would be good. The spec is inconsistent, in multiple ways. * Open() is inconsistent with OpenEx(), as the latter does require "This" to be a directory -- the Summary on OpenEx() says, "Opens a new file relative to the source directory’s location." * The "location of base file handle" is inconsistently defined (ill-defined) by the spec, between the base file being a directory (--> implying a "child" relationship), versus the base file being a regular file (--> implying a "sibling" relationship at best). This definition is more fundamental IMO than the pathname being absolute or relative. >> (2) With this modification in place, shim is happy, but grub isn't. When >> I realized that, I looked relatively deeply into making grub work on top >> of virtio-fs as well -- and my findings were horrendous. >> >> I wrote up my findings in a private email to some colleagues; you were >> among the recipients. > > Yes, I remember. > > Didn't try anything in grub yet. It boots to the grub shell prompt, but > there is no kernel on the host ESP anyway. And no real grub.cfg either, > only a stub pointing to the real grub.cfg on a different filesystem. > > But, yes, that is probably a dead end. IIRC grub completely ignores the > efi filesystem drivers and uses its own exclusively (using its own > interfaces, not registering them in efi). That's how I remember it too; grub uses BlockIo / DiskIo protocols as the basis for its own filesystem drivers, and does not consume EFI SimpleFs at all. > > Trying to fix that opens the can of worms where you can have both grub > and efi filesystem drivers being active and stomping on each others > feet. Right; in a nutshell, that's the core issue. > >> If you have a use case where you rely on shim but *not* on Grub >> (UKIs?), then I'm OK relaxing the strictness of VirtioFsDxe. > > No concrete plans yet. There are ideas/plans to allow booting container > images as VM. In that case the root fileystem would be virtiofs, and > given that OVMF has a driver it IMHO makes sense to use that to boot the > system. I'd love that; it was one idea behind <https://bugzilla.redhat.com/show_bug.cgi?id=1741615>. (Unfortunately that BZ is private; I don't know why -- probably because it was cloned from a kernel ticket.) > And, yes, using UKIs for that makes sense, although the exact > workflow is far from being clear. > > I think one possible option would be to simply have an /EFI subdirectory > on the root filesystem, place the files you would have on the ESP there, > so the rootfs becomes EFI-bootable. No need to have a boot filesystem > or directory, given that EFI can read the complete filesystem anyway you > can slurp the UKIs directly from /lib/modules/$kernel/vmlinuz-virt.efi. > >> In that case, I'd prefer upstreaming my above patch, from April, >> rather than taking yours. What do you think about that? > > Fine with me. Thanks, I'll submit the patch then. > > The UEFI spec is not clear and I don't feel like bikeshedding whenever > shim behavior is a bug or not. That's very generous of you (I'm not kidding -- I'm serious). I am not that generous; I insist we call this -- for now! -- a shim and UEFI spec bug. The reason is that, when I was writing VirtioFsSimpleFileOpen(), I struggled for a *very* long time making heads or tails of the spec. I decided, already back then, that the spec was buggy, so I chose the safest (most restrictive) interpretation. If the spec gets changed "my way" (i.e., restricting the behavior, which officially makes shim's current behavior a bug), then my edk2 patch can remain in place. If the spec gets clarified the opposite way, then edk2 my patch can (and should) be replaced by yours. > Once the UEFI spec has been fixed the > one way or other we can revisit this. Yes. > >> Here's a further (independent) caveat: if you are using VirtioFsDxe with >> the rust language virtiofsd, then you might experience hangs in >> VirtioFsInit. For fixing that, you need the following *qemu* patch set: >> >> [PATCH v3 0/7] vhost-user: call VHOST_USER_SET_VRING_ENABLE synchronously >> https://patchew.org/QEMU/20231002203221.17241-1-lersek@redhat.com/ >> >> (This patch set has been on qemu-devel for nearly 2 months now, counting >> from v1; I'm going to ping MST again. It's been ready for merging for >> weeks now!) > > Good to know, I think I tapped into this when I tried to pass the > complete host filesystem into the guest. On a tangent: the somewhat funny thing is that, the way the Linux virtiofs driver is implemented, an indefinite time delay is *guaranteed* to exist between the virtio setup and the FUSE_INIT message, in practice. And that masks the race. Details here: http://mid.mail-archive.com/b24315a1-906d-f3af-02e5-e6788765639c@redhat.com > IIRC mst tends to do large pulls in large intervals, I wouldn't worry > too much yet. Pinging doesn't hurt though. Yes, Michael is on it now. Thanks, Laszlo -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#109762): https://edk2.groups.io/g/devel/message/109762 Mute This Topic: https://groups.io/mt/102036263/7686176 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/leave/12367111/7686176/1913456212/xyzzy [rebecca@openfw.io] -=-=-=-=-=-=-=-=-=-=-=- ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2023-10-18 15:13 UTC | newest] Thread overview: 8+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2023-10-18 10:33 [edk2-devel] [PATCH 1/1] OvmfPkg/VirtioFsDxe: fix SimpleFileOpen Gerd Hoffmann 2023-10-18 11:20 ` Laszlo Ersek 2023-10-18 11:33 ` Pedro Falcato 2023-10-18 12:20 ` Laszlo Ersek 2023-10-18 13:08 ` Pedro Falcato 2023-10-18 14:03 ` Laszlo Ersek 2023-10-18 13:13 ` Gerd Hoffmann 2023-10-18 15:13 ` Laszlo Ersek
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox