Date: Wed, 22 Mar 2023 18:46:31 UTC
https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=266101 --- Comment #1 from Mateusz Guzik <mjg@FreeBSD.org> --- the refcount API is just a poor man's mitigation, especially for the cred stuff. Perf wise, I have to note the counter gets modified *a lot* all while the struct keeps being accessed. Atomic op on a centralized counter is tons of memory traffic which does not need to happen, but I don't have numbers handy. Here are some tidbits: 1. One massive limitation of refcount API, which can't be helped, is that *underflows* cannot possibly be detected, thus use-after-free remains possible. 2. Even if there were no conditions triggering premature freeing, buggy code can still have a stale pointer to creds which gets used later. 3. There is nothing being done to prevent overflows from non-cred objects messing with said creds. instead, memory is allocated with malloc: cr = malloc(sizeof(*cr), M_CRED, M_WAITOK | M_ZERO); If one is serious about hardening creds, most of cred management has to be reworked. Typical case is that new creds get very temporarily allocated, get modified in a small manner and assigned. This happens a bunch of times before the process settles on final state which is then used for a *long time*. Trivial example: logging in and setgid, setuid etc. calls Apart from the churn, this also happens to increase memory usage -- you log in twice, you end up with 2 sets of identical creds which persist. Thus the general idea would be to maintain a global hash of creds, where you locally create what creds would look like you and try to find them in said struct. If that succeds, alloc/free trip got avoided, therwise you alloc to add them. I can't stress enough that creds are unmodifiable after creation and *never* freed. With the proposed scheme ref counting can be literally eliminated(!). The problem to solve here is making sure a nasty userspace cannot keep creating creds, without breaking anything. There is probably some bad idioms in place which result in more allocations than needed. It may be a hybrid approach will be prudent -- you *do* refcount(9) for all the transient stuff and "stabilize" after enough traffic. There are some gaps to fill in the proposal, but the general idea should be clear. I can't stress enough how the refcount API fails to address anything most of things of importance and that addressing them renders it mostly moot. -- You are receiving this mail because: You are the assignee for the bug.