[PATCH 0/1] MR498: server: Allow creating named pipes using \Device\NamedPipe\ as RootDirectory. (#52105)
On Windows, \Device\NamedPipe\ is the root directory of the named pipe file system (NPFS), and can be used as RootDirectory to skip its path when accessing the NPFS namespace. This introduces a regression that makes it possible to use \Device\NamedPipe (note the lack of trailing backslash) as RootDirectory as well, since Wine does not distinguish between \Device\NamedPipe (the device itself) and \Device\NamedPipe\ (the root directory of the filesystem). (Previous iteration of this patch did distinguish between the two, but the method in which it was implemented was deemed hacky.) -- https://gitlab.winehq.org/wine/wine/-/merge_requests/498
From: Jinoh Kang <jinoh.kang.kr(a)gmail.com> On Windows, \Device\NamedPipe\ is the root directory of the named pipe file system (NPFS), and can be used as RootDirectory to skip its path when accessing the NPFS namespace. This introduces a regression that makes it possible to use \Device\NamedPipe (note the lack of trailing backslash) as RootDirectory as well, since Wine does not distinguish between \Device\NamedPipe (the device itself) and \Device\NamedPipe\ (the root directory of the filesystem). Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=52105 --- dlls/ntdll/tests/pipe.c | 5 +++-- server/named_pipe.c | 11 ++++++++++- 2 files changed, 13 insertions(+), 3 deletions(-) diff --git a/dlls/ntdll/tests/pipe.c b/dlls/ntdll/tests/pipe.c index 0ad09daaa82..f0584a8efd1 100644 --- a/dlls/ntdll/tests/pipe.c +++ b/dlls/ntdll/tests/pipe.c @@ -2527,6 +2527,7 @@ static void test_empty_name(void) timeout.QuadPart = -(LONG64)10000000; status = pNtCreateNamedPipeFile(&hpipe, GENERIC_READ|GENERIC_WRITE, &attr, &io, FILE_SHARE_READ|FILE_SHARE_WRITE, FILE_CREATE, FILE_PIPE_FULL_DUPLEX, 0, 0, 0, 1, 256, 256, &timeout); + todo_wine ok(status == STATUS_OBJECT_NAME_INVALID, "unexpected status from NtCreateNamedPipeFile: %#lx\n", status); if (!status) CloseHandle(hpipe); @@ -2647,11 +2648,11 @@ static void test_empty_name(void) timeout.QuadPart = -(LONG64)10000000; status = pNtCreateNamedPipeFile(&hpipe, GENERIC_READ|GENERIC_WRITE, &attr, &io, FILE_SHARE_READ|FILE_SHARE_WRITE, FILE_CREATE, FILE_PIPE_FULL_DUPLEX, 0, 0, 0, 1, 256, 256, &timeout); - todo_wine ok(!status, "unexpected failure from NtCreateNamedPipeFile: %#lx\n", status); + ok(!status, "unexpected failure from NtCreateNamedPipeFile: %#lx\n", status); handle = CreateFileA("\\\\.\\pipe\\test3\\pipe", GENERIC_READ, FILE_SHARE_READ|FILE_SHARE_WRITE, NULL, OPEN_EXISTING, 0, 0 ); - todo_wine ok(handle != INVALID_HANDLE_VALUE, "Failed to open NamedPipe (%lu)\n", GetLastError()); + ok(handle != INVALID_HANDLE_VALUE, "Failed to open NamedPipe (%lu)\n", GetLastError()); CloseHandle(handle); CloseHandle(hpipe); diff --git a/server/named_pipe.c b/server/named_pipe.c index 3e6cf09d4f2..ff18651a549 100644 --- a/server/named_pipe.c +++ b/server/named_pipe.c @@ -277,6 +277,8 @@ static const struct object_ops named_pipe_device_ops = static void named_pipe_device_file_dump( struct object *obj, int verbose ); static struct fd *named_pipe_device_file_get_fd( struct object *obj ); static WCHAR *named_pipe_device_file_get_full_name( struct object *obj, data_size_t *len ); +static struct object *named_pipe_device_file_lookup_name( struct object *obj, struct unicode_str *name, + unsigned int attr, struct object *root ); static void named_pipe_device_ioctl( struct fd *fd, ioctl_code_t code, struct async *async ); static enum server_fd_type named_pipe_device_file_get_fd_type( struct fd *fd ); static void named_pipe_device_file_destroy( struct object *obj ); @@ -296,7 +298,7 @@ static const struct object_ops named_pipe_device_file_ops = default_get_sd, /* get_sd */ default_set_sd, /* set_sd */ named_pipe_device_file_get_full_name, /* get_full_name */ - no_lookup_name, /* lookup_name */ + named_pipe_device_file_lookup_name, /* lookup_name */ no_link_name, /* link_name */ NULL, /* unlink_name */ no_open_file, /* open_file */ @@ -568,6 +570,13 @@ static WCHAR *named_pipe_device_file_get_full_name( struct object *obj, data_siz return file->device->obj.ops->get_full_name( &file->device->obj, len ); } +static struct object *named_pipe_device_file_lookup_name( struct object *obj, struct unicode_str *name, + unsigned int attr, struct object *root ) +{ + struct named_pipe_device_file *file = (struct named_pipe_device_file *)obj; + return grab_object( file->device ); +} + static enum server_fd_type named_pipe_device_file_get_fd_type( struct fd *fd ) { return FD_TYPE_DEVICE; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/498
On 7/21/22 08:37, Jinoh Kang wrote:
From: Jinoh Kang <jinoh.kang.kr(a)gmail.com>
On Windows, \Device\NamedPipe\ is the root directory of the named pipe file system (NPFS), and can be used as RootDirectory to skip its path when accessing the NPFS namespace.
This introduces a regression that makes it possible to use \Device\NamedPipe (note the lack of trailing backslash) as RootDirectory as well, since Wine does not distinguish between \Device\NamedPipe (the device itself) and \Device\NamedPipe\ (the root directory of the filesystem).
We should probably fix that first, then. Related questions: * is \Device\NamedPipe any different from a normal pipe? * is \Device\NamedPipe\ any different from a normal pipe? * is "\Device\NamedPipe\foo" a different object than "\Device\NamedPipe\foo\", and, if so, does it behave any differently?
On Sun, Oct 9, 2022 at 2:06 AM Zebediah Figura <zfigura(a)codeweavers.com> wrote:
On 7/21/22 08:37, Jinoh Kang wrote:
From: Jinoh Kang <jinoh.kang.kr(a)gmail.com>
On Windows, \Device\NamedPipe\ is the root directory of the named pipe file system (NPFS), and can be used as RootDirectory to skip its path when accessing the NPFS namespace.
This introduces a regression that makes it possible to use \Device\NamedPipe (note the lack of trailing backslash) as RootDirectory as well, since Wine does not distinguish between \Device\NamedPipe (the device itself) and \Device\NamedPipe\ (the root directory of the filesystem).
We should probably fix that first, then. Related questions:
* is \Device\NamedPipe any different from a normal pipe?
Yes, it is. It's a device object.
* is \Device\NamedPipe\ any different from a normal pipe?
Yes, it is. It's a directory of a filesystem, named NPFS. This is the only directory of NPFS. There does not exist a directory with any other pathname for NPFS.
* is "\Device\NamedPipe\foo" a different object than "\Device\NamedPipe\foo\", and, if so, does it behave any differently?
Yes, it is. They are both normal named pipes, each with a unique, different name. Two pathnames you have mentioned are both valid. They are not aliases to each other.
On 10/10/22 10:12, Jin-oh Kang wrote:
* is \Device\NamedPipe any different from a normal pipe?
Yes, it is. It's a device object.
Well, yes and no; there is a device object that exists in the object tree at \Device\NamedPipe (it has an object type of "Device"), but I mean the file object that you get when you open that exact path (which has an object type of "File"). Is that different from a normal pipe, and if so, how?
* is \Device\NamedPipe\ any different from a normal pipe?
Yes, it is. It's a directory of a filesystem, named NPFS. This is the only directory of NPFS. There does not exist a directory with any other pathname for NPFS.
What does this mean in practice, though? We don't have tests for trying to do anything with the \Device\NamedPipe\ file, other than calling FSCTL_PIPE_WAIT on it. (And we don't have tests for FSCTL_PIPE_WAIT on "normal" pipes either.) Also, as far as I can find, we don't have tests for using a RootDirectory other than \Device\NamedPipe or \Device\NamedPipe\.
* is "\Device\NamedPipe\foo" a different object than "\Device\NamedPipe\foo\", and, if so, does it behave any differently?
Yes, it is. They are both normal named pipes, each with a unique, different name. Two pathnames you have mentioned are both valid. They are not aliases to each other.
Do we have tests for this anywhere?
On Tue, Oct 11, 2022, 2:56 AM Zebediah Figura <zfigura(a)codeweavers.com> wrote:
On 10/10/22 10:12, Jin-oh Kang wrote:
* is \Device\NamedPipe any different from a normal pipe?
Yes, it is. It's a device object.
Well, yes and no; there is a device object that exists in the object tree at \Device\NamedPipe (it has an object type of "Device"), but I mean the file object that you get when you open that exact path (which has an object type of "File"). Is that different from a normal pipe, and if so, how?
ntdll:pipe test_empty_name() says that calling FSCTL_PIPE_WAIT on it returns STATUS_ILLEGAL_FUNCTION. It doesn't say anything else about it. Do you mind if I add some tests that use it?
* is \Device\NamedPipe\ any different from a normal pipe?
Yes, it is. It's a directory of a filesystem, named NPFS. This is the only directory of NPFS. There does not exist a directory with any other pathname for NPFS.
What does this mean in practice, though? We don't have tests for trying to do anything with the \Device\NamedPipe\ file, other than calling FSCTL_PIPE_WAIT on it.
We do have tests that use it, though. ntdll:pipe test_empty_name() uses the \Device\NamedPipe\ file as RootDirectory for creating named pipes. Again, do you mind if I add some tests that exercise it? (And we don't have tests for FSCTL_PIPE_WAIT on
"normal" pipes either.)
Also, as far as I can find, we don't have tests for using a RootDirectory other than \Device\NamedPipe or \Device\NamedPipe\.
* is "\Device\NamedPipe\foo" a different object than "\Device\NamedPipe\foo\", and, if so, does it behave any differently?
Yes, it is. They are both normal named pipes, each with a unique, different name. Two pathnames you have mentioned are both valid. They are not aliases to each other.
Do we have tests for this anywhere?
It's on testbot: https://testbot.winehq.org/JobDetails.pl?Key=124840 Do you have any feedback on the test before it is submitted?
On 10/11/22 07:13, Jin-oh Kang wrote:
On Tue, Oct 11, 2022, 2:56 AM Zebediah Figura <zfigura(a)codeweavers.com> wrote:
On 10/10/22 10:12, Jin-oh Kang wrote:
* is \Device\NamedPipe any different from a normal pipe?
Yes, it is. It's a device object.
Well, yes and no; there is a device object that exists in the object tree at \Device\NamedPipe (it has an object type of "Device"), but I mean the file object that you get when you open that exact path (which has an object type of "File"). Is that different from a normal pipe, and if so, how?
ntdll:pipe test_empty_name() says that calling FSCTL_PIPE_WAIT on it returns STATUS_ILLEGAL_FUNCTION. It doesn't say anything else about it. Do you mind if I add some tests that use it?
My concern is basically that we don't really prove that this pipe is different from a normal pipe. I.e. we don't try calling FSCTL_PIPE_WAIT on a normal pipe, and we don't do anything else with this pipe. (How does it react to FSCTL_PIPE_LISTEN? FSCTL_PIPE_PEEK? FSCTL_PIPE_GET_CONNECTION_ATTRIBUTE? Read or write?) Of course more tests are always welcome; I'm just trying to describe the specific tests I think would be interesting to answer.
* is \Device\NamedPipe\ any different from a normal pipe?
Yes, it is. It's a directory of a filesystem, named NPFS. This is the only directory of NPFS. There does not exist a directory with any other pathname for NPFS.
What does this mean in practice, though? We don't have tests for trying to do anything with the \Device\NamedPipe\ file, other than calling FSCTL_PIPE_WAIT on it.
We do have tests that use it, though. ntdll:pipe test_empty_name() uses the \Device\NamedPipe\ file as RootDirectory for creating named pipes. Again, do you mind if I add some tests that exercise it?
Right, sorry, I mean tests other than how this interacts with the object tree. I think that's relatively clear by now.
(And we don't have tests for FSCTL_PIPE_WAIT on
"normal" pipes either.)
Also, as far as I can find, we don't have tests for using a RootDirectory other than \Device\NamedPipe or \Device\NamedPipe\.
* is "\Device\NamedPipe\foo" a different object than "\Device\NamedPipe\foo\", and, if so, does it behave any differently?
Yes, it is. They are both normal named pipes, each with a unique, different name. Two pathnames you have mentioned are both valid. They are not aliases to each other.
Do we have tests for this anywhere?
It's on testbot: https://testbot.winehq.org/JobDetails.pl?Key=124840
Do you have any feedback on the test before it is submitted?
No, that's pretty much what I was looking for :-)
On Sun, Oct 9, 2022 at 2:06 AM Zebediah Figura <zfigura(a)codeweavers.com> wrote:
On 7/21/22 08:37, Jinoh Kang wrote:
From: Jinoh Kang <jinoh.kang.kr(a)gmail.com>
On Windows, \Device\NamedPipe\ is the root directory of the named pipe file system (NPFS), and can be used as RootDirectory to skip its path when accessing the NPFS namespace.
This introduces a regression that makes it possible to use \Device\NamedPipe (note the lack of trailing backslash) as RootDirectory as well, since Wine does not distinguish between \Device\NamedPipe (the device itself) and \Device\NamedPipe\ (the root directory of the filesystem).
We should probably fix that first, then.
This is a previous iteration of the patch that attempts to fix both at once: https://www.winehq.org/pipermail/wine-devel/2022-March/211849.html This patch does not significantly from the patch mentioned in https://bugs.winehq.org/show_bug.cgi?id=52105. The patch apparently never got merged becuase it involves a hack to differentiate \Device\NamedPipe\ and \Device\NamedPipe. The hack exploits the fact that `!name->len && name->str` when the path ends with a trailing slash. Properly differentiating the two would at least involve changes to the `lookup_name` function's parameters. Actually implementing \Device\NamedPipe\'s directory semantics (i.e. listing) would involve further changes to Wine's object manager emulation.
On Tue, Oct 11, 2022 at 12:22 AM Jin-oh Kang <jinoh.kang.kr(a)gmail.com> wrote:
On Sun, Oct 9, 2022 at 2:06 AM Zebediah Figura <zfigura(a)codeweavers.com> wrote:
On 7/21/22 08:37, Jinoh Kang wrote:
From: Jinoh Kang <jinoh.kang.kr(a)gmail.com>
On Windows, \Device\NamedPipe\ is the root directory of the named pipe file system (NPFS), and can be used as RootDirectory to skip its path when accessing the NPFS namespace.
This introduces a regression that makes it possible to use \Device\NamedPipe (note the lack of trailing backslash) as RootDirectory as well, since Wine does not distinguish between \Device\NamedPipe (the device itself) and \Device\NamedPipe\ (the root directory of the filesystem).
We should probably fix that first, then.
This is a previous iteration of the patch that attempts to fix both at once:
https://www.winehq.org/pipermail/wine-devel/2022-March/211849.html
This patch does not significantly from the patch mentioned in https://bugs.winehq.org/show_bug.cgi?id=52105.
significantly differ*
On 10/10/22 10:22, Jin-oh Kang wrote:
This is a previous iteration of the patch that attempts to fix both at once:
https://www.winehq.org/pipermail/wine-devel/2022-March/211849.html
This patch does not significantly from the patch mentioned in https://bugs.winehq.org/show_bug.cgi?id=52105.
The patch apparently never got merged becuase it involves a hack to differentiate \Device\NamedPipe\ and \Device\NamedPipe. The hack exploits the fact that `!name->len && name->str` when the path ends with a trailing slash. Properly differentiating the two would at least involve changes to the `lookup_name` function's parameters.
Well, yeah, we should probably fix that too, then.
Actually implementing \Device\NamedPipe\'s directory semantics (i.e. listing) would involve further changes to Wine's object manager emulation.
Well, we'd need to hook up NtQueryDirectoryFile to fd_ops for device files, but I don't think we'd need anything more drastic than that. That said, we'd also want tests, and probably to defer this until we find something that needs it...
Simple patch. Looks good to me, and it fixes the related issue in Msys2. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/498#note_9251
Hi @iamahuman , would you be able to rebase this? I think it might help improve the chances of getting this included. I'm currently working on fixing issues affecting Msys2 (and Cygwin), and these patches make a clear improvement. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/498#note_10031
participants (5)
-
Jin-oh Kang -
Jinoh Kang -
Jinoh Kang (＠iamahuman)
-
Joel Holdsworth (＠jhol)
-
Zebediah Figura