Re: [PATCH v4 00/16] hw/9pfs: Add 9pfs support for Windows

[Marc-André Lureau]

Hi

On Wed, Feb 1, 2023 at 5:05 PM Shi, Guohuai <Guohuai.Shi@windriver.com> wrote:
>
>
>
> > From: Marc-André Lureau <marcandre.lureau@redhat.com>
> > Sent: Tuesday, January 31, 2023 23:07
> > To: Daniel P. Berrangé <berrange@redhat.com>
> > Cc: Meng, Bin <Bin.Meng@windriver.com>; Greg Kurz <groug@kaod.org>; 
> > Christian Schoenebeck <qemu_oss@crudebyte.com>; qemu-devel@nongnu.org; Shi, 
> > Guohuai <Guohuai.Shi@windriver.com>; Laurent > Vivier <lvivier@redhat.com>; 
> > Paolo Bonzini <pbonzini@redhat.com>; Philippe Mathieu-Daudé 
> > <philmd@linaro.org>; Thomas Huth <thuth@redhat.com>
> > Subject: Re: [PATCH v4 00/16] hw/9pfs: Add 9pfs support for Windows
> >
> > CAUTION: This email comes from a non Wind River email account!
> > Do not click links or open attachments unless you recognize the sender and 
> > know the content is safe.
> > Hi
> >
> > On Tue, Jan 31, 2023 at 6:39 PM Daniel P. Berrangé 
> > <mailto:berrange@redhat.com> wrote:
> > On Tue, Jan 31, 2023 at 06:31:39PM +0400, Marc-André Lureau wrote:
> > > Hi
> > >
> > > On Mon, Jan 30, 2023 at 1:52 PM Bin Meng <mailto:bin.meng@windriver.com> 
> > > wrote:
> > >
> > > > At present there is no Windows support for 9p file system.
> > > > This series adds initial Windows support for 9p file system.
> > > >
> > > > 'local' file system backend driver is supported on Windows,
> > > > including open, read, write, close, rename, remove, etc.
> > > > All security models are supported. The mapped (mapped-xattr)
> > > > security model is implemented using NTFS Alternate Data Stream
> > > > (ADS) so the 9p export path shall be on an NTFS partition.
> > > >
> > > > 'synth' driver is adapted for Windows too so that we can now
> > > > run qtests on Windows for 9p related regression testing.
> > > >
> > > > Example command line to test:
> > > >
> > > >   "-fsdev local,path=c:\msys64,security_model=mapped,id=p9 -device
> > > > virtio-9p-pci,fsdev=p9,mount_tag=p9fs"
> > > >
> > > > Base-commit: 13356edb87506c148b163b8c7eb0695647d00c2a
> > > >
> > > > Changes in v4:
> > > > - Fixed 9pfs mounted as read-only issue on Windows host, adding a
> > > >   win32_error_to_posix() to translate Windows native API error to
> > > >   POSIX one.
> > > > - Fixed errors of handling symbolic links
> > > > - Added forward declaration to avoid using 'void *'
> > > > - Implemented Windows specific xxxdir() APIs for safe directory access
> > > >
> > > >
> > > Sorry to look a bit late at this series, I don't know what was discussed
> > > previously.
> > >
> > > My general feeling is that a lot of this FS portability work would be
> > > better handled by using GIO (even though this may add some extra
> > > dependency). GIO lacks some features on win32 (for example xattributes on
> > > win32), but they could have been proposed there too and benefiting other
> > > apps.
>
> GIO function is actually same as MinGW APIs, which is not safety as MinGW 
> (discussed in previous versions).
>
> https://gitlab.gnome.org/GNOME/glib/-/blob/main/glib/dirent/dirent.c#L61
> https://github.com/Alexpux/mingw-w64/blob/master/mingw-w64-crt/misc/dirent.c#L42
>
> GIO function also does not handle symbolic links on Windows host, this may 
> cause security issues.
> GIO functions also use Windows POSIX APIs without extra security checks (does 
> not provide NO_FOLLOW flags):
> https://gitlab.gnome.org/GNOME/glib/-/blob/main/glib/gstdio.c#L1050
>
> 9pfs need functions like openat() to make sure that the sub-sequence 
> operation is working in the expected parent.
>
> So using GIO will still have security issues.

Fair enough, it's a bit of a shame it's not easy to sandbox a process
and not have to worry about those links..