Date: Wed, 22 Mar 2023 18:46:31 +0000 From: bugzilla-noreply@freebsd.org To: bugs@FreeBSD.org Subject: [Bug 266101] ucred reference count may overflow Message-ID: <bug-266101-227-GNDRgYpEyx@https.bugs.freebsd.org/bugzilla/> In-Reply-To: <bug-266101-227@https.bugs.freebsd.org/bugzilla/> References: <bug-266101-227@https.bugs.freebsd.org/bugzilla/>
next in thread | previous in thread | raw e-mail | index | archive | help
https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=3D266101 --- 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 hand= y. 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 possi= ble. 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 =3D 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 proce= ss settles on final state which is then used for a *long time*. Trivial exampl= e: 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 crea= ting 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. --=20 You are receiving this mail because: You are the assignee for the bug.=
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?bug-266101-227-GNDRgYpEyx>