[PATCH v3 0/2] MR9553: ntdll: Refuse to start if wineserver is in wrong pid namespace
To reproduce: - `unshare -Upf ./wine cmd` - (in another terminal) `unshare -Upf ./wine not-a-command` The first terminal will now go completely unresponsive, until you kill -9 the relevant start.exe from the second terminal. Root cause analysis: In the first Wine, wineserver gets Unix pid 3 in its namespace. (And a different pid in the root namespace, but no relevant process sees that.) In the second Wine, not-a-command gets Unix pid 3 in its namespace, and sends it to wineserver. When not-a-command notices it doesn't exist and exits, wineserver checks if pid 3 did indeed exit, and SIGKILLs it if not. But since that pid is from wrong namespace, wineserver ends up killing itself instead. While this only happens in badly configured sandboxes, such things do exist in the wild. https://github.com/flathub/org.winehq.Wine/issues/41 The real solution would be either fixing the sandbox config, or making Wine use pidfds instead of pids, but former is out of our control, and latter would be a lot of effort and ifdefs (and unlikely to work, there's no process_vm_readv for pidfds). The second commit only blocks cases where wineserver exists in ntdll's namespace, but not the other way round. It's a much rarer case than having the processes in mutually-inaccessible sibling namespaces, it's a much bigger patch than the ntdll side, and the error is detected at wrong side (meaning it can't print a friendly error). I'm not sure if it's worth keeping. -- v3: server: Reject clients from wrong pid namespace https://gitlab.winehq.org/wine/wine/-/merge_requests/9553
From: Alfred Agrell <floating(a)muncher.se> --- dlls/ntdll/unix/server.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/dlls/ntdll/unix/server.c b/dlls/ntdll/unix/server.c index 258a959de72..1fb02aa4125 100644 --- a/dlls/ntdll/unix/server.c +++ b/dlls/ntdll/unix/server.c @@ -1655,6 +1655,10 @@ size_t server_init_process(void) /* work around Ubuntu's ptrace breakage */ if (server_pid != -1) prctl( 0x59616d61 /* PR_SET_PTRACER */, server_pid ); #endif + /* ensure wineserver exists in our pid namespace */ + if (server_pid == 0) + fatal_error( "cannot determine wineserver's process ID.\n" + "Is it running in wrong pid namespace?\n" ); /* ignore SIGPIPE so that we get an EPIPE error instead */ sig_act.sa_handler = SIG_IGN; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/9553
From: Alfred Agrell <floating(a)muncher.se> --- server/process.c | 33 +++++++++++++++++++++++++++++++++ 1 file changed, 33 insertions(+) diff --git a/server/process.c b/server/process.c index 52abaa21d75..09fa20885b7 100644 --- a/server/process.c +++ b/server/process.c @@ -1146,6 +1146,30 @@ int set_process_debug_flag( struct process *process, int flag ) return write_process_memory( process, process->peb + 2, 1, &data, NULL ); } +#ifdef __linux__ +int socket_peer_has_pid(int socket_fd) +{ + /* ensure the caller exists in our pid namespace */ + struct ucred ucred; + socklen_t len = sizeof(struct ucred); + if (getsockopt(socket_fd, SOL_SOCKET, SO_PEERCRED, &ucred, &len) < 0) + return 0; + if (ucred.uid != getuid()) + return 0; + /* this is the pid of whoever created the socket, i.e. the parent of the newly created process */ + /* we still trust the unix_pid in init_first_thread */ + if (ucred.pid == 0) + return 0; + return 1; +} +#else +int socket_peer_has_pid(int socket_fd) +{ + /* pid namespaces aren't a thing outside Linux */ + return 1; +} +#endif + /* create a new process */ DECL_HANDLER(new_process) { @@ -1188,6 +1212,15 @@ DECL_HANDLER(new_process) close( socket_fd ); return; } + if (!socket_peer_has_pid( socket_fd )) + { + fprintf( stderr, "wineserver: Rejecting client with unknown PID in peercred\n" ); + set_error( STATUS_UNSUCCESSFUL ); + /* caller gave us a bad pid, discard it, don't kill the named pid */ + current->process->unix_pid = -1; + close( socket_fd ); + return; + } if (req->parent_process) { -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/9553
On Sat Nov 22 14:36:59 2025 +0000, Alfred Agrell wrote:
changed this line in [version 3 of the diff](/wine/wine/-/merge_requests/9553/diffs?diff_id=226776&start_sha=60abd3865bd2abcbb8118760bbb068f028837ff8#c9d2907d0f5a89f79a28a80568c303e7f0683af1_1217_1217) Good point. I just grepped the error list for anything containing namespace, didn't check what exactly it means.
The error number doesn't seem to show up anywhere, anyways. Good idea complaining to stderr, too - I couldn't find wineserver's ERR macro, and forgot looking for alternatives. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/9553#note_123356
This merge request was closed by Alexandre Julliard. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/9553
That sounds like a packaging issue, and should be fixed there. I don't think we want to add complexity only to fail differently. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/9553#note_123849
Yep, it very much is a packaging issue. We already have some code to check for certain packaging issues and print a clear error, in the winediag debug channel. But if you feel this one is different, then so be it. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/9553#note_123853
participants (3)
-
Alexandre Julliard (＠julliard) -
Alfred Agrell
-
Alfred Agrell (＠Alcaro)