Page MenuHomeFreeBSD

procctl(2) addressed by process descriptor (P_PROCDESC)
Needs ReviewPublic

Authored by inquire_JohnEricson.me on Thu, Aug 6, 1:50 PM.
Tags
None
Referenced Files
F168070631: D58694.diff
Wed, Aug 26, 6:02 AM
Unknown Object (File)
Fri, Aug 21, 10:16 PM
Unknown Object (File)
Thu, Aug 20, 11:31 PM
Unknown Object (File)
Wed, Aug 19, 12:25 PM
Unknown Object (File)
Tue, Aug 18, 8:41 PM
Unknown Object (File)
Tue, Aug 18, 8:36 PM
Unknown Object (File)
Tue, Aug 18, 4:24 PM
Unknown Object (File)
Tue, Aug 18, 4:10 PM
Subscribers

Details

Summary

Adds a P_PROCDESC idtype naming the target by a process descriptor the
caller holds rather than by a pid, so process control stays available in
capability mode -- procctl(2) becomes CAPENABLED, and in capability
mode P_PROCDESC is the only identifier accepted. Guarded by a new
CAP_PROCCTL right, which is not granted on the output of pdfork(2) by
default; a caller opts in with the PD_PROCCTL_CAP flag.

This follows the direction of D58586, which reaches ptrace(2) operations
through a process descriptor rather than adding parallel syscalls. It
likewise made CAP_PTRACE not granted by default, too. Using the
existing multiplexer keeps the ABI addition to one idtype instead of one
syscall per operation, and it happens to be the natural fit for an
embryonic process: pdnew(2) yields a descriptor and no pid the caller
should have to use.

posix_spawn(3) uses it for POSIX_SPAWN_DISABLE_ASLR_NP, which is
already a procctl(PROC_ASLR_CTL) and so needs no new command, removing
that attribute from the set that still falls back to vfork(2).

Even for P_PROCDESC, not every command belongs in capability mode: the
reaping and group operations reach beyond the process the descriptor
names, which is more authority than holding that one descriptor should
grant. Each command therefore carries a cap_safe bit; in capability
mode only the ones marked so may be issued, and the machine-dependent
commands are denied there entirely for now. Everything else fails with
ECAPMODE. Erring on the side of prohibiting, only PROC_ASLR_CTL ---
what posix_spawn(3) needs --- is marked so far; the list can grow as
commands are audited.

A tiny kern_procctl_permit() is also factored out to make sure that
this one liner --- which now would have occured in three, not two
places, otherwise --- doesn't drift out of sync.

Depends on D58586

Assisted-by: Claude Code (Claude Opus 5)

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Skipped
Unit
Tests Skipped
Build Status
Buildable 75486
Build 72369: arc lint + arc unit

Event Timeline

The change modifies the current semantic of procdesc privileges. Any existing code that pdforks and then passes the pfd around implicitly grants the procctl() privileges to the pfd recepient. This is why I added the PD_PTRACE_CAP and made the capability not enabled by default.

sys/kern/kern_procctl.c
1266

Can we avoid adding one more wrapper, instead doing the check in the kern_procctl_single()?

1336

This comment might be useful for your understanding of the code, but really it is not needed. Also it does not belong there, but to fget_procdesc(), where it would repeat the herald comment, much more verbosely.

Also, move the fp declaration to the top, and remove the {} block around the case code.

1340

This variable is mostly useless, use the check for lock_tree directly.

sys/sys/wait.h
127

This is too much text there. A short description in the same line as done for other P_ symbols is enough. Perhaps note that the scope is only effective for procctl(2).

sys/kern/kern_procctl.c
1336

Ah, I do love narrower scopes like that, showing that the open file (and associated cost) is only needed in the one case, but it's OK. Will have a new draft with everything fixed in a moment.

inquire_JohnEricson.me edited the summary of this revision. (Show Details)

Edit the patch as @kib requested

The one thing I did differently was that I didn't put the cansee/candebug
check in kern_procctl_single, but instead kept the code closer to how it was
before, and merely deduped the ternary expression. Otherwise, this check will
occur *twice* in the P_PGID case (once to skip the process, and again,
redudantly afterwords), and that didn't feel clean to me.

I hope this is basically still in the same spirt; I agree the
kern_procctl_found function wasn't really paying for itself, and while this
function is shorter, it has 3 callers not 2.

I have a global question about this change. Do we really want or need to enable all procctl(2) ops in capability mode?
E.g. all reaping control ops sounds too much. For instance, the REAP_KILL gives the permission to kill a process for which we do not necessarily posses a process descriptor.

lib/libsys/pdfork.2
109

I do not think that this item should be in your patch.

sys/kern/kern_procctl.c
38

sys/procdesc.h should be placed at alphabetically ordered line.

1279–1287

I am not sure that this comment is needed, it repeats the condition. But if leaving it in, it should be like

P_PID and P_PROCDESC name exactly one process
1336

If you guard this and the next if() with one more if(error == 0), the procdesc_out label can be eliminated.

I have a global question about this change. Do we really want or need to enable all procctl(2) ops in capability mode?

That's a good question, and one I forgot to consider. I agree the answer is probably "no". At a minimum, I think it's prudent to adopt an "guilty until proven innocent" mindset and restrict each one until it's been audited a bit.

inquire_JohnEricson.me edited the summary of this revision. (Show Details)

Address @kib's line comments. Also take a stab at blacklisting most
individual commands, and what future work on that would look like.