On Tue, Oct 01, 2024 at 08:42:56PM +0200, Aleksa Sarai wrote: > On 2024-10-01, Tycho Andersen <tycho@tycho.pizza> wrote: > > 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. > > > > Change exec path to fix up /proc/pid/comm in the case where we have > > allocated one of these synthetic paths in bprm_init(). This way the actual > > exec machinery is unchanged, but cosmetically the comm looks reasonable to > > admins investigating things. > > While I still think the argv[0] solution was semantically nicer, it > seems this is enough to fix the systemd problem for most cases and so we > can revisit the argv[0] discussion in another 10 years. :D Hi Tycho and everyone else, First, thank you so much for picking this up! Second, sorry for being late with a reply… Third, I tested the kernel with the patch and with systemd with the fexecve option enabled, and it all works as expected. Unfortunately, I don't think that the approach with f_path.dentry->d_name.name can be used :( As discussed previously, there are various places where symlinks are used. Alternatives and busybox were raised. Some additional examples: systemd (e.g. /usr/lib/systemd/systemd-udevd symlinks to /usr/bin/udevadm), yum-builddep, yum-config-manager, yumdownloader, yum-groups-manager all symlink to a multicall dnf-utils binary, and so on. The question is whether "pgrep" and similar tools not getting the expected name is a problem, and I think that the answer is, unfortunately, "yes". Users will notice this. As a distro maintainer, I would be _very_ wary of flipping this on in systemd, because there certainly are scripts and other tools that use that logic to check if things are running on the system. systemd uses cgroups and doesn't care about COMM at all, and I expect that many other modern tools won't either, but we have to take into account the long tail of local admin scripts and older tools. To avoid regressions and complaints, we really want an API that replaces the current execve invocations with an fd-based approach but doesn't change how things otherwise look. Arguably, the current patch would work great for 99% of cases, but that's not enough. (In particular, with rust being used more often for low level tools, and the binaries being large because of the static linking, I expect multicall binaries to become even more common. E.g. uutils-coreutils that might become the default coreutils implementation in a few years. It uses a multi-call binary and symlinks. Having 'coreutils' instead of 'sleep' as COMM is not good.) Please consider going back to the approach with argv[0]. Zbyszek > > v2: * drop the flag, everyone :) > > * change the rendered value to f_path.dentry->d_name.name instead of > > argv[0], Eric > > + __set_task_comm(me, bprm->file->f_path.dentry->d_name.name, true);
