[PATCH v8 0/2] 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. --- **Note**: `subpath->str` may look hacky, but it's used to indicate trailing `\\`[^bks] and existing code already does it too: - `server/directory.c` checks `name->str`[^dir] to test if this is the last component. This is evident by the fact that changing it to `name->len` will lead to major regression. - `server/registry.c` has similar lookup logic.[^reg1][^reg2] [^bks]: https://gitlab.winehq.org/wine/wine/-/blob/c64aa0006e4a33d755a57a693cd81dc1e... [^dir]: https://gitlab.winehq.org/wine/wine/-/blob/c64aa0006e4a33d755a57a693cd81dc1e... [^reg1]: <https://gitlab.winehq.org/wine/wine/-/blob/c64aa0006e4a33d755a57a693cd81dc1ed95fa9d/server/registry.c#L521> [^reg2]: <https://gitlab.winehq.org/wine/wine/-/blob/c64aa0006e4a33d755a57a693cd81dc1ed95fa9d/server/registry.c#L531> -- v8: server: Implement more FSCTLs on \Device\NamedPipe and \Device\NamedPipe\. server: Allow creating named pipes using \Device\NamedPipe\ as RootDirectory. https://gitlab.winehq.org/wine/wine/-/merge_requests/498
From: Jinoh Kang <jinoh.kang.kr(a)gmail.com> Separate the named pipe root directory from the named pipe device file. Open the root directory instead of the device file if the path ends with backslash. Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=52105 Signed-off-by: Jinoh Kang <jinoh.kang.kr(a)gmail.com> --- dlls/ntdll/tests/om.c | 2 +- dlls/ntdll/tests/pipe.c | 6 +-- server/named_pipe.c | 117 ++++++++++++++++++++++++++++++++++++++-- 3 files changed, 118 insertions(+), 7 deletions(-) diff --git a/dlls/ntdll/tests/om.c b/dlls/ntdll/tests/om.c index 82f49376a97..d500a32cde2 100644 --- a/dlls/ntdll/tests/om.c +++ b/dlls/ntdll/tests/om.c @@ -1966,7 +1966,7 @@ static void test_query_object(void) handle = CreateFileA( "\\\\.\\pipe\\", 0, 0, NULL, OPEN_EXISTING, 0, 0 ); ok( handle != INVALID_HANDLE_VALUE, "CreateFile failed (%lu)\n", GetLastError() ); - test_object_name( handle, L"\\Device\\NamedPipe\\", TRUE ); + test_object_name( handle, L"\\Device\\NamedPipe\\", FALSE ); test_object_type( handle, L"File" ); test_file_info( handle ); diff --git a/dlls/ntdll/tests/pipe.c b/dlls/ntdll/tests/pipe.c index cdf5cdb4151..6816d7d4e2d 100644 --- a/dlls/ntdll/tests/pipe.c +++ b/dlls/ntdll/tests/pipe.c @@ -2775,7 +2775,7 @@ static void test_empty_name(void) pRtlInitUnicodeString(&name, L"nonexistent_pipe"); status = wait_pipe(hdirectory, &name, &zero_timeout); - todo_wine ok(status == STATUS_ILLEGAL_FUNCTION, "unexpected status for FSCTL_PIPE_WAIT on \\Device\\NamedPipe: %#lx\n", status); + ok(status == STATUS_ILLEGAL_FUNCTION, "unexpected status for FSCTL_PIPE_WAIT on \\Device\\NamedPipe: %#lx\n", status); subtest_empty_name_pipe_operations(hdirectory); @@ -2920,11 +2920,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 dd8c14b30a9..ef87401e4e8 100644 --- a/server/named_pipe.c +++ b/server/named_pipe.c @@ -321,6 +321,39 @@ static const struct fd_ops named_pipe_device_fd_ops = default_fd_reselect_async /* reselect_async */ }; +static void named_pipe_dir_dump( struct object *obj, int verbose ); +static struct fd *named_pipe_dir_get_fd( struct object *obj ); +static WCHAR *named_pipe_dir_get_full_name( struct object *obj, data_size_t *ret_len ); +static struct object *named_pipe_dir_lookup_name( struct object *obj, struct unicode_str *name, + unsigned int attr, struct object *root ); +static struct object *named_pipe_dir_open_file( struct object *obj, unsigned int access, + unsigned int sharing, unsigned int options ); +static void named_pipe_dir_destroy( struct object *obj ); + +static const struct object_ops named_pipe_dir_ops = +{ + sizeof(struct named_pipe_device_file), /* size */ + &file_type, /* type */ + named_pipe_dir_dump, /* dump */ + add_queue, /* add_queue */ + remove_queue, /* remove_queue */ + default_fd_signaled, /* signaled */ + no_satisfied, /* satisfied */ + no_signal, /* signal */ + named_pipe_dir_get_fd, /* get_fd */ + default_map_access, /* map_access */ + default_get_sd, /* get_sd */ + default_set_sd, /* set_sd */ + named_pipe_dir_get_full_name, /* get_full_name */ + named_pipe_dir_lookup_name, /* lookup_name */ + no_link_name, /* link_name */ + NULL, /* unlink_name */ + named_pipe_dir_open_file, /* open_file */ + no_kernel_obj_list, /* get_kernel_obj_list */ + no_close_handle, /* close_handle */ + named_pipe_dir_destroy /* destroy */ +}; + static void named_pipe_dump( struct object *obj, int verbose ) { fputs( "Named pipe\n", stderr ); @@ -501,6 +534,18 @@ static struct object *named_pipe_device_lookup_name( struct object *obj, struct assert( device->pipes ); if (!name) return NULL; /* open the device itself */ + if (!name->len && name->str) + { + /* open the root directory */ + struct named_pipe_device_file *dir = alloc_object( &named_pipe_dir_ops ); + + if (!dir) return NULL; + + dir->fd = NULL; /* defer alloc_pseudo_fd() until after we have options */ + dir->device = (struct named_pipe_device *)grab_object( obj ); + + return &dir->obj; + } if ((found = find_object( device->pipes, name, attr | OBJ_CASE_INSENSITIVE ))) name->len = 0; @@ -581,6 +626,70 @@ static void named_pipe_device_file_destroy( struct object *obj ) release_object( file->device ); } +static void named_pipe_dir_dump( struct object *obj, int verbose ) +{ + struct named_pipe_device_file *dir = (struct named_pipe_device_file *)obj; + + fprintf( stderr, "Root directory of named pipe device %p\n", dir->device ); +} + +static struct fd *named_pipe_dir_get_fd( struct object *obj ) +{ + struct named_pipe_device_file *dir = (struct named_pipe_device_file *)obj; + return (struct fd *)grab_object( dir->fd ); +} + +static WCHAR *named_pipe_dir_get_full_name( struct object *obj, data_size_t *ret_len ) +{ + struct named_pipe_device_file *dir = (struct named_pipe_device_file *)obj; + data_size_t len; + char *device_name, *ret; + + device_name = (char *)dir->device->obj.ops->get_full_name( &dir->device->obj, &len ); + if (!device_name) return NULL; + + len += sizeof(WCHAR); + ret = realloc(device_name, len); + if (!ret) + { + free(device_name); + return NULL; + } + *(WCHAR *)(ret + len - sizeof(WCHAR)) = '\\'; + + *ret_len = len; + return (WCHAR *)ret; +} + +static struct object *named_pipe_dir_lookup_name( struct object *obj, struct unicode_str *name, + unsigned int attr, struct object *root ) +{ + struct named_pipe_device_file *dir = (struct named_pipe_device_file *)obj; + if (!name || !name->len) return NULL; /* open the directory itself */ + return dir->device->obj.ops->lookup_name( &dir->device->obj, name, attr, root ); +} + +static struct object *named_pipe_dir_open_file( struct object *obj, unsigned int access, + unsigned int sharing, unsigned int options ) +{ + struct named_pipe_device_file *dir = (struct named_pipe_device_file *)obj; + if (!dir->fd) + { + if (!(dir->fd = alloc_pseudo_fd( &named_pipe_device_fd_ops, obj, options ))) + return NULL; + allow_fd_caching( dir->fd ); + } + return grab_object( obj ); +} + +static void named_pipe_dir_destroy( struct object *obj ) +{ + struct named_pipe_device_file *file = (struct named_pipe_device_file*)obj; + assert( obj->ops == &named_pipe_dir_ops ); + if (file->fd) release_object( file->fd ); + if (file->device) release_object( file->device ); +} + static void pipe_end_flush( struct fd *fd, struct async *async ) { struct pipe_end *pipe_end = get_fd_user( fd ); @@ -1321,14 +1430,13 @@ static struct pipe_end *create_pipe_client( struct named_pipe *pipe, data_size_t static int named_pipe_link_name( struct object *obj, struct object_name *name, struct object *parent ) { - struct named_pipe_device *dev = (struct named_pipe_device *)parent; - + if (parent->ops == &named_pipe_dir_ops) parent = &((struct named_pipe_device_file *)parent)->device->obj; if (parent->ops != &named_pipe_device_ops) { set_error( STATUS_OBJECT_NAME_INVALID ); return 0; } - namespace_add( dev->pipes, name ); + namespace_add( ((struct named_pipe_device *)parent)->pipes, name ); name->parent = grab_object( parent ); return 1; } @@ -1376,6 +1484,7 @@ static void named_pipe_device_ioctl( struct fd *fd, ioctl_code_t code, struct as switch(code) { case FSCTL_PIPE_WAIT: + if (device->obj.ops == &named_pipe_dir_ops) { const FILE_PIPE_WAIT_FOR_BUFFER *buffer = get_req_data(); data_size_t size = get_req_data_size(); @@ -1404,6 +1513,8 @@ static void named_pipe_device_ioctl( struct fd *fd, ioctl_code_t code, struct as release_object( pipe ); return; } + set_error( STATUS_ILLEGAL_FUNCTION ); + return; default: default_fd_ioctl( fd, code, async ); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/498
From: Jinoh Kang <jinoh.kang.kr(a)gmail.com> --- dlls/ntdll/tests/pipe.c | 1 - server/named_pipe.c | 14 ++++++++++++++ 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/dlls/ntdll/tests/pipe.c b/dlls/ntdll/tests/pipe.c index 6816d7d4e2d..df693d07cbd 100644 --- a/dlls/ntdll/tests/pipe.c +++ b/dlls/ntdll/tests/pipe.c @@ -2731,7 +2731,6 @@ static void subtest_empty_name_pipe_operations(HANDLE handle) WaitForSingleObject(event, INFINITE); status = io.Status; } - todo_wine_if(ft->status != STATUS_NOT_SUPPORTED) ok(status == ft->status || (ft->status_broken && broken(status == ft->status_broken)), "NtFsControlFile(%s) on \\Device\\NamedPipe: expected %#lx, got %#lx\n", ft->name, ft->status, status); diff --git a/server/named_pipe.c b/server/named_pipe.c index ef87401e4e8..18b8072287d 100644 --- a/server/named_pipe.c +++ b/server/named_pipe.c @@ -1516,6 +1516,20 @@ static void named_pipe_device_ioctl( struct fd *fd, ioctl_code_t code, struct as set_error( STATUS_ILLEGAL_FUNCTION ); return; + case FSCTL_PIPE_LISTEN: + case FSCTL_PIPE_IMPERSONATE: + set_error( STATUS_ILLEGAL_FUNCTION ); + return; + + case FSCTL_PIPE_DISCONNECT: + case FSCTL_PIPE_TRANSCEIVE: + set_error( STATUS_PIPE_DISCONNECTED ); + return; + + case FSCTL_PIPE_QUERY_CLIENT_PROCESS: + set_error( STATUS_INVALID_PARAMETER ); + return; + default: default_fd_ioctl( fd, code, async ); } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/498
v8: Revert to the previous approach of separate ops for named pipe directory, from March 2022[^1], with a few tweaks to highlight similarities between `\Device\NamedPipe` and `\Device\NamedPipe\`. Unlike the mailing list patch[^1], the two object types still share the same fd ops (as per @zfigura's suggestion: https://gitlab.winehq.org/wine/wine/-/merge_requests/498?diff_id=86003#note_...). This requires an explicit check (`device->obj.ops == &named_pipe_dir_ops`) from `named_pipe_device_ioctl` on the fd user object's ops, which is what I meant by "the 'boolean flag' pattern getting moved to the fd object, not eliminated." 1. The explicit check (`device->obj.ops == &named_pipe_dir_ops`) resides in `named_pipe_device_ioctl`. This is because `FSCTL_PIPE_WAIT` works only on `\Device\NamedPipe\`. 2. `\Device\NamedPipe\` supports `NtQueryDirectoryFile()`[^2] on native but not yet on Wine. To implement this, I believe we should switch to `FD_TYPE_DIR` for `\Device\NamedPipe\` as well as implement (hypothetical) "list directory" operation, both of which will require the explicit check as well. Both of the concerns above can be resolved if we duplicate the fd ops as well, which I'm happy to implement. Nevertheless, if this looks OK, let's go with this. [^1]: [[PATCH v4 resend 2/2] server: Allow creating named pipes using \Device\NamedPipe\ as RootDirectory.](https://marc.info/?l=wine-devel&m=164813696830725&w=2) Jinoh Kang. March 25 2022. [^2]: Sysinternals `pipelist.exe` is known to use this to list existing named pipes on a system. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/498#note_85517
Both of the concerns above can be resolved if we duplicate the fd ops as well, which I'm happy to implement.
I expect that this would be cleaner. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/498#note_85599
participants (3)
-
Alexandre Julliard (＠julliard) -
Jinoh Kang
-
Jinoh Kang (＠iamahuman)