[PATCH 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, cmd.exe gets Unix pid 3 in its namespace, and sends it to wineserver. When the second cmd 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. 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. -- 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 | 32 ++++++++++++++++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/server/process.c b/server/process.c index 52abaa21d75..3fd059a022f 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(struct process *process) +{ + /* pid namespaces aren't a thing outside Linux */ + return 1; +} +#endif + /* create a new process */ DECL_HANDLER(new_process) { @@ -1188,6 +1212,14 @@ DECL_HANDLER(new_process) close( socket_fd ); return; } + if (!socket_peer_has_pid( socket_fd )) + { + set_error( SCHED_E_NAMESPACE ); + /* 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
participants (2)
-
Alfred Agrell -
Alfred Agrell (＠Alcaro)