[PATCH 0/2] MR8984: ntdll: Check signal access rights for inproc syncs.
From: Rémi Bernon <rbernon(a)codeweavers.com> --- server/inproc_sync.c | 7 ++++--- server/object.h | 2 +- server/protocol.def | 4 ++-- server/queue.c | 2 +- 4 files changed, 8 insertions(+), 7 deletions(-) diff --git a/server/inproc_sync.c b/server/inproc_sync.c index f20410c85ed..afad68664d1 100644 --- a/server/inproc_sync.c +++ b/server/inproc_sync.c @@ -87,14 +87,15 @@ static const struct object_ops inproc_sync_ops = inproc_sync_destroy, /* destroy */ }; -struct inproc_sync *create_inproc_event_sync( int manual, int signaled ) +struct inproc_sync *create_inproc_internal_sync( int manual, int signaled ) { struct ntsync_event_args args = {.signaled = signaled, .manual = manual}; struct inproc_sync *event; if (!(event = alloc_object( &inproc_sync_ops ))) return NULL; - event->type = INPROC_SYNC_EVENT; + event->type = INPROC_SYNC_INTERNAL; event->fd = ioctl( get_inproc_device_fd(), NTSYNC_IOC_CREATE_EVENT, &args ); + assert( event->fd != -1 ); return event; } @@ -154,7 +155,7 @@ int get_inproc_device_fd(void) return -1; } -struct inproc_sync *create_inproc_event_sync( int manual, int signaled ) +struct inproc_sync *create_inproc_internal_sync( int manual, int signaled ) { return NULL; } diff --git a/server/object.h b/server/object.h index 51b74429e73..203734a565f 100644 --- a/server/object.h +++ b/server/object.h @@ -244,7 +244,7 @@ extern void abandon_mutexes( struct thread *thread ); struct inproc_sync; extern int get_inproc_device_fd(void); -extern struct inproc_sync *create_inproc_event_sync( int manual, int signaled ); +extern struct inproc_sync *create_inproc_internal_sync( int manual, int signaled ); extern void signal_inproc_sync( struct inproc_sync *sync ); extern void reset_inproc_sync( struct inproc_sync *sync ); diff --git a/server/protocol.def b/server/protocol.def index 6bb7ae155c6..785f0e26d4e 100644 --- a/server/protocol.def +++ b/server/protocol.def @@ -4126,8 +4126,8 @@ struct handle_info enum inproc_sync_type { - INPROC_SYNC_UNKNOWN = 0, - INPROC_SYNC_EVENT = 1, + INPROC_SYNC_UNKNOWN = 0, + INPROC_SYNC_INTERNAL = 1, }; /* Get the in-process synchronization fd associated with the waitable handle */ diff --git a/server/queue.c b/server/queue.c index 596742bc64c..0e286063285 100644 --- a/server/queue.c +++ b/server/queue.c @@ -328,7 +328,7 @@ static struct msg_queue *create_msg_queue( struct thread *thread, struct thread_ list_init( &queue->expired_timers ); for (i = 0; i < NB_MSG_KINDS; i++) list_init( &queue->msg_list[i] ); - if (get_inproc_device_fd() >= 0 && !(queue->inproc_sync = create_inproc_event_sync( 1, 0 ))) goto error; + if (get_inproc_device_fd() >= 0 && !(queue->inproc_sync = create_inproc_internal_sync( 1, 0 ))) goto error; if (!(queue->shared = alloc_shared_object())) goto error; SHARED_WRITE_BEGIN( queue->shared, queue_shm_t ) -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/8984
From: Elizabeth Figura <zfigura(a)codeweavers.com> --- dlls/ntdll/unix/sync.c | 37 ++++++++++++++++++++++++++++++++++--- 1 file changed, 34 insertions(+), 3 deletions(-) diff --git a/dlls/ntdll/unix/sync.c b/dlls/ntdll/unix/sync.c index acdb7b3d9f8..57e67933cd3 100644 --- a/dlls/ntdll/unix/sync.c +++ b/dlls/ntdll/unix/sync.c @@ -309,9 +309,9 @@ static unsigned int validate_open_object_attributes( const OBJECT_ATTRIBUTES *at struct inproc_sync { - int fd; - unsigned int access; - unsigned int type : 2; + int fd; /* unix file descriptor */ + unsigned int access; /* handle access rights */ + unsigned int type; /* enum inproc_sync_type as short to save space */ }; static void release_inproc_sync( struct inproc_sync *sync ) @@ -355,6 +355,18 @@ static NTSTATUS get_inproc_sync( HANDLE handle, ACCESS_MASK desired_access, stru return STATUS_SUCCESS; } +extern unsigned int check_signal_access( struct inproc_sync *sync ) +{ + switch (sync->type) + { + case INPROC_SYNC_INTERNAL: + return STATUS_OBJECT_TYPE_MISMATCH; + } + + assert( 0 ); + return STATUS_OBJECT_TYPE_MISMATCH; +} + static NTSTATUS inproc_release_semaphore( HANDLE handle, ULONG count, ULONG *prev_count ) { if (inproc_device_fd < 0) return STATUS_NOT_IMPLEMENTED; @@ -429,7 +441,26 @@ static NTSTATUS inproc_wait( DWORD count, const HANDLE *handles, BOOLEAN wait_an static NTSTATUS inproc_signal_and_wait( HANDLE signal, HANDLE wait, BOOLEAN alertable, const LARGE_INTEGER *timeout ) { + struct inproc_sync stack_signal, stack_wait, *signal_sync = &stack_signal, *wait_sync = &stack_wait; + NTSTATUS ret; + if (inproc_device_fd < 0) return STATUS_NOT_IMPLEMENTED; + + if ((ret = get_inproc_sync( signal, 0, signal_sync ))) return ret; + if ((ret = check_signal_access( signal_sync ))) + { + release_inproc_sync( signal_sync ); + return ret; + } + + if ((ret = get_inproc_sync( wait, SYNCHRONIZE, wait_sync ))) + { + release_inproc_sync( signal_sync ); + return ret; + } + + release_inproc_sync( signal_sync ); + release_inproc_sync( wait_sync ); return STATUS_NOT_IMPLEMENTED; } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/8984
This merge request was approved by Rémi Bernon. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/8984
Alfred Agrell (@Alcaro) commented about dlls/ntdll/unix/sync.c:
struct inproc_sync { - int fd; - unsigned int access; - unsigned int type : 2; + int fd; /* unix file descriptor */ + unsigned int access; /* handle access rights */ + unsigned int type; /* enum inproc_sync_type as short to save space */
That's a strange way to spell short. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/8984#note_116051
Alfred Agrell (@Alcaro) commented about dlls/ntdll/unix/sync.c:
return STATUS_SUCCESS; }
+extern unsigned int check_signal_access( struct inproc_sync *sync ) +{ + switch (sync->type) + { + case INPROC_SYNC_INTERNAL: + return STATUS_OBJECT_TYPE_MISMATCH;
Shouldn't the return type be NTSTATUS? -- https://gitlab.winehq.org/wine/wine/-/merge_requests/8984#note_116052
No comment on various incomplete pieces, I don't know how to review that. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/8984#note_116053
On Wed Sep 17 08:56:06 2025 +0000, Alfred Agrell wrote:
That's a strange way to spell short. Eh, it'll be short eventually... but indeed, not yet.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/8984#note_116057
participants (3)
-
Alfred Agrell (＠Alcaro) -
Elizabeth Figura
-
Rémi Bernon