[PATCH v6 0/5] 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> -- v6: server: Allow creating named pipes using \Device\NamedPipe\ as RootDirectory. server: Add stub lookup_name implementation for named_pipe_device_file_ops. server: Pass the object subpath to open_file instead of rejecting it. server: Inline open_named_object() call in open_file_object handler. ntdll/tests: Test opening a named pipe with \Device\NamedPipe\ as RootDirectory. https://gitlab.winehq.org/wine/wine/-/merge_requests/498
From: Jinoh Kang <jinoh.kang.kr(a)gmail.com> --- dlls/ntdll/tests/pipe.c | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/dlls/ntdll/tests/pipe.c b/dlls/ntdll/tests/pipe.c index 697774dfa09..23e14f63e0c 100644 --- a/dlls/ntdll/tests/pipe.c +++ b/dlls/ntdll/tests/pipe.c @@ -2806,6 +2806,24 @@ static void test_empty_name(void) FILE_SHARE_READ|FILE_SHARE_WRITE, FILE_OPEN, 0, NULL, 0 ); ok(!status, "Got unexpected status %#lx.\n", status); + pRtlInitUnicodeString(&name, L"\\Device\\NamedPipe\\winetestxyz"); + attr.RootDirectory = NULL; + attr.ObjectName = &name; + 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); + ok(status == STATUS_SUCCESS, "NtCreateNamedPipeFile \"\\Device\\NamedPipe\\winetestxyz\": %#lx\n", status); + if (status) hpipe = NULL; + + pRtlInitUnicodeString(&name, L"winetestxyz"); + attr.RootDirectory = hdirectory; + attr.ObjectName = &name; + status = NtCreateFile(&handle, SYNCHRONIZE, &attr, &io, NULL, 0, FILE_SHARE_READ | FILE_SHARE_WRITE, FILE_OPEN, 0, NULL, 0 ); + todo_wine + ok(status == STATUS_SUCCESS, "open \"winetestxyz\" root \\Device\\NamedPipe\\: got %#lx.\n", status); + if (!status) NtClose(handle); + if (hpipe) NtClose(hpipe); + pRtlInitUnicodeString(&name, L"nonexistent_pipe"); status = wait_pipe(hdirectory, &name, &zero_timeout); ok(status == STATUS_OBJECT_NAME_NOT_FOUND, "unexpected status for FSCTL_PIPE_WAIT on \\Device\\NamedPipe\\: %#lx\n", status); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/498
From: Jinoh Kang <jinoh.kang.kr(a)gmail.com> Prepare for passing the remaining subpath after object namespace traversal to the open_file() method. --- server/fd.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/server/fd.c b/server/fd.c index f28937466fc..dabba024fb9 100644 --- a/server/fd.c +++ b/server/fd.c @@ -2769,14 +2769,19 @@ DECL_HANDLER(open_file_object) { struct unicode_str name = get_req_unicode_str(); struct object *obj, *result, *root = NULL; + struct unicode_str name_left; if (req->rootdir && !(root = get_handle_obj( current->process, req->rootdir, 0, NULL ))) return; - obj = open_named_object( root, NULL, &name, req->attributes ); + obj = lookup_named_object( root, &name, req->attributes, &name_left ); if (root) release_object( root ); if (!obj) return; - if ((result = obj->ops->open_file( obj, req->access, req->sharing, req->options ))) + if (name_left.len) /* not fully parsed */ + { + set_error( STATUS_OBJECT_NAME_NOT_FOUND ); + } + else if ((result = obj->ops->open_file( obj, req->access, req->sharing, req->options ))) { reply->handle = alloc_handle( current->process, result, req->access, req->attributes ); release_object( result ); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/498
From: Jinoh Kang <jinoh.kang.kr(a)gmail.com> This is required to emulate Windows NT virtual file systems (e.g., \Device\NamedPipe). In general, Windows file system drivers process pathnames on its own (for IRP_MJ_CREATE), since putting every existing file into the in-memory namespace is slow and highly inefficient. To avoid regression, make the refactoring as close to no-op as possible. Some (subpath->len) checks could turn into an assert, but that seems highly superfluous and we don't know which are actually safe to do so. --- server/console.c | 99 ++++++++++++++++++++++++++++++++------------- server/device.c | 14 +++++-- server/fd.c | 6 +-- server/file.c | 14 +++++-- server/mailslot.c | 28 +++++++++---- server/named_pipe.c | 44 ++++++++++++++------ server/object.c | 9 +++-- server/object.h | 8 ++-- server/sock.c | 14 +++++-- 9 files changed, 163 insertions(+), 73 deletions(-) diff --git a/server/console.c b/server/console.c index b64283baf4a..002a9b30a55 100644 --- a/server/console.c +++ b/server/console.c @@ -69,8 +69,8 @@ static int console_signaled( struct object *obj, struct wait_queue_entry *entry static struct fd *console_get_fd( struct object *obj ); static struct object *console_lookup_name( struct object *obj, struct unicode_str *name, unsigned int attr, struct object *root ); -static struct object *console_open_file( struct object *obj, unsigned int access, - unsigned int sharing, unsigned int options ); +static struct object *console_open_file( struct object *obj, const struct unicode_str *subpath, + unsigned int access, unsigned int sharing, unsigned int options ); static int console_add_queue( struct object *obj, struct wait_queue_entry *entry ); static const struct object_ops console_ops = @@ -147,8 +147,8 @@ static int console_server_signaled( struct object *obj, struct wait_queue_entry static struct fd *console_server_get_fd( struct object *obj ); static struct object *console_server_lookup_name( struct object *obj, struct unicode_str *name, unsigned int attr, struct object *root ); -static struct object *console_server_open_file( struct object *obj, unsigned int access, - unsigned int sharing, unsigned int options ); +static struct object *console_server_open_file( struct object *obj, const struct unicode_str *subpath, + unsigned int access, unsigned int sharing, unsigned int options ); static const struct object_ops console_server_ops = { @@ -216,8 +216,8 @@ static void screen_buffer_dump( struct object *obj, int verbose ); static void screen_buffer_destroy( struct object *obj ); static int screen_buffer_add_queue( struct object *obj, struct wait_queue_entry *entry ); static struct fd *screen_buffer_get_fd( struct object *obj ); -static struct object *screen_buffer_open_file( struct object *obj, unsigned int access, - unsigned int sharing, unsigned int options ); +static struct object *screen_buffer_open_file( struct object *obj, const struct unicode_str *subpath, + unsigned int access, unsigned int sharing, unsigned int options ); static const struct object_ops screen_buffer_ops = { @@ -265,8 +265,8 @@ static const struct fd_ops screen_buffer_fd_ops = static void console_device_dump( struct object *obj, int verbose ); static struct object *console_device_lookup_name( struct object *obj, struct unicode_str *name, unsigned int attr, struct object *root ); -static struct object *console_device_open_file( struct object *obj, unsigned int access, - unsigned int sharing, unsigned int options ); +static struct object *console_device_open_file( struct object *obj, const struct unicode_str *subpath, + unsigned int access, unsigned int sharing, unsigned int options ); static const struct object_ops console_device_ops = { @@ -299,8 +299,8 @@ struct console_input }; static void console_input_dump( struct object *obj, int verbose ); -static struct object *console_input_open_file( struct object *obj, unsigned int access, - unsigned int sharing, unsigned int options ); +static struct object *console_input_open_file( struct object *obj, const struct unicode_str *subpath, + unsigned int access, unsigned int sharing, unsigned int options ); static int console_input_add_queue( struct object *obj, struct wait_queue_entry *entry ); static struct fd *console_input_get_fd( struct object *obj ); static void console_input_destroy( struct object *obj ); @@ -358,8 +358,8 @@ struct console_output static void console_output_dump( struct object *obj, int verbose ); static int console_output_add_queue( struct object *obj, struct wait_queue_entry *entry ); static struct fd *console_output_get_fd( struct object *obj ); -static struct object *console_output_open_file( struct object *obj, unsigned int access, - unsigned int sharing, unsigned int options ); +static struct object *console_output_open_file( struct object *obj, const struct unicode_str *subpath, + unsigned int access, unsigned int sharing, unsigned int options ); static void console_output_destroy( struct object *obj ); static const struct object_ops console_output_ops = @@ -415,8 +415,8 @@ static void console_connection_dump( struct object *obj, int verbose ); static struct fd *console_connection_get_fd( struct object *obj ); static struct object *console_connection_lookup_name( struct object *obj, struct unicode_str *name, unsigned int attr, struct object *root ); -static struct object *console_connection_open_file( struct object *obj, unsigned int access, - unsigned int sharing, unsigned int options ); +static struct object *console_connection_open_file( struct object *obj, const struct unicode_str *subpath, + unsigned int access, unsigned int sharing, unsigned int options ); static int console_connection_close_handle( struct object *obj, struct process *process, obj_handle_t handle ); static void console_connection_destroy( struct object *obj ); @@ -805,9 +805,15 @@ static struct object *console_lookup_name( struct object *obj, struct unicode_st return NULL; } -static struct object *console_open_file( struct object *obj, unsigned int access, - unsigned int sharing, unsigned int options ) +static struct object *console_open_file( struct object *obj, const struct unicode_str *subpath, + unsigned int access, unsigned int sharing, unsigned int options ) { + if (subpath->len) + { + set_error( STATUS_OBJECT_NAME_NOT_FOUND ); + return NULL; + } + return grab_object( obj ); } @@ -833,9 +839,15 @@ static void screen_buffer_destroy( struct object *obj ) free_async_queue( &screen_buffer->ioctl_q ); } -static struct object *screen_buffer_open_file( struct object *obj, unsigned int access, - unsigned int sharing, unsigned int options ) +static struct object *screen_buffer_open_file( struct object *obj, const struct unicode_str *subpath, + unsigned int access, unsigned int sharing, unsigned int options ) { + if (subpath->len) + { + set_error( STATUS_OBJECT_NAME_NOT_FOUND ); + return NULL; + } + return grab_object( obj ); } @@ -920,9 +932,15 @@ static struct fd *console_server_get_fd( struct object* obj ) return (struct fd *)grab_object( server->fd ); } -static struct object *console_server_open_file( struct object *obj, unsigned int access, - unsigned int sharing, unsigned int options ) +static struct object *console_server_open_file( struct object *obj, const struct unicode_str *subpath, + unsigned int access, unsigned int sharing, unsigned int options ) { + if (subpath->len) + { + set_error( STATUS_OBJECT_NAME_NOT_FOUND ); + return NULL; + } + return grab_object( obj ); } @@ -1230,9 +1248,15 @@ static struct object *console_connection_lookup_name( struct object *obj, struct return NULL; } -static struct object *console_connection_open_file( struct object *obj, unsigned int access, - unsigned int sharing, unsigned int options ) +static struct object *console_connection_open_file( struct object *obj, const struct unicode_str *subpath, + unsigned int access, unsigned int sharing, unsigned int options ) { + if (subpath->len) + { + set_error( STATUS_OBJECT_NAME_NOT_FOUND ); + return NULL; + } + return grab_object( obj ); } @@ -1355,10 +1379,17 @@ static struct object *console_device_lookup_name( struct object *obj, struct uni return NULL; } -static struct object *console_device_open_file( struct object *obj, unsigned int access, - unsigned int sharing, unsigned int options ) +static struct object *console_device_open_file( struct object *obj, const struct unicode_str *subpath, + unsigned int access, unsigned int sharing, unsigned int options ) { int is_output; + + if (subpath->len) + { + set_error( STATUS_OBJECT_NAME_NOT_FOUND ); + return NULL; + } + access = default_map_access( obj, access ); is_output = access & FILE_WRITE_DATA; if (!current->process->console || (is_output && !current->process->console)) @@ -1396,9 +1427,15 @@ static struct fd *console_input_get_fd( struct object *obj ) return (struct fd *)grab_object( console_input->fd ); } -static struct object *console_input_open_file( struct object *obj, unsigned int access, - unsigned int sharing, unsigned int options ) +static struct object *console_input_open_file( struct object *obj, const struct unicode_str *subpath, + unsigned int access, unsigned int sharing, unsigned int options ) { + if (subpath->len) + { + set_error( STATUS_OBJECT_NAME_NOT_FOUND ); + return NULL; + } + return grab_object( obj ); } @@ -1468,9 +1505,15 @@ static struct fd *console_output_get_fd( struct object *obj ) return (struct fd *)grab_object( console_output->fd ); } -static struct object *console_output_open_file( struct object *obj, unsigned int access, - unsigned int sharing, unsigned int options ) +static struct object *console_output_open_file( struct object *obj, const struct unicode_str *subpath, + unsigned int access, unsigned int sharing, unsigned int options ) { + if (subpath->len) + { + set_error( STATUS_OBJECT_NAME_NOT_FOUND ); + return NULL; + } + return grab_object( obj ); } diff --git a/server/device.c b/server/device.c index 436dac6bfe9..32f8e217b86 100644 --- a/server/device.c +++ b/server/device.c @@ -151,8 +151,8 @@ struct device static void device_dump( struct object *obj, int verbose ); static void device_destroy( struct object *obj ); -static struct object *device_open_file( struct object *obj, unsigned int access, - unsigned int sharing, unsigned int options ); +static struct object *device_open_file( struct object *obj, const struct unicode_str *subpath, + unsigned int access, unsigned int sharing, unsigned int options ); static struct list *device_get_kernel_obj_list( struct object *obj ); static const struct object_ops device_ops = @@ -420,13 +420,19 @@ static void add_irp_to_queue( struct device_manager *manager, struct irp_call *i if (list_head( &manager->requests ) == &irp->mgr_entry) wake_up( &manager->obj, 0 ); /* first one */ } -static struct object *device_open_file( struct object *obj, unsigned int access, - unsigned int sharing, unsigned int options ) +static struct object *device_open_file( struct object *obj, const struct unicode_str *subpath, + unsigned int access, unsigned int sharing, unsigned int options ) { struct device *device = (struct device *)obj; struct device_file *file; struct unicode_str nt_name; + if (subpath->len) /* TODO: handle non-empty filenames (filesystem device) */ + { + set_error( STATUS_OBJECT_NAME_NOT_FOUND ); + return NULL; + } + if (!(file = alloc_object( &device_file_ops ))) return NULL; file->device = (struct device *)grab_object( device ); diff --git a/server/fd.c b/server/fd.c index dabba024fb9..a39215a8d1e 100644 --- a/server/fd.c +++ b/server/fd.c @@ -2777,11 +2777,7 @@ DECL_HANDLER(open_file_object) if (root) release_object( root ); if (!obj) return; - if (name_left.len) /* not fully parsed */ - { - set_error( STATUS_OBJECT_NAME_NOT_FOUND ); - } - else if ((result = obj->ops->open_file( obj, req->access, req->sharing, req->options ))) + if ((result = obj->ops->open_file( obj, &name_left, req->access, req->sharing, req->options ))) { reply->handle = alloc_handle( current->process, result, req->access, req->attributes ); release_object( result ); diff --git a/server/file.c b/server/file.c index 76c687833c9..6d2fc1e747a 100644 --- a/server/file.c +++ b/server/file.c @@ -79,8 +79,8 @@ static struct security_descriptor *file_get_sd( struct object *obj ); static int file_set_sd( struct object *obj, const struct security_descriptor *sd, unsigned int set_info ); static struct object *file_lookup_name( struct object *obj, struct unicode_str *name, unsigned int attr, struct object *root ); -static struct object *file_open_file( struct object *obj, unsigned int access, - unsigned int sharing, unsigned int options ); +static struct object *file_open_file( struct object *obj, const struct unicode_str *subpath, + unsigned int access, unsigned int sharing, unsigned int options ); static struct list *file_get_kernel_obj_list( struct object *obj ); static void file_destroy( struct object *obj ); @@ -556,8 +556,8 @@ static struct object *file_lookup_name( struct object *obj, struct unicode_str * return NULL; } -static struct object *file_open_file( struct object *obj, unsigned int access, - unsigned int sharing, unsigned int options ) +static struct object *file_open_file( struct object *obj, const struct unicode_str *subpath, + unsigned int access, unsigned int sharing, unsigned int options ) { struct file *file = (struct file *)obj; struct object *new_file = NULL; @@ -566,6 +566,12 @@ static struct object *file_open_file( struct object *obj, unsigned int access, assert( obj->ops == &file_ops ); + if (subpath->len) + { + set_error( STATUS_OBJECT_NAME_NOT_FOUND ); + return NULL; + } + if ((unix_name = dup_fd_name( file->fd, "" ))) { get_nt_name( file->fd, &nt_name ); diff --git a/server/mailslot.c b/server/mailslot.c index 2d8697ec9bd..58e2b1a2ceb 100644 --- a/server/mailslot.c +++ b/server/mailslot.c @@ -62,8 +62,8 @@ static void mailslot_dump( struct object*, int ); static struct fd *mailslot_get_fd( struct object * ); static unsigned int mailslot_map_access( struct object *obj, unsigned int access ); static int mailslot_link_name( struct object *obj, struct object_name *name, struct object *parent ); -static struct object *mailslot_open_file( struct object *obj, unsigned int access, - unsigned int sharing, unsigned int options ); +static struct object *mailslot_open_file( struct object *obj, const struct unicode_str *subpath, + unsigned int access, unsigned int sharing, unsigned int options ); static void mailslot_destroy( struct object * ); static const struct object_ops mailslot_ops = @@ -184,8 +184,8 @@ struct mailslot_device_file static void mailslot_device_dump( struct object *obj, int verbose ); static struct object *mailslot_device_lookup_name( struct object *obj, struct unicode_str *name, unsigned int attr, struct object *root ); -static struct object *mailslot_device_open_file( struct object *obj, unsigned int access, - unsigned int sharing, unsigned int options ); +static struct object *mailslot_device_open_file( struct object *obj, const struct unicode_str *subpath, + unsigned int access, unsigned int sharing, unsigned int options ); static void mailslot_device_destroy( struct object *obj ); static const struct object_ops mailslot_device_ops = @@ -313,13 +313,19 @@ static int mailslot_link_name( struct object *obj, struct object_name *name, str return 1; } -static struct object *mailslot_open_file( struct object *obj, unsigned int access, - unsigned int sharing, unsigned int options ) +static struct object *mailslot_open_file( struct object *obj, const struct unicode_str *subpath, + unsigned int access, unsigned int sharing, unsigned int options ) { struct mailslot *mailslot = (struct mailslot *)obj; struct mail_writer *writer; int unix_fd; + if (subpath->len) + { + set_error( STATUS_OBJECT_NAME_NOT_FOUND ); + return NULL; + } + if (!(sharing & FILE_SHARE_READ)) { set_error( STATUS_SHARING_VIOLATION ); @@ -399,11 +405,17 @@ static struct object *mailslot_device_lookup_name( struct object *obj, struct un return found; } -static struct object *mailslot_device_open_file( struct object *obj, unsigned int access, - unsigned int sharing, unsigned int options ) +static struct object *mailslot_device_open_file( struct object *obj, const struct unicode_str *subpath, + unsigned int access, unsigned int sharing, unsigned int options ) { struct mailslot_device_file *file; + if (subpath->len) + { + set_error( STATUS_OBJECT_NAME_NOT_FOUND ); + return NULL; + } + if (!(file = alloc_object( &mailslot_device_file_ops ))) return NULL; file->device = (struct mailslot_device *)grab_object( obj ); if (!(file->fd = alloc_pseudo_fd( &mailslot_device_fd_ops, obj, options ))) diff --git a/server/named_pipe.c b/server/named_pipe.c index f3404a33c3b..e30bb4297fa 100644 --- a/server/named_pipe.c +++ b/server/named_pipe.c @@ -107,8 +107,8 @@ static void named_pipe_dump( struct object *obj, int verbose ); static unsigned int named_pipe_map_access( struct object *obj, unsigned int access ); static WCHAR *named_pipe_get_full_name( struct object *obj, data_size_t *ret_len ); static int named_pipe_link_name( struct object *obj, struct object_name *name, struct object *parent ); -static struct object *named_pipe_open_file( struct object *obj, unsigned int access, - unsigned int sharing, unsigned int options ); +static struct object *named_pipe_open_file( struct object *obj, const struct unicode_str *subpath, + unsigned int access, unsigned int sharing, unsigned int options ); static void named_pipe_destroy( struct object *obj ); static const struct object_ops named_pipe_ops = @@ -154,8 +154,8 @@ static void pipe_end_get_file_info( struct fd *fd, obj_handle_t handle, unsigned static void pipe_server_dump( struct object *obj, int verbose ); static struct object *pipe_server_lookup_name( struct object *obj, struct unicode_str *name, unsigned int attr, struct object *root ); -static struct object *pipe_server_open_file( struct object *obj, unsigned int access, - unsigned int sharing, unsigned int options ); +static struct object *pipe_server_open_file( struct object *obj, const struct unicode_str *subpath, + unsigned int access, unsigned int sharing, unsigned int options ); static void pipe_server_destroy( struct object *obj); static void pipe_server_ioctl( struct fd *fd, ioctl_code_t code, struct async *async ); @@ -246,8 +246,8 @@ static const struct fd_ops pipe_client_fd_ops = static void named_pipe_device_dump( struct object *obj, int verbose ); static struct object *named_pipe_device_lookup_name( struct object *obj, struct unicode_str *name, unsigned int attr, struct object *root ); -static struct object *named_pipe_device_open_file( struct object *obj, unsigned int access, - unsigned int sharing, unsigned int options ); +static struct object *named_pipe_device_open_file( struct object *obj, const struct unicode_str *subpath, + unsigned int access, unsigned int sharing, unsigned int options ); static void named_pipe_device_destroy( struct object *obj ); static const struct object_ops named_pipe_device_ops = @@ -462,12 +462,18 @@ static struct object *pipe_server_lookup_name( struct object *obj, struct unicod return NULL; } -static struct object *pipe_server_open_file( struct object *obj, unsigned int access, - unsigned int sharing, unsigned int options ) +static struct object *pipe_server_open_file( struct object *obj, const struct unicode_str *subpath, + unsigned int access, unsigned int sharing, unsigned int options ) { struct pipe_server *server = (struct pipe_server *)obj; - return server->pipe_end.pipe->obj.ops->open_file( &server->pipe_end.pipe->obj, access, sharing, options ); + if (subpath->len) + { + set_error( STATUS_OBJECT_NAME_NOT_FOUND ); + return NULL; + } + + return server->pipe_end.pipe->obj.ops->open_file( &server->pipe_end.pipe->obj, subpath, access, sharing, options ); } static void pipe_server_destroy( struct object *obj ) @@ -508,11 +514,17 @@ static struct object *named_pipe_device_lookup_name( struct object *obj, struct return found; } -static struct object *named_pipe_device_open_file( struct object *obj, unsigned int access, - unsigned int sharing, unsigned int options ) +static struct object *named_pipe_device_open_file( struct object *obj, const struct unicode_str *subpath, + unsigned int access, unsigned int sharing, unsigned int options ) { struct named_pipe_device_file *file; + if (subpath->len) + { + set_error( STATUS_OBJECT_NAME_NOT_FOUND ); + return NULL; + } + if (!(file = alloc_object( &named_pipe_device_file_ops ))) return NULL; file->device = (struct named_pipe_device *)grab_object( obj ); if (!(file->fd = alloc_pseudo_fd( &named_pipe_device_fd_ops, obj, options ))) @@ -1333,14 +1345,20 @@ static int named_pipe_link_name( struct object *obj, struct object_name *name, s return 1; } -static struct object *named_pipe_open_file( struct object *obj, unsigned int access, - unsigned int sharing, unsigned int options ) +static struct object *named_pipe_open_file( struct object *obj, const struct unicode_str *subpath, + unsigned int access, unsigned int sharing, unsigned int options ) { struct named_pipe *pipe = (struct named_pipe *)obj; struct pipe_server *server; struct pipe_end *client; unsigned int pipe_sharing; + if (subpath->len) + { + set_error( STATUS_OBJECT_NAME_NOT_FOUND ); + return NULL; + } + if (list_empty( &pipe->listeners )) { set_error( STATUS_PIPE_NOT_AVAILABLE ); diff --git a/server/object.c b/server/object.c index 459ead5f3a5..20877cb129c 100644 --- a/server/object.c +++ b/server/object.c @@ -702,10 +702,13 @@ void default_unlink_name( struct object *obj, struct object_name *name ) list_remove( &name->entry ); } -struct object *no_open_file( struct object *obj, unsigned int access, unsigned int sharing, - unsigned int options ) +struct object *no_open_file( struct object *obj, const struct unicode_str *subpath, + unsigned int access, unsigned int sharing, unsigned int options ) { - set_error( STATUS_OBJECT_TYPE_MISMATCH ); + if (subpath->len) /* not fully parsed */ + set_error( STATUS_OBJECT_NAME_NOT_FOUND ); + else + set_error( STATUS_OBJECT_TYPE_MISMATCH ); return NULL; } diff --git a/server/object.h b/server/object.h index d4d66536b81..61221728f46 100644 --- a/server/object.h +++ b/server/object.h @@ -99,8 +99,8 @@ struct object_ops /* unlink an object's name from its parent */ void (*unlink_name)(struct object *, struct object_name *); /* open a file object to access this object */ - struct object *(*open_file)(struct object *, unsigned int access, unsigned int sharing, - unsigned int options); + struct object *(*open_file)(struct object *, const struct unicode_str *filename, + unsigned int access, unsigned int sharing, unsigned int options); /* return list of kernel objects */ struct list *(*get_kernel_obj_list)(struct object *); /* close a handle to this object */ @@ -178,8 +178,8 @@ extern struct object *no_lookup_name( struct object *obj, struct unicode_str *na unsigned int attributes, struct object *root ); extern int no_link_name( struct object *obj, struct object_name *name, struct object *parent ); extern void default_unlink_name( struct object *obj, struct object_name *name ); -extern struct object *no_open_file( struct object *obj, unsigned int access, unsigned int sharing, - unsigned int options ); +extern struct object *no_open_file( struct object *obj, const struct unicode_str *filename, unsigned int access, + unsigned int sharing, unsigned int options ); extern struct list *no_kernel_obj_list( struct object *obj ); extern int no_close_handle( struct object *obj, struct process *process, obj_handle_t handle ); extern void no_destroy( struct object *obj ); diff --git a/server/sock.c b/server/sock.c index 8a95caded8b..e93ac95a7ea 100644 --- a/server/sock.c +++ b/server/sock.c @@ -3770,8 +3770,8 @@ static void sock_release_ifchange( struct sock *sock ) static void socket_device_dump( struct object *obj, int verbose ); static struct object *socket_device_lookup_name( struct object *obj, struct unicode_str *name, unsigned int attr, struct object *root ); -static struct object *socket_device_open_file( struct object *obj, unsigned int access, - unsigned int sharing, unsigned int options ); +static struct object *socket_device_open_file( struct object *obj, const struct unicode_str *subpath, + unsigned int access, unsigned int sharing, unsigned int options ); static const struct object_ops socket_device_ops = { @@ -3809,11 +3809,17 @@ static struct object *socket_device_lookup_name( struct object *obj, struct unic return NULL; } -static struct object *socket_device_open_file( struct object *obj, unsigned int access, - unsigned int sharing, unsigned int options ) +static struct object *socket_device_open_file( struct object *obj, const struct unicode_str *subpath, + unsigned int access, unsigned int sharing, unsigned int options ) { struct sock *sock; + if (subpath->len) + { + set_error( STATUS_OBJECT_NAME_NOT_FOUND ); + return NULL; + } + if (!(sock = create_socket())) return NULL; if (!(sock->fd = alloc_pseudo_fd( &sock_fd_ops, &sock->obj, options ))) { -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/498
From: Jinoh Kang <jinoh.kang.kr(a)gmail.com> Prepare allowing creating named pipes using \Device\NamedPipe\ as RootDirectory. --- server/named_pipe.c | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/server/named_pipe.c b/server/named_pipe.c index e30bb4297fa..2909dbb57c3 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 */ @@ -585,6 +587,12 @@ static enum server_fd_type named_pipe_device_file_get_fd_type( struct fd *fd ) return FD_TYPE_DEVICE; } +static struct object *named_pipe_device_file_lookup_name( struct object *obj, struct unicode_str *name, + unsigned int attr, struct object *root ) +{ + return no_lookup_name( obj, name, attr, root ); +} + static void named_pipe_device_file_destroy( struct object *obj ) { struct named_pipe_device_file *file = (struct named_pipe_device_file*)obj; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/498
From: Jinoh Kang <jinoh.kang.kr(a)gmail.com> Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=52105 Fixes: 2600ecd4edfdb71097105c74312f83845305a4f2 --- dlls/ntdll/tests/om.c | 2 +- dlls/ntdll/tests/pipe.c | 5 ++--- server/named_pipe.c | 46 ++++++++++++++++++++++++++++++++++++----- 3 files changed, 44 insertions(+), 9 deletions(-) diff --git a/dlls/ntdll/tests/om.c b/dlls/ntdll/tests/om.c index fb45fe777b7..6b61cfec52c 100644 --- a/dlls/ntdll/tests/om.c +++ b/dlls/ntdll/tests/om.c @@ -1965,7 +1965,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 23e14f63e0c..931c4b61bca 100644 --- a/dlls/ntdll/tests/pipe.c +++ b/dlls/ntdll/tests/pipe.c @@ -2819,7 +2819,6 @@ static void test_empty_name(void) attr.RootDirectory = hdirectory; attr.ObjectName = &name; status = NtCreateFile(&handle, SYNCHRONIZE, &attr, &io, NULL, 0, FILE_SHARE_READ | FILE_SHARE_WRITE, FILE_OPEN, 0, NULL, 0 ); - todo_wine ok(status == STATUS_SUCCESS, "open \"winetestxyz\" root \\Device\\NamedPipe\\: got %#lx.\n", status); if (!status) NtClose(handle); if (hpipe) NtClose(hpipe); @@ -2929,11 +2928,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 2909dbb57c3..35a198441a6 100644 --- a/server/named_pipe.c +++ b/server/named_pipe.c @@ -99,6 +99,7 @@ struct named_pipe_device struct named_pipe_device_file { struct object obj; /* object header */ + int is_rootdir; /* is root directory file? */ struct fd *fd; /* pseudo-fd for ioctls */ struct named_pipe_device *device; /* named pipe device */ }; @@ -528,6 +529,7 @@ static struct object *named_pipe_device_open_file( struct object *obj, const str } if (!(file = alloc_object( &named_pipe_device_file_ops ))) return NULL; + file->is_rootdir = !!subpath->str; /* TRUE: "\\Device\\NamedPipe\\" (trailing \\) */ file->device = (struct named_pipe_device *)grab_object( obj ); if (!(file->fd = alloc_pseudo_fd( &named_pipe_device_fd_ops, obj, options ))) { @@ -567,7 +569,8 @@ static void named_pipe_device_file_dump( struct object *obj, int verbose ) { struct named_pipe_device_file *file = (struct named_pipe_device_file *)obj; - fprintf( stderr, "File on named pipe device %p\n", file->device ); + fprintf( stderr, "%s on named pipe device %p\n", + file->is_rootdir ? "Root directory" : "File", file->device ); } static struct fd *named_pipe_device_file_get_fd( struct object *obj ) @@ -576,10 +579,32 @@ static struct fd *named_pipe_device_file_get_fd( struct object *obj ) return (struct fd *)grab_object( file->fd ); } -static WCHAR *named_pipe_device_file_get_full_name( struct object *obj, data_size_t *len ) +static WCHAR *named_pipe_device_file_get_full_name( struct object *obj, data_size_t *ret_len ) { struct named_pipe_device_file *file = (struct named_pipe_device_file *)obj; - return file->device->obj.ops->get_full_name( &file->device->obj, len ); + WCHAR *device_name; + data_size_t len; + + device_name = file->device->obj.ops->get_full_name( &file->device->obj, &len ); + if (!device_name) return NULL; + + if (file->is_rootdir) + { + WCHAR *newbuf; + + len += sizeof(WCHAR); + if (!(newbuf = realloc(device_name, len))) + { + free(device_name); + return NULL; + } + + device_name = newbuf; + *(WCHAR *)((char *)device_name + len - sizeof(WCHAR)) = '\\'; + } + + *ret_len = len; + return device_name; } static enum server_fd_type named_pipe_device_file_get_fd_type( struct fd *fd ) @@ -590,6 +615,11 @@ static enum server_fd_type named_pipe_device_file_get_fd_type( struct fd *fd ) 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; + + if (file->is_rootdir) + return grab_object( file->device ); + return no_lookup_name( obj, name, attr, root ); } @@ -1341,14 +1371,20 @@ 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_device_file_ops) + { + struct named_pipe_device_file *file = (struct named_pipe_device_file *)parent; + + if (file->is_rootdir) + 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; } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/498
Sorry for the delay in review, I'll admit it's at least partly because the existing code structure is not the easiest to follow...
In general, Windows file system drivers process pathnames on its own (for IRP_MJ_CREATE), since putting every existing file into the in-memory namespace is slow and highly inefficient.
Well... sort of? Putting every *opened* file in the in-memory namespace is necessary no matter what. Putting every *openable* file path in the in-memory namespace isn't actually necessary. You can create files from lookup_name(). The only problem is that I think this is architecturally ugly (although perhaps that's just because lookup_name() is called what it is). But there's precedent for it, namely console_device_lookup_name(). I explicitly argued against doing this earlier, probably because I didn't realize we had precedent for it. We could also implement this within the existing architecture and also avoid creating objects from lookup_name. In order to do this you'd need to create a separate object (when the named pipe device is created, presumably), which isn't exactly a file *or* a device but is rather analogous to struct named_pipe, so it would have an open_file which would probably return a named_pipe_device_file; a link_name (unless we just bypass that), and named_pipe_device_lookup_name() would return it if and only if there was a trailing backslash. I'm inclined to believe this patch series is better, although it does some arguably unnecessary refactoring. It avoids creating a new object type, avoids the conceptual weirdness of creating objects from lookup_name(), and I think it moves towards making the object lookup simpler and more intuitive. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/498#note_74285
This merge request was approved by Elizabeth Figura. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/498
Should this be enough to fix https://bugs.winehq.org/show_bug.cgi?id=52105 ? Because even with this MR I get the cygwin installer hang. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/498#note_74625
On Fri Jun 28 11:30:26 2024 +0000, Fabian Maurer wrote:
Should this be enough to fix https://bugs.winehq.org/show_bug.cgi?id=52105 ? Because even with this MR I get the cygwin installer hang. As far as I know, it's enough to fix bug #52105 **and** bug #52105 specifically. #52105 is only about NamedPipe\\, not all Cygwin hang causes.
It might be insufficient for fixing Cygwin completely. This MR is not known to fix bug #54511. - #52105 - Cygwin setup hangs **(handle to \\Device\NamedPipe\\ used as the RootDirectory for NtCreateNamedPipeFile)**: Fixed by this MR - #54511 - Cygwin installer hangs during postprocessing step: Might not be fixed by this MR -- https://gitlab.winehq.org/wine/wine/-/merge_requests/498#note_74655
On Fri Jun 28 11:31:13 2024 +0000, Jinoh Kang wrote:
As far as I know, it's enough to fix bug #52105 **and** bug #52105 specifically. #52105 is only about NamedPipe\\, not all Cygwin hang causes. It might be insufficient for fixing Cygwin completely. This MR is not known to fix bug #54511. - #52105 - Cygwin setup hangs **(handle to \\Device\NamedPipe\\ used as the RootDirectory for NtCreateNamedPipeFile)**: Fixed by this MR - #54511 - Cygwin installer hangs during postprocessing step: Might not be fixed by this MR Alright thanks, just wanted to make sure I didn't miss something here.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/498#note_74738
On Thu Oct 3 21:57:39 2024 +0000, Jinoh Kang wrote:
v5 no longer creates an object in `named_pipe_device_lookup_name`. File object is always created in `named_pipe_device_open_file`, just like before. Julliard doesn't like the v5 refactoring, so I think we should revisit this approach.
The design feels unclean, but I don't see anothet way. To reiterate we also have a precedent (create object at lookup) in console.c, so I guess it's fine? -- https://gitlab.winehq.org/wine/wine/-/merge_requests/498#note_84058
participants (4)
-
Elizabeth Figura (＠zfigura) -
Fabian Maurer (＠DarkShadow44)
-
Jinoh Kang
-
Jinoh Kang (＠iamahuman)