On Tue, Sep 24, 2024 at 12:39:35PM -0500, Eric W. Biederman wrote: > Tycho Andersen <tycho@tycho.pizza> writes: > > > From: Tycho Andersen <tandersen@xxxxxxxxxxx> > > > > Zbigniew mentioned at Linux Plumber's that systemd is interested in > > switching to execveat() for service execution, but can't, because the > > contents of /proc/pid/comm are the file descriptor which was used, > > instead of the path to the binary. This makes the output of tools like > > top and ps useless, especially in a world where most fds are opened > > CLOEXEC so the number is truly meaningless. > > > > This patch adds an AT_ flag to fix up /proc/pid/comm to instead be the > > contents of argv[0], instead of the fdno. I tried this version (with a local modification to drop the flag and enable the new codepath if get_user_arg_ptr(argv, 0) returns nonnull as suggested later in the thread), and it seems to work as expected. In particular, 'pgrep' finds for the original name in case of symlinks. > All of that said I am not a fan of the implementation below as it has > the side effect of replacing /dev/fd/N with a filename that is not > usable by #! interpreters. So I suggest an implementation that affects > task->comm and not brpm->filename. Hmm, I don't understand this. /dev/fd/ would not generally contain an open fd for the original binary. It only would if the caller uses fexecve with an fd opened without O_CLOEXEC, but then it'd be something like /dev/fd/3 or /dev/fd/4 and the callee would be confused by having an extra fd, so except for some specialed cases, the caller should always use O_CLOEXEC. With this patch: $ sudo ln -sv /bin/sleep /usr/local/bin/sleep-link $ sudo systemd-run sleep-link 10000 $ sudo strace -f -e execve,execveat -p 1 ... [pid 1200] execve("/proc/self/fd/9", ["/usr/lib/systemd/systemd-executo"..., "--deserialize", "150", "--log-level", "info", "--log-target", "journal-or-kmsg"], 0x7ffe97b98178 /* 3 vars */) = 0 [pid 1200] execveat(4, "", ["/usr/local/bin/sleep-link", "10000"], 0xd8edf70 /* 9 vars */, AT_EMPTY_PATH) = 0 ^C $ pgrep sleep-link 1200 $ sudo ls -l /proc/1200/fd total 0 lr-x------ 1 root root 64 Oct 2 17:13 0 -> /dev/null lrwx------ 1 root root 64 Oct 2 17:13 1 -> 'socket:[8585]' lrwx------ 1 root root 64 Oct 2 17:13 2 -> 'socket:[8585]' $ head -n1 /proc/1200/{comm,status,stat} ==> /proc/1200/comm <== sleep-link ==> /proc/1200/status <== Name: sleep-link ==> /proc/1200/stat <== 1200 (sleep-link) ... This all looks good. Zbyszek