From: Christian Schoenebeck <qemu_oss@crudebyte.com>
To: qemu-devel@nongnu.org, "Meng, Bin" <Bin.Meng@windriver.com>
Cc: Greg Kurz <groug@kaod.org>, Bin Meng <bmeng.cn@gmail.com>,
"Shi, Guohuai" <Guohuai.Shi@windriver.com>
Subject: Re: [PATCH 5/9] hw/9pfs: Add a 'local' file system backend driver for Windows
Date: Tue, 10 May 2022 12:18:33 +0200 [thread overview]
Message-ID: <3148498.2bi0oRTOK1@silver> (raw)
In-Reply-To: <MN2PR11MB41738234AB88DED21F896459EFC99@MN2PR11MB4173.namprd11.prod.outlook.com>
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/dirent.
> > 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.
> > >
> > >
> > >
> > > 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-crea
> > 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
next prev parent reply other threads:[~2022-05-10 10:23 UTC|newest]
Thread overview: 34+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-04-25 14:26 [PATCH 0/9] 9pfs: Add 9pfs support for Windows host Bin Meng
2022-04-25 14:26 ` [PATCH 1/9] hw/9pfs: Compile 9p-local.c and 9p-proxy.c for Linux and macOS Bin Meng
2022-04-25 14:26 ` [PATCH 2/9] qemu/xatth.h: Update for Windows build Bin Meng
2022-04-25 14:26 ` [PATCH 3/9] hw/9pfs: Extract common stuff to 9p-local.h Bin Meng
2022-04-25 14:27 ` [PATCH 4/9] fsdev: Add missing definitions for Windows in file-op-9p.h Bin Meng
2022-05-04 17:35 ` Christian Schoenebeck
2022-04-25 14:27 ` [PATCH 5/9] hw/9pfs: Add a 'local' file system backend driver for Windows Bin Meng
2022-05-04 18:01 ` Christian Schoenebeck
2022-05-04 19:34 ` Shi, Guohuai
2022-05-05 11:43 ` Christian Schoenebeck
2022-05-06 6:46 ` Shi, Guohuai
2022-05-09 14:29 ` Greg Kurz
2022-05-09 15:09 ` Shi, Guohuai
2022-05-09 16:20 ` Greg Kurz
2022-05-10 2:13 ` Shi, Guohuai
2022-05-10 2:17 ` Shi, Guohuai
2022-05-10 10:18 ` Christian Schoenebeck [this message]
2022-05-10 11:54 ` Christian Schoenebeck
2022-05-10 13:40 ` Greg Kurz
2022-05-10 14:04 ` Christian Schoenebeck
2022-05-10 14:34 ` Greg Kurz
2022-05-10 15:35 ` Shi, Guohuai
2022-05-11 11:18 ` Christian Schoenebeck
2022-05-11 12:18 ` Greg Kurz
2022-05-11 15:57 ` Shi, Guohuai
2022-05-24 12:23 ` Christian Schoenebeck
2022-04-25 14:27 ` [PATCH 6/9] hw/9pfs: Update 9p-synth.c for Windows build Bin Meng
2022-04-25 14:27 ` [PATCH 7/9] fsdev: Enable 'local' file system driver backend for Windows Bin Meng
2022-04-25 14:27 ` [PATCH 8/9] meson.build: Turn on virtfs for Windows host Bin Meng
2022-04-25 14:27 ` [PATCH 9/9] hw/9p: win32: Translate Windows error number to Linux value Bin Meng
2022-05-04 18:15 ` Christian Schoenebeck
2022-04-26 1:41 ` [PATCH 0/9] 9pfs: Add 9pfs support for Windows host Bin Meng
2022-05-03 3:42 ` Bin Meng
2022-05-04 17:16 ` Christian Schoenebeck
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=3148498.2bi0oRTOK1@silver \
--to=qemu_oss@crudebyte.com \
--cc=Bin.Meng@windriver.com \
--cc=Guohuai.Shi@windriver.com \
--cc=bmeng.cn@gmail.com \
--cc=groug@kaod.org \
--cc=qemu-devel@nongnu.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).