LWN: Comments on "The kernel's command-line commotion"

Can we record comm in fdtable at open()?

consend — Wed, 18 Dec 2024 12:22:21 +0000

Thank you for your reply! I realized that putting it in the fdtable was indeed a wrong idea. Intuitively, the same file instance should share the same name, and placing it in the fdtable is semantically unclear and violates the lightweight design of descriptors. Just like you said, it's a property of open file, not of a descriptor. Additionally, you mentioned saving filenames for all files. I have simply thought about adding a flag to control whether to save through the open function, but indeed, adding a flag for an extremely small number of special cases may not be a good choice.

Can we record comm in fdtable at open()?

viro — Sun, 15 Dec 2024 16:48:34 +0000

First of all, fdtable is obviously wrong place for anything of that sort - you'd have to copy that on dup(), fork(), etc.; if anything, it's a property of open file, not of a descriptor. Putting the last component of pathname that had been used to open a file into resulting struct file... Theoretically doable, but that would cost quite a bit, with no clear benefit - you'd have to do that to all files, just for the sake of vanishingly small subset that would be involved in fexecve(), and usefulness of fexecve() itself is not obvious.

Can we record comm in fdtable at open()?

consend — Sun, 15 Dec 2024 07:35:04 +0000

I understand that the only accurate information fexecve() can use is the file descriptor (fd). So how about selectively storing comm in the fdtable during open? This would allow fexecve() to read it. However, this might bring some issues, such as increasing the burden of open(), requiring more memory, etc., or there might be some obvious problems that I haven't thought of?

The proposed patch was agreed during session at LPC 2024

Wol — Fri, 13 Dec 2024 21:18:08 +0000

Like it or not, the BDFL model is very successful.

As is "he who pays the piper calls the tune" or, in FLOSS terms, "he who puts in the work makes the rules".

I'm not saying all of them are, but many calls for "democracy" in FLOSS projects are "we want to control the committee that tells you where to direct your efforts". That usually is a dead flop as far as volunteers are concerned. And as we've seen - with Firefox amongst others - all too often a foundation intended to support a project has great difficulty paying developers, for whatever reason ...

Cheers,
Wol

Sad outcome

roblucid — Fri, 13 Dec 2024 12:40:53 +0000

The difference is the cost is only paid by those debugging, not expose every user unwittingly to potential hostile shenanigans by relying on mutable user space variables. The people changing to use fexecve(2) for the security benefits should perhaps add a logging option saying what they're calling and what the child pid was.

I developed many years, then later ran a lot of server machines distributed over many sites including network centersw and kernel level smoke & mirrors undermines the whole point of switching to the fd based call. Developers have a tendency to pick the easy option and if you're worried about exploits to race conditions, giving them shell access gets your hosts remotely cracked.

It doesn't make sense to worry about multicall binaries.

roblucid — Fri, 13 Dec 2024 12:17:47 +0000

Hmmmm, less useful unless you're interested in the truth of it ..
execve(2) behaviour was not changing, in the fexecve(2) case if you're not willing to pay some cost as you are wanting to see a file with a verified signature why are you bothering with the file descriptor? If say you have written a shell with fexecve(2) support as a feature, surely you can set up an environment variable and do more smoke & mirrors processing on ps(1)/top(1) via builtin to protect users from their illusions being shattered.
Scripts have trace features to help debugging, couldn't you just turn off the use of fexecve when developing if necessary?
As somebody said allowing obfuscation of what you are really running seems to be to the benefit the "shenanigans" use case.

It doesn't make sense to worry about multicall binaries.

maxfragg — Wed, 04 Dec 2024 09:57:53 +0000

all true, but the output of ps and co suddenly becomes a lot less useful, when half of you system shows up as toybox/busybox instead of sh, sleep, cat, ....

Sad outcome

intelfx — Tue, 03 Dec 2024 15:29:21 +0000

> 1. Some people want/need to be able to prevent certain kinds of shenanigans, which can only be done by using fexecve
> 2. Other people who aren't as concerned about those shenanigans want the utility of /proc/fd/comm
> 3. Developers writing launchers such as systemd don't want to have to write separate code paths to satisfy both groups.

I'd have rather said that everyone wants the utility of /proc/fd/comm. However, while people that specifically have a goal of preventing shenanigans (i.e. those operating secure environments), are probably willing to pay the cost of reduced convenience for security, the people in charge of systemd have a goal of "security by default". And security by default only works if it does not inflict misery elsewhere.

Sad outcome

stevie-oh — Tue, 03 Dec 2024 15:14:11 +0000

> If the whole point of the function is to disallow symlink shenanigans

Note that symlinks aren't necessary to this. The operative word here is _shenanigans_: fexecve is designed to prevent this sort of scenario:

1. Guard launcher opens path "/bin/foo"
2. Guard launcher proceeds to read the file contents and verifies the checksum. (It probably also verifies the checksum on all SOs that are /usr/bin/foo are linked to)
3. While #2 is happening, rogue user with sufficient access deletes "/bin/foo" and replaces it with a modified version.
4. Guard launcher finishes verifying checksum on /bin/foo and the checksum passes, so it execve's "/bin/foo". Since that path now refers to a different file/inode, the guard launcher executes the wrong program. Whoops!

By using fexecve instead of execve in step 4, the guard launcher can guarantee that the executable it launches is the _exact same file that it originally opened_.

I see three primary goals here, which currently don't work well together:

1. Some people want/need to be able to prevent certain kinds of shenanigans, which can only be done by using fexecve
2. Other people who aren't as concerned about those shenanigans want the utility of /proc/fd/comm
3. Developers writing launchers such as systemd don't want to have to write separate code paths to satisfy both groups.

Systemd has bug

mezcalero — Mon, 02 Dec 2024 09:25:47 +0000

You are mixing up things: we are not making use of the executable fd so far much, because we don't actually use execveat() unless you set ENABLE_FEXECVE macro, which nobody does. The code to use this was added a while back, in hopeful preparation that some day we could use execveat() properly, but that future never came, so nothing else was moved over to using only the executable fd, because that would be dead code.

It's like arguing: we don't need washing machines, because everyone washes their clothes by hand. Of course they do, if they have no washing machine!

In systemd we are moving the codebase bit by bit over to reference things by fds rather than by paths, i.e. for new stuff we generally only use O_PATH, openat() and friends. For old code we port things over, but we'll never be able to do that properly for execveat(), since it's so unusable right now.

I am not going to comment on the SMACK stuff, it's contributed code by SMACK folks, I have no comprehensive understanding of that.

Lennart

That will be very confusing when using extreme multicall binaries like busybox and gzip, combined with debian style alternatives!

cplaplante — Sun, 01 Dec 2024 14:19:54 +0000

It's a shame we can't enhance `ps` to report the symlinks, like `ls`. E.g.:

$ ps
ls -> /bin/busybox
tar -> /bin/busybox

etc.

Sad outcome

geuder — Sat, 30 Nov 2024 21:08:19 +0000

What would systemd have offered us if they could have switched to fexecve()?

That will be very confusing when using extreme multicall binaries like busybox and gzip, combined with debian style alternatives!

koenkooi — Sat, 30 Nov 2024 08:24:01 +0000

$ ps
busybox
busybox
busybox
gzip
busybox

This seems to be one of the rare cases where you can, in good faith, ask "which truth"? The user types in ls, tar, wget, zgrep but under the hood it's either busybox or gzip.

Systemd has bug

ebiederm — Fri, 29 Nov 2024 20:01:33 +0000

I just read through the systemd code to see why people desire to use fexecve in the systemd code

Once the file descriptor for the binary is open systemd makes some sanity checks, that are redundant with the checks execve makes in the kernel.

Other then those sanity there is the only user of executable_fd in systemd, setup_smack.

As I read setup_smack, it reads the xattr that holds the label, smack will apply during exec. If the xattr is present systemd applies the label before smack does. Which is silly, but fine except the systemd code skips the checks the smack kernel code makes before applying the label.

Once smack_setup is fixed to not do unnecessary and buggy work there is no reason for systemd to open the file before exec, and thus no reason to call fexecve.

So all of that work can be removed from systemd and the code can become faster and more reliable. As well as making the entire issue of symlinks to binaries a nonissue, because fexecve is unnecessary.

Sad outcome

NYKevin — Fri, 29 Nov 2024 15:46:50 +0000

I would tend to assume that there is no existing userspace reliance on this behavior (it would be quite insane to use execveat to test whether a path is a symlink, when fstat is right there). Perhaps there are security issues, but OTOH execveat already supports AT_SYMLINK_NOFOLLOW for non-null pathname, and it would (presumably) be trivial to extend that to the AT_EMPTY_PATH case.

Unfortunately, this whole discussion is probably moot for fexecve(3), considering this passage in the man page:

> The idea behind fexecve() is to allow the caller to verify (checksum) the contents of an executable before executing it. Simply opening the file, checksumming the contents, and then doing an execve(2) would not suffice, since, between the two steps, the filename, or a directory prefix of the pathname, could have been exchanged (by, for example, modifying the target of a symbolic link).

If the whole point of the function is to disallow symlink shenanigans, then it is obviously a non-starter to deliberately reintroduce those semantics, so libc would presumably just start passing AT_SYMLINK_NOFOLLOW (if it does not already), and we would be right back where we started.

Simple solution

vbabka — Fri, 29 Nov 2024 14:04:45 +0000

Maybe even the pathname argument could be repurposed to become comm with AT_EMPTY_PATH (plus/or another new flag to control this new behavior), because normally it's an empty string with AT_EMPTY_PATH? That would avoid the need for prctl().

Sad outcome

dskoll — Thu, 28 Nov 2024 22:24:49 +0000

Ah, ok, missed that... sorry.

Simple solution

rweikusat2 — Thu, 28 Nov 2024 20:22:04 +0000

That's basically the same idea as copying *argv, just a bit more complicated. /proc/self/comm is one of two things:

1. The (first 16 characters of the) file which was actually executed by the kernel.
2. A string the program which was executed passed as argument to PR_SET_NAME.

This means that it's not under control of the code which executed the exec system call. In contrast to this, *argv is the first string of the argument vector. By convention, this is also the filename of the executed file but that's really just a convention. It can be any string the executing process desired to use as first argument and it may even not exist at all, ie *argv may be NULL.

Copying *argv (or, for that matter, any other string the executing process can either chose freely or omit at all) thus doesn't solve the problem that, for programs executed via file descriptor, the correct comm value is useless for determining information about the actually running program.

There's no correct solution for setting comm to the value it would have had had execve with a filename been used instead of execveat/ fexecve because the name which was used to open the file descriptor may no longer refer to the same file by the time it's executed. Using the name from the dentry is probably the best approximation as that's at least a name referring to the file which is being executed.

Sad outcome

intelfx — Thu, 28 Nov 2024 20:15:36 +0000

> The first argument of execveat needs to be a directory file descriptor. Unless /bin/false is a directory on your system, this shouldn't work even without O_NOFOLLOW.

AT_EMPTY_PATH was used, though.

Sad outcome

dskoll — Thu, 28 Nov 2024 19:54:27 +0000

The first argument of execveat needs to be a directory file descriptor. Unless /bin/false is a directory on your system, this shouldn't work even without O_NOFOLLOW.

As to why you get an ELOOP error return, I guess that's just a strange detail of the implementation. I would have thought ENOTDIR would be the appropriate error return.

Sad outcome

adobriyan — Thu, 28 Nov 2024 18:50:06 +0000

Apparently you can't start lookup from a symlink.

readlink("symlink", "/bin/false", 4096) = 10
openat(AT_FDCWD, "symlink", O_RDONLY|O_NOFOLLOW|O_PATH) = 3
execveat(3, "", NULL, NULL, AT_EMPTY_PATH) = -1 ELOOP

Sad outcome

NYKevin — Thu, 28 Nov 2024 16:54:51 +0000

But if we pass O_NOFOLLOW, then we should end up with a path fd that refers to the symlink, not an fd that refers to the executable. Given that (as you say) the fd pins the dentry of the requested file, and the requested file is a symlink and not its target, I don't understand how the kernel would avoid pinning the symlink dentry in that case.

Is the problem that execveat fails to dereference the symlink afterwards?

Sad outcome

adobriyan — Thu, 28 Nov 2024 16:40:01 +0000

See terminate_walk() in path_lookupat().

Descriptor pins "struct file" which pins dentry which pins inode.
You can walk upwards to the root and get _some_ name, that's what readlink(/proc/*/fd/*) does.

Now _some_ history must kept for loop detection purposes and too-deep-recursion detection but it surely won't exist once system call exits.

In theory, the name of the first symlink which started last pathname resolution chain could be kept to use as argv[0] but I don't want to be the one sending such patch. :-)

What if there is no dentry?

epa — Thu, 28 Nov 2024 14:54:28 +0000

What happens if you open an executable file, unlink it from the directory, and then run it with fexecve()?
Or if the file has two links?

Sad outcome

NYKevin — Thu, 28 Nov 2024 14:42:16 +0000

When exactly does it get tossed? Suppose we construct a file descriptor with open(..., O_NOFOLLOW|O_PATH) (or openat or what have you). Then it probably can't get tossed until after we're in execveat, at which point the kernel should be able to preserve it if it so desires.

Neither the dentry nor the argv[0] seems a good solution

THALES — Thu, 28 Nov 2024 12:26:04 +0000

There is a reason the user cannot access the comm when a process has been executed with fexecve(). That is because the kernel has no guarantees that the file being executed is the same as the one on the disk. A process could open an executable file, tamper with it, then execute it. The real safe behavior is the existing one, neither the dentry nor the argv[0] seems a good solution.

execv is weird for not resolving symlinks for comm

jengelh — Thu, 28 Nov 2024 11:34:59 +0000

Given { symlink("/bin/sleep", "slp"); execl("./slp", "sla", "3600", NULL); } or e.g. `less -S slp`, and ignoring base filesystem symlinks like /bin -> /usr/bin under some distros, tell me who the oddball is without telling me who the oddball is:

* Sun lsof: /bin/sleep
* Sun ps -o args: sla 3600
* Sun ps -o comm: sla
* FreeBSD lsof: /bin/sleep
* FreeBSD ps -o args: sla 3600
* FreeBSD ps -o comm: sleep
* Linux lsof (/proc/N/fd/,/proc/N/maps): /bin/sleep
* Linux ps -o args: sla 3600
* Linux ps -o comm: slp

Simple solution

lkundrak — Thu, 28 Nov 2024 11:27:39 +0000

Yes. Or even use the existing call, with a flag not to touch the comm altogether. That way the calling process could just: fork(); prctl(PR_SET_NAME, "lalala"); execveat(..., AT_KEEP_PR_NAME); and be done with it.

Add a execveat flag?

mezcalero — Thu, 28 Nov 2024 08:35:27 +0000

That would make sense to me.

Sad outcome

mezcalero — Thu, 28 Nov 2024 08:32:52 +0000

Sure I can also copy the binary and rename it, and then glue a floppy disk to my forehead. It's a bit ridiculous to suggest this was a suitable method of operation for *all* service binary invocations while at the same time saying that initializing comm[] from argv[0] was not an option because doing such a memcpy() would be "too slow".

I mean, come on.

(Yes, I know it wasn't you who who said copying argv[0] → comm[] was too slow, that was Linus.)

Sad outcome

mezcalero — Thu, 28 Nov 2024 08:28:34 +0000

It's not really up to us to decide what people use. People use what people use. Some people use hardlinks, some people use symlinks, some people look at comm[], others at argv[0] to implement multi-call binaries.

As a systemd maintainer I certainly can give people guidelines, and I can maybe be strict on not supporting completley broken behaviour, but frankly in this case, we don't have that luxury: I don't think using hardlinks or softlinks or comm[] or argv[0] could constitute "clearly broken behaviour", I think all of it is fine in a world where execve() is the law of the land. The whole mess just starts because fexecve() became a thing and it's such a incomplete (to not use the word "broken") interface. And I seriously doubt the right approach is to tell a myriad of projects and distributions to rearrange their stuff to make fexecve() workable, but instead maybe it is to just fix that broken interface.

Sad outcome

mezcalero — Thu, 28 Nov 2024 08:21:36 +0000

One of the reasons I dislike /proc/self/attr/exec that it maintains process/state-wide state: if somebody uses this they first have to set up the label, and then do the execve() in a 2nd step. If for some reason the abort the whole thing, and they don't end up doing an execve() (or the execve() fails, maybe due to ENOENT) they have to undo the label preparation, but that takes care to do right.

I really hate logic like that that establishes some hidden state, for a secondary operation, that if for some reason fails or is not executed for some reason needs to be manually rolled back. A much better interface would be if this data would be passed to the actual execve() so that it's very clear what this is intended for and that it has no other lifecycle, cannot be applied to the wrong execve(), isn't sticky and so on.

Hence lsm_set_self_attr() might be slightly better as it doesn't require procfs anymore, but the fundamental ugliness doesn't really go away, im my PoV.

Lennart

Simple solution

rrolls — Thu, 28 Nov 2024 08:04:41 +0000

Add a new version of fexecve/execveat which takes an arbitrary string to be placed on `comm` in addition to the file descriptor.

Programs wishing to use this instead of execve, when the original path is a symlink, can get the basename of the original path themselves, do whatever opening and checking they like of the contents of the file, then pass that basename to be stored in `comm`.

Everyone wins.

Sad outcome

neilbrown — Thu, 28 Nov 2024 04:15:23 +0000

I agree that an fexecve2() that takes a "comm" argument would be a good thing. But it isn't necessary.

Just find some private directory, create a symlink from my_comm to /proc/self/fd/NN, make sure NN is CLOSE-ON-EXEC,
and
execveat(private-dir-fd, "my_comm", argv, envp, 0);

As close-on-exec is processed after the target file is opened, this gives you all you need.

Having to find a private directory isn't ideal, but shouldn't be too hard. /run/fexec/$UID/$PID/ ??
Cleaning up might be awkward.

The security concern

npws — Thu, 28 Nov 2024 02:22:21 +0000

The entire discussion is a complete train wreck. Linus keeps ranting on and on, making incorrect claims left and right, while shouting and insulting people. Apparently neither CoC nor userspace matter when he doesn't like it.

The security concern

roc — Wed, 27 Nov 2024 23:20:32 +0000

The attacker can create a hard link to get the same effect. Or they can use ptrace to inject a prctl call after exec.

There are some situations where a restricted attacker could manipulate argv[0] but not comm. But they're very narrow. Just ranting that "comm is THE TRUTH" is totally misleading.

The security concern

kees — Wed, 27 Nov 2024 22:44:00 +0000

> He ought to have apologised for that

Yes, he should have. Especially after directly insulting all the other participants in the discussion. I thought we were supposed to have a sane CoC?

Add a execveat flag?

aszs — Wed, 27 Nov 2024 22:15:16 +0000

Why not just add a flag to execveat so that it uses pathname to set comm (and treats dirfd as the executable's fd)?

Sad outcome

fman — Wed, 27 Nov 2024 21:43:20 +0000

Uh oh. That essentially means, when using fexecve() that is, everything running on an embedded system would essentially be "busybox" :-O

The security concern

carlosrodfern — Wed, 27 Nov 2024 21:40:07 +0000

He may not be referring to programs that change their name programmatically, but attackers reusing existing binaries and hiding their attack with links.

For example,

$ ln -s /usr/bin/sleep ./bash
./bash 60

$ ps -e -o comm,cmd | grep bash
bash ./bash 60