Page MenuHomeFreeBSD

cred: proc_set_cred(), proc_unset_cred(): Update user's process count
ClosedPublic

Authored by olce on Oct 4 2024, 8:12 AM.
Tags
None
Referenced Files
Unknown Object (File)
Mon, Feb 9, 4:31 AM
Unknown Object (File)
Sun, Feb 8, 4:43 PM
Unknown Object (File)
Fri, Feb 6, 4:59 PM
Unknown Object (File)
Sun, Feb 1, 8:04 AM
Unknown Object (File)
Sat, Jan 31, 3:29 PM
Unknown Object (File)
Thu, Jan 29, 8:06 PM
Unknown Object (File)
Thu, Jan 29, 5:17 AM
Unknown Object (File)
Thu, Jan 29, 5:17 AM

Details

Summary

As a process really changes credentials at the moment proc_set_cred() or
proc_unset_cred() is called, these functions are the proper locations to
perform the update of the new and old real users' process count (using
chgproccnt()).

Before this change, change_ruid() instead would perform that update,
although it operates only on a passed credential which is a priori not
tied to the calling process (or not to any process at all). This was
arguably a flaw of commit b1fc0ec1a7a49ded, r77183, based on its commit
message, and in particular the portion "(...) In each case, the call now
acts on a credential not a process (...)".

Fixing this makes using change_ruid() more natural when building
candidate credentials that in the end are not applied to a process,
e.g., because of some intervening privilege check. Also, it removes
a hack around this unwanted process count change in unionfs.

We also introduce the new proc_set_cred_enforce_proc_lim() so that
callers can respect the per-user process limit, and will use it for the
upcoming setcred(). We plan to change all callers of proc_set_cred() to
call this new function instead at some point. In the meantime, both
proc_set_cred() and the new function will coexist.

As detailed in some proc_set_cred_enforce_proc_lim()'s comment, checking
against the process limit is currently flawed as the kernel doesn't
really maintain the number of processes per UID (besides RLIMIT_NPROC,
this in fact also applies to RLIMIT_KQUEUES, RLIMIT_NPTS, RLIMIT_SBSIZE
and RLIMIT_SWAP). The applied limit is currently that of the old real
UID. Root (or a process granted with PRIV_PROC_LIMIT) is not subject to
this limit.

Fixes: b1fc0ec1a7a49ded

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

olce requested review of this revision.Oct 4 2024, 8:12 AM
This revision was not accepted when it landed; it landed in state Needs Review.Dec 16 2024, 2:46 PM
This revision was automatically updated to reflect the committed changes.
bdrewery added inline comments.
sys/kern/kern_prot.c
2304

Why the new cr_ref == 1 assertion?

sys/kern/kern_prot.c
2304

(This revision is more than one year old.)

We were already asserting that cr_users is 0, as we forcibly set it to 1 below, meaning that we do not expect that counter to raise in the meantime. If there is more than one cr_ref, that means we have another thread having a reference, which concurrently could raise cr_users without our noticing (technically, that could happen also if cr_ref is also 1, if we hand some pointer to another thread and then wait for it to finish with it, but AFAIK we don't do that for credentials). At that time, all callers were passing new credentials with cr_ref set to 1, so I just added this safety net.

These constraints on cr_users and cr_ref were later lifted in D53636.