[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH 5/9] hw/9pfs: Add a 'local' file system backend driver for Wi
From: |
Christian Schoenebeck |
Subject: |
Re: [PATCH 5/9] hw/9pfs: Add a 'local' file system backend driver for Windows |
Date: |
Tue, 10 May 2022 13:54:46 +0200 |
On Dienstag, 10. Mai 2022 12:18:33 CEST Christian Schoenebeck wrote:
> On Dienstag, 10. Mai 2022 04:17:44 CEST Shi, Guohuai wrote:
> [...]
>
> > > > > > I tend to agree with Christian's remarks that this patch is too
> > > > > > big
> > > > > > and that the choice of introducing right away a new implementation
> > > > > > of 9p-local for windows hosts is too bold to start with. We need
> > > > > > to
> > > > > > clearly understand what's diverging between windows and linux in
> > > > > > order
> > > > > > to make such a decision. You should first try to introduce the
> > > > > > required
> > > > > > abstractions to cope with these differences, so that we can
> > > > > > review.
> > > > >
> > > > > Here is the basic introductions of 9PFS for Windows development:
> > > > >
> > > > >
> > > > >
> > > > > Windows always returns -1 when try to call open() for a directory.
> > > > > Windows (actually MinGW library) only allows opendir() for a
> > > > > directory.
>
> That missing behaviour could be implemented in 9p-util-win.c, similar to the
> missing behaviours of mknodat() for macOS which did not support a bunch of
> things like creating a UNIX socket file and more:
>
> https://github.com/qemu/qemu/commit/055ab89327bab83f1bd07e9de07f7628643d3d8d
> > > > Does MinGW have dirfd() ?
> > >
> > > No.
> > > MinGW does not open any directory.
> > > Here is opendir() source code of MinGW:
> > > https://github.com/mirror/mingw-w64/blob/master/mingw-w64-crt/misc/diren
> > > t.
> > > c#L42
> > >
> > > So MinGW do not have a fd associated to a directory.
> > >
> > > > > Windows does not support APIs like "*at" (openat(), renameat(),
> > > > > etc.)
>
> Like already suggested before on your previous RFC version, it is possible
> to use the same workaround as we are using for macOS hosts already (which
> was missing mknodat()):
>
> pthread_fchdir_np(...)
> mknod(...)
>
> https://github.com/qemu/qemu/blob/master/hw/9pfs/9p-util-darwin.c#L84
>
> So on Windows it would be viable to:
>
> chdir(...)
> open(...)
>
> The same approach could be used for any missing *at() function for Windows.
Problem though is that the chdir() functions on Windows all seem to have
process-wide effect, we would need to change the current directory only for
the current thread, because filesystem access of 9p server is multi-threaded.
Protecting the chdir(); foo(); calls by a process wide global mutex isn't very
appealing either. :/
> > > > Ouch...
> > > >
> > > > > So 9PFS can not use any openat() for opening a sub file or directory
> > > > > in 9P
> > >
> > > mount
> > >
> > > > directory.
> > > >
> > > > > This commit use merge_fs_path() to build up full filename by string
> > >
> > > concatenation.
> > >
> > > > > I know that may have a risk of security, but Windows does fully
> > > > > support POSIX
>
> You will not find anybody merging code that's inherently insecure.
>
> > > > I understand from your various answers that symlinks aren't
> > > > currently supported by window's POSIX API. Is this forever ?
> > > > Google do mentions symlinks in windows 10. What's the story
> > > > there ? How do they behave ? How would they be exposed to the
> > > > client ? Be aware that, even if the client cannot create symlinks,
> > > > an existing symlink could be used to escape with rename().
> > > >
> > > >
> > > >
> > > > If the code "may have a risk of security" then it must be
> > > > fixed or avoided in some way before being merged upstream.
> > > >
> > > >
> > > >
> > > > Other thing that comes to mind is that windows hosts should
> > > > maybe use the mapped or mapped-file security modes since
> > > > they emulate symlinks with a simple file hidden in the
> > > > VIRTFS_META_DIR directory.
> > > >
> > > >
> > > >
> > > > Cheers,
> > > >
> > > >
> > > >
> > > > --
> > > > Greg
> > >
> > > Windows native API support symbolic link file start from Windows Vista:
> > > https://docs.microsoft.com/en-us/windows/win32/api/winbase/nf-winbase-cr
> > > ea
> > > tes ymboliclinka
> > >
> > > I mean Windows POSIX APIs do not support symbolic link (MinGW use Win32
> > > POSIX APIs) So we can not create symbolic link by MinGW.
>
> A function with POSIX signature could be added to 9p-util-win.c which would
> call the native Windows function to create symlinks.
>
> > > Anyway, there is another solution: re-work whole 9PFS code: not only
> > > 9p-local.c, but also every file in 9p driver.
> > > Replace every MinGW/POSIX APIs (e.g. open, lseek, read, write, close),
> > > by Windows Native APIs (e.g. open -> CreateFile, lseek ->
> > > SetFilePointer,
> > > read -> ReadFile, write -> WriteFile, close -> CloseHandle, etc.)
> > > Then 9P can use Windows symbolic link feature.
> > > However, I do think it is a good idea to replace everything.
> >
> > TYPO: it NOT is a good idea to replace everything.
>
> Right, that does not make sense. The way to go is adding and implementing
> missing system functions with POSIX signatures and POSIX behaviour for
> Windows. Not turning the entire code base upside down.
>
> Best regards,
> Christian Schoenebeck
- Re: [PATCH 5/9] hw/9pfs: Add a 'local' file system backend driver for Windows, Christian Schoenebeck, 2022/05/04
- Re: [PATCH 5/9] hw/9pfs: Add a 'local' file system backend driver for Windows, Greg Kurz, 2022/05/09
- RE: [PATCH 5/9] hw/9pfs: Add a 'local' file system backend driver for Windows, Shi, Guohuai, 2022/05/09
- Re: [PATCH 5/9] hw/9pfs: Add a 'local' file system backend driver for Windows, Greg Kurz, 2022/05/09
- RE: [PATCH 5/9] hw/9pfs: Add a 'local' file system backend driver for Windows, Shi, Guohuai, 2022/05/09
- RE: [PATCH 5/9] hw/9pfs: Add a 'local' file system backend driver for Windows, Shi, Guohuai, 2022/05/09
- Re: [PATCH 5/9] hw/9pfs: Add a 'local' file system backend driver for Windows, Christian Schoenebeck, 2022/05/10
- Re: [PATCH 5/9] hw/9pfs: Add a 'local' file system backend driver for Windows,
Christian Schoenebeck <=
- Re: [PATCH 5/9] hw/9pfs: Add a 'local' file system backend driver for Windows, Greg Kurz, 2022/05/10
- Re: [PATCH 5/9] hw/9pfs: Add a 'local' file system backend driver for Windows, Christian Schoenebeck, 2022/05/10
- Re: [PATCH 5/9] hw/9pfs: Add a 'local' file system backend driver for Windows, Greg Kurz, 2022/05/10
- RE: [PATCH 5/9] hw/9pfs: Add a 'local' file system backend driver for Windows, Shi, Guohuai, 2022/05/10
- Re: [PATCH 5/9] hw/9pfs: Add a 'local' file system backend driver for Windows, Christian Schoenebeck, 2022/05/11
- Re: [PATCH 5/9] hw/9pfs: Add a 'local' file system backend driver for Windows, Greg Kurz, 2022/05/11
- RE: [PATCH 5/9] hw/9pfs: Add a 'local' file system backend driver for Windows, Shi, Guohuai, 2022/05/11
- Re: [PATCH 5/9] hw/9pfs: Add a 'local' file system backend driver for Windows, Christian Schoenebeck, 2022/05/24