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

Bin Meng posted 16 patches 1 year, 2 months ago
There is a newer version of this series
meson.build                           |   10 +-
fsdev/file-op-9p.h                    |   33 +
hw/9pfs/9p-linux-errno.h              |  151 +++
hw/9pfs/9p-local.h                    |    8 +
hw/9pfs/9p-util.h                     |  139 ++-
hw/9pfs/9p.h                          |   43 +
tests/qtest/libqos/virtio-9p-client.h |    7 +
fsdev/qemu-fsdev.c                    |    2 +
hw/9pfs/9p-local.c                    |  269 ++++-
hw/9pfs/9p-synth.c                    |    5 +-
hw/9pfs/9p-util-win32.c               | 1305 +++++++++++++++++++++++++
hw/9pfs/9p.c                          |   90 +-
hw/9pfs/codir.c                       |    2 +-
fsdev/meson.build                     |    1 +
hw/9pfs/meson.build                   |    8 +-
15 files changed, 2008 insertions(+), 65 deletions(-)
create mode 100644 hw/9pfs/9p-linux-errno.h
create mode 100644 hw/9pfs/9p-util-win32.c
[PATCH v4 00/16] hw/9pfs: Add 9pfs support for Windows
Posted by Bin Meng 1 year, 2 months ago
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

Bin Meng (2):
  hw/9pfs: Update helper qemu_stat_rdev()
  hw/9pfs: Add a helper qemu_stat_blksize()

Guohuai Shi (14):
  hw/9pfs: Add missing definitions for Windows
  hw/9pfs: Implement Windows specific utilities functions for 9pfs
  hw/9pfs: Replace the direct call to xxxdir() APIs with a wrapper
  hw/9pfs: Implement Windows specific xxxdir() APIs
  hw/9pfs: Update the local fs driver to support Windows
  hw/9pfs: Support getting current directory offset for Windows
  hw/9pfs: Disable unsupported flags and features for Windows
  hw/9pfs: Update v9fs_set_fd_limit() for Windows
  hw/9pfs: Add Linux error number definition
  hw/9pfs: Translate Windows errno to Linux value
  fsdev: Disable proxy fs driver on Windows
  hw/9pfs: Update synth fs driver for Windows
  tests/qtest: virtio-9p-test: Adapt the case for win32
  meson.build: Turn on virtfs for Windows

 meson.build                           |   10 +-
 fsdev/file-op-9p.h                    |   33 +
 hw/9pfs/9p-linux-errno.h              |  151 +++
 hw/9pfs/9p-local.h                    |    8 +
 hw/9pfs/9p-util.h                     |  139 ++-
 hw/9pfs/9p.h                          |   43 +
 tests/qtest/libqos/virtio-9p-client.h |    7 +
 fsdev/qemu-fsdev.c                    |    2 +
 hw/9pfs/9p-local.c                    |  269 ++++-
 hw/9pfs/9p-synth.c                    |    5 +-
 hw/9pfs/9p-util-win32.c               | 1305 +++++++++++++++++++++++++
 hw/9pfs/9p.c                          |   90 +-
 hw/9pfs/codir.c                       |    2 +-
 fsdev/meson.build                     |    1 +
 hw/9pfs/meson.build                   |    8 +-
 15 files changed, 2008 insertions(+), 65 deletions(-)
 create mode 100644 hw/9pfs/9p-linux-errno.h
 create mode 100644 hw/9pfs/9p-util-win32.c

-- 
2.25.1
Re: [PATCH v4 00/16] hw/9pfs: Add 9pfs support for Windows
Posted by Marc-André Lureau 1 year, 2 months ago
Hi

On Mon, Jan 30, 2023 at 1:52 PM Bin Meng <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.

Btw, I would not count on mingw adding support for flags/API (S_IFLNK etc),
that do not make sense on win32. Did you request them?

I suppose the 9pfs maintainers (Greg, Christian) will have to decide. I can
take a deeper look if the overall approach is approved, and as needed.

Bin Meng (2):
>   hw/9pfs: Update helper qemu_stat_rdev()
>   hw/9pfs: Add a helper qemu_stat_blksize()
>
> Guohuai Shi (14):
>   hw/9pfs: Add missing definitions for Windows
>   hw/9pfs: Implement Windows specific utilities functions for 9pfs
>   hw/9pfs: Replace the direct call to xxxdir() APIs with a wrapper
>   hw/9pfs: Implement Windows specific xxxdir() APIs
>   hw/9pfs: Update the local fs driver to support Windows
>   hw/9pfs: Support getting current directory offset for Windows
>   hw/9pfs: Disable unsupported flags and features for Windows
>   hw/9pfs: Update v9fs_set_fd_limit() for Windows
>   hw/9pfs: Add Linux error number definition
>   hw/9pfs: Translate Windows errno to Linux value
>   fsdev: Disable proxy fs driver on Windows
>   hw/9pfs: Update synth fs driver for Windows
>   tests/qtest: virtio-9p-test: Adapt the case for win32
>   meson.build: Turn on virtfs for Windows
>
>  meson.build                           |   10 +-
>  fsdev/file-op-9p.h                    |   33 +
>  hw/9pfs/9p-linux-errno.h              |  151 +++
>  hw/9pfs/9p-local.h                    |    8 +
>  hw/9pfs/9p-util.h                     |  139 ++-
>  hw/9pfs/9p.h                          |   43 +
>  tests/qtest/libqos/virtio-9p-client.h |    7 +
>  fsdev/qemu-fsdev.c                    |    2 +
>  hw/9pfs/9p-local.c                    |  269 ++++-
>  hw/9pfs/9p-synth.c                    |    5 +-
>  hw/9pfs/9p-util-win32.c               | 1305 +++++++++++++++++++++++++
>  hw/9pfs/9p.c                          |   90 +-
>  hw/9pfs/codir.c                       |    2 +-
>  fsdev/meson.build                     |    1 +
>  hw/9pfs/meson.build                   |    8 +-
>  15 files changed, 2008 insertions(+), 65 deletions(-)
>  create mode 100644 hw/9pfs/9p-linux-errno.h
>  create mode 100644 hw/9pfs/9p-util-win32.c
>
> --
> 2.25.1
>
>
Re: [PATCH v4 00/16] hw/9pfs: Add 9pfs support for Windows
Posted by Daniel P. Berrangé 1 year, 2 months ago
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 <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.

The currently impl relies on the openat, fstatat, mkdirat, renameat,
utimensat, unlinkat functions. IIRC this was in order to deal with
various security vulnerabilities that exist due to race conditions.
AFAIK, there's no way to achieve the same with GIO as its a higher
level API which doesn't expose this kind of functionality


With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|


Re: [PATCH v4 00/16] hw/9pfs: Add 9pfs support for Windows
Posted by Marc-André Lureau 1 year, 2 months ago
Hi

On Tue, Jan 31, 2023 at 6:39 PM Daniel P. Berrangé <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 <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.
>
> The currently impl relies on the openat, fstatat, mkdirat, renameat,
> utimensat, unlinkat functions. IIRC this was in order to deal with
> various security vulnerabilities that exist due to race conditions.
> AFAIK, there's no way to achieve the same with GIO as its a higher
> level API which doesn't expose this kind of functionality
>
>
Correct me if I am wrong, but that doesn't seem to hold much since the
protocol doesn't keep a context (with associated fds) around. But perhaps
GIO API alone can't provide safe implementations of the FileOperations
callbacks?

Also a lot of 9p-unix specific details may not map easily to the GIO API.
How they can be ported to win32 is certainly a challenge, mostly
duplicating the effort done in GIO to me.
RE: [PATCH v4 00/16] hw/9pfs: Add 9pfs support for Windows
Posted by Shi, Guohuai 1 year, 2 months ago

> 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.

>
> The currently impl relies on the openat, fstatat, mkdirat, renameat,
> utimensat, unlinkat functions. IIRC this was in order to deal with
> various security vulnerabilities that exist due to race conditions.
> AFAIK, there's no way to achieve the same with GIO as its a higher
> level API which doesn't expose this kind of functionality
>
> Correct me if I am wrong, but that doesn't seem to hold much since the protocol doesn't keep a context (with associated fds) around. But perhaps GIO API alone can't provide safe implementations of the FileOperations callbacks?
>
> Also a lot of 9p-unix specific details may not map easily to the GIO API. How they can be ported to win32 is certainly a challenge, mostly duplicating the effort done in GIO to me.

Thanks
Guohuai
Re: [PATCH v4 00/16] hw/9pfs: Add 9pfs support for Windows
Posted by Marc-André Lureau 1 year, 2 months ago
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..