[PATCH v9 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> -- v9: 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 | 154 ++++++++++++++++++++++++++++++++++++++-- 3 files changed, 153 insertions(+), 9 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..a628ce96f30 100644 --- a/server/named_pipe.c +++ b/server/named_pipe.c @@ -321,6 +321,56 @@ 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 void named_pipe_dir_ioctl( struct fd *fd, ioctl_code_t code, struct async *async ); +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 const struct fd_ops named_pipe_dir_fd_ops = +{ + default_fd_get_poll_events, /* get_poll_events */ + default_poll_event, /* poll_event */ + NULL, /* get_fd_type */ + no_fd_read, /* read */ + no_fd_write, /* write */ + no_fd_flush, /* flush */ + default_fd_get_file_info, /* get_file_info */ + no_fd_get_volume_info, /* get_volume_info */ + named_pipe_dir_ioctl, /* ioctl */ + default_fd_cancel_async, /* cancel_async */ + default_fd_queue_async, /* queue_async */ + default_fd_reselect_async /* reselect_async */ +}; + static void named_pipe_dump( struct object *obj, int verbose ) { fputs( "Named pipe\n", stderr ); @@ -501,6 +551,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 +643,77 @@ 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) + { + /* Trying to open by (already opened) file object */ + return no_open_file( obj, access, sharing, options ); + } + + /* Turn this "proto-object" into an actual file object */ + + if (!(dir->fd = alloc_pseudo_fd( &named_pipe_dir_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 +1454,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; } @@ -1370,6 +1502,19 @@ static struct object *named_pipe_open_file( struct object *obj, unsigned int acc } static void named_pipe_device_ioctl( struct fd *fd, ioctl_code_t code, struct async *async ) +{ + switch(code) + { + case FSCTL_PIPE_WAIT: + set_error( STATUS_ILLEGAL_FUNCTION ); + return; + + default: + default_fd_ioctl( fd, code, async ); + } +} + +static void named_pipe_dir_ioctl( struct fd *fd, ioctl_code_t code, struct async *async ) { struct named_pipe_device *device = get_fd_user( fd ); @@ -1406,11 +1551,10 @@ static void named_pipe_device_ioctl( struct fd *fd, ioctl_code_t code, struct as } default: - default_fd_ioctl( fd, code, async ); + named_pipe_device_ioctl( fd, code, async ); } } - DECL_HANDLER(create_named_pipe) { struct named_pipe *pipe; -- 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 | 11 +++++++++++ 2 files changed, 11 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 a628ce96f30..8660fb8d7a8 100644 --- a/server/named_pipe.c +++ b/server/named_pipe.c @@ -1506,9 +1506,20 @@ static void named_pipe_device_ioctl( struct fd *fd, ioctl_code_t code, struct as switch(code) { case FSCTL_PIPE_WAIT: + 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
Hi, It looks like your patch introduced the new failures shown below. Please investigate and fix them before resubmitting your patch. If they are not new, fixing them anyway would help a lot. Otherwise please ask for the known failures list to be updated. The tests also ran into some preexisting test failures. If you know how to fix them that would be helpful. See the TestBot job for the details: The full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=149311 Your paranoid android. === debian11b (64 bit WoW report) === user32: input.c:4305: Test succeeded inside todo block: button_down_hwnd_todo 1: got MSG_TEST_WIN hwnd 00000000019700D8, msg WM_LBUTTONDOWN, wparam 0x1, lparam 0x320032
On Fri Oct 25 11:54:08 2024 +0000, Jinoh Kang wrote:
Removed special logic for `is_rootdir = 1` case entirely. The `FSCTL_PIPE_WAIT` test is now left todo. Unresolving, since we are now (technically) back to separate fd_ops.
Feel free to resolve this thread if this is no longer a concern anyway. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/498#note_86059
v9: Use separate fd ops for named pipe directory. Note that the `get_fd_type` slot is set to NULL (since we don't have a unix fd for it). -- https://gitlab.winehq.org/wine/wine/-/merge_requests/498#note_86060
On Fri Oct 25 11:55:19 2024 +0000, Alexandre Julliard wrote:
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. Done.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/498#note_86061
participants (3)
-
Jinoh Kang -
Jinoh Kang (＠iamahuman)
-
Marvin