[PATCH 0/1] MR498: server: Allow creating named pipes using \Device\NamedPipe\ as RootDirectory. (#52105)

[Jinoh Kang (＠iamahuman)]

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

[PATCH 1/1] server: Allow creating named pipes using \Device\NamedPipe\ as RootDirectory.

[Jinoh Kang]

From: Jinoh Kang

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;

[PATCH 1/1] server: Allow creating named pipes using \Device\NamedPipe\ as RootDirectory.

[Zebediah Figura]

On 7/21/22 08:37, Jinoh Kang wrote:
> From: Jinoh Kang
>
> 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?

[PATCH 1/1] server: Allow creating named pipes using \Device\NamedPipe\ as RootDirectory.

[Jin-oh Kang]

On Sun, Oct 9, 2022 at 2:06 AM Zebediah Figura  wrote:
> On 7/21/22 08:37, Jinoh Kang wrote:
> > From: Jinoh Kang
> >
> > 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.
>

[PATCH 1/1] server: Allow creating named pipes using \Device\NamedPipe\ as RootDirectory.

[Zebediah Figura]

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?

[PATCH 1/1] server: Allow creating named pipes using \Device\NamedPipe\ as RootDirectory.

[Jin-oh Kang]

On Tue, Oct 11, 2022, 2:56 AM Zebediah Figura 
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?
>

[PATCH 1/1] server: Allow creating named pipes using \Device\NamedPipe\ as RootDirectory.

[Zebediah Figura]

On 10/11/22 07:13, Jin-oh Kang wrote:
> On Tue, Oct 11, 2022, 2:56 AM Zebediah Figura 
> 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 :-)

[PATCH 1/1] server: Allow creating named pipes using \Device\NamedPipe\ as RootDirectory.

[Jin-oh Kang]

On Sun, Oct 9, 2022 at 2:06 AM Zebediah Figura  wrote:
> On 7/21/22 08:37, Jinoh Kang wrote:
> > From: Jinoh Kang
> >
> > 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.

[PATCH 1/1] server: Allow creating named pipes using \Device\NamedPipe\ as RootDirectory.

[Jin-oh Kang]

On Tue, Oct 11, 2022 at 12:22 AM Jin-oh Kang  wrote:
> On Sun, Oct 9, 2022 at 2:06 AM Zebediah Figura  wrote:
> > On 7/21/22 08:37, Jinoh Kang wrote:
> > > From: Jinoh Kang
> > >
> > > 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*

[PATCH 1/1] server: Allow creating named pipes using \Device\NamedPipe\ as RootDirectory.

[Zebediah Figura]

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

[Joel Holdsworth (＠jhol)]

Simple patch. Looks good to me, and it fixes the related issue in Msys2.

[Joel Holdsworth (＠jhol)]

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.