Changeset View
Standalone View
sys/kern/kern_prot.c
Show First 20 Lines • Show All 1,845 Lines • ▼ Show 20 Lines | |||||
* If users > 0 and curthread->td_realucred == cred, then updates are performed | * If users > 0 and curthread->td_realucred == cred, then updates are performed | ||||
* against td_ucredref. | * against td_ucredref. | ||||
* In other cases updates are performed against cr_ref. | * In other cases updates are performed against cr_ref. | ||||
* | * | ||||
* Changing td_realucred into something else decrements cr_users and transfers | * Changing td_realucred into something else decrements cr_users and transfers | ||||
* accumulated updates. | * accumulated updates. | ||||
*/ | */ | ||||
struct ucred * | struct ucred * | ||||
crcowget(struct ucred *cr) | crcowget(struct ucred *cr) | ||||
jrtc27: Please use `__SIZEOF_LONG__` or some other similar variable for these; CHERI architectures are… | |||||
zleiUnsubmitted Not Done Inline ActionsI see crcowget() is called during thread creating. Can we possibly reach 2^32 threads? That sounds horrible. Maybe user want be warned when tuning and kern.maxproc * kern.threads.max_threads_per_proc exceed 2^32 . zlei: I see `crcowget()` is called during thread creating. Can we possibly reach 2^32 threads? That… | |||||
{ | { | ||||
mtx_lock(&cr->cr_mtx); | mtx_lock(&cr->cr_mtx); | ||||
KASSERT(cr->cr_users > 0, ("%s: users %d not > 0 on cred %p", | KASSERT(cr->cr_users > 0, ("%s: users %d not > 0 on cred %p", | ||||
__func__, cr->cr_users, cr)); | __func__, cr->cr_users, cr)); | ||||
cr->cr_users++; | cr->cr_users++; | ||||
cr->cr_ref++; | cr->cr_ref++; | ||||
zleiUnsubmitted Not Done Inline Actions
+1 for that. So we might assert a overflow on increase. KASSERT(cr->cr_ref > 0, ("overflow")); And bench on a machine with huge amount of resources. zlei: > I don't understand why we can't simply check for over/underflow when manipulating the counter… | |||||
mjgAuthorUnsubmitted Done Inline ActionsThat's impossible to do here because the value is completely arbitrary as long as cr_users > 0 -- then the real can only be found by summing up all the counts from all threads. mjg: That's impossible to do here because the value is completely arbitrary as long as cr_users > 0… | |||||
zleiUnsubmitted Not Done Inline Actions
I don't quite understand the statement by summing up all the counts from all threads . As both cr->cr_users and cr->cr_ref is mutex protected, it is impossible that cr->cr_ref got touched by other threads (if they follows the order lock and write), so I think the overflow (of cr_ref) can be safely and stably caught. Am I misreading? That is indeed not enough as I see crunuse() and crunusebatch() also increase cr_ref. zlei: > That's impossible to do here because the value is completely arbitrary as long as cr_users >… | |||||
mjgAuthorUnsubmitted Done Inline Actionsi refer you to the comment starting with 'Credential management' in the file mjg: i refer you to the comment starting with 'Credential management' in the file | |||||
mtx_unlock(&cr->cr_mtx); | mtx_unlock(&cr->cr_mtx); | ||||
return (cr); | return (cr); | ||||
} | } | ||||
static struct ucred * | static struct ucred * | ||||
crunuse(struct thread *td) | crunuse(struct thread *td) | ||||
{ | { | ||||
struct ucred *cr, *crold; | struct ucred *cr, *crold; | ||||
MPASS(td->td_realucred == td->td_ucred); | MPASS(td->td_realucred == td->td_ucred); | ||||
cr = td->td_realucred; | cr = td->td_realucred; | ||||
mtx_lock(&cr->cr_mtx); | mtx_lock(&cr->cr_mtx); | ||||
cr->cr_ref += td->td_ucredref; | cr->cr_ref += td->td_ucredref; | ||||
zleiUnsubmitted Not Done Inline ActionsKASSERT(cr->cr_ref >= td->td_ucredref, ("overflow")); zlei: ```
KASSERT(cr->cr_ref >= td->td_ucredref, ("overflow"));
``` | |||||
td->td_ucredref = 0; | td->td_ucredref = 0; | ||||
KASSERT(cr->cr_users > 0, ("%s: users %d not > 0 on cred %p", | KASSERT(cr->cr_users > 0, ("%s: users %d not > 0 on cred %p", | ||||
__func__, cr->cr_users, cr)); | __func__, cr->cr_users, cr)); | ||||
cr->cr_users--; | cr->cr_users--; | ||||
if (cr->cr_users == 0) { | if (cr->cr_users == 0) { | ||||
KASSERT(cr->cr_ref > 0, ("%s: ref %d not > 0 on cred %p", | KASSERT(cr->cr_ref > 0, ("%s: ref %ld not > 0 on cred %p", | ||||
__func__, cr->cr_ref, cr)); | __func__, cr->cr_ref, cr)); | ||||
crold = cr; | crold = cr; | ||||
} else { | } else { | ||||
cr->cr_ref--; | cr->cr_ref--; | ||||
crold = NULL; | crold = NULL; | ||||
} | } | ||||
mtx_unlock(&cr->cr_mtx); | mtx_unlock(&cr->cr_mtx); | ||||
td->td_realucred = NULL; | td->td_realucred = NULL; | ||||
return (crold); | return (crold); | ||||
} | } | ||||
static void | static void | ||||
crunusebatch(struct ucred *cr, int users, int ref) | crunusebatch(struct ucred *cr, int users, int ref) | ||||
{ | { | ||||
KASSERT(users > 0, ("%s: passed users %d not > 0 ; cred %p", | KASSERT(users > 0, ("%s: passed users %d not > 0 ; cred %p", | ||||
__func__, users, cr)); | __func__, users, cr)); | ||||
mtx_lock(&cr->cr_mtx); | mtx_lock(&cr->cr_mtx); | ||||
KASSERT(cr->cr_users >= users, ("%s: users %d not > %d on cred %p", | KASSERT(cr->cr_users >= users, ("%s: users %d not > %d on cred %p", | ||||
__func__, cr->cr_users, users, cr)); | __func__, cr->cr_users, users, cr)); | ||||
cr->cr_users -= users; | cr->cr_users -= users; | ||||
cr->cr_ref += ref; | cr->cr_ref += ref; | ||||
zleiUnsubmitted Not Done Inline ActionsKASSERT(cr->cr_ref >= ref, ("overflow")); zlei: ```
KASSERT(cr->cr_ref >= ref, ("overflow"));
``` | |||||
cr->cr_ref -= users; | cr->cr_ref -= users; | ||||
if (cr->cr_users > 0) { | if (cr->cr_users > 0) { | ||||
mtx_unlock(&cr->cr_mtx); | mtx_unlock(&cr->cr_mtx); | ||||
return; | return; | ||||
} | } | ||||
KASSERT(cr->cr_ref >= 0, ("%s: ref %d not >= 0 on cred %p", | KASSERT(cr->cr_ref >= 0, ("%s: ref %ld not >= 0 on cred %p", | ||||
__func__, cr->cr_ref, cr)); | __func__, cr->cr_ref, cr)); | ||||
if (cr->cr_ref > 0) { | if (cr->cr_ref > 0) { | ||||
mtx_unlock(&cr->cr_mtx); | mtx_unlock(&cr->cr_mtx); | ||||
return; | return; | ||||
} | } | ||||
crfree_final(cr); | crfree_final(cr); | ||||
} | } | ||||
▲ Show 20 Lines • Show All 124 Lines • ▼ Show 20 Lines | if (__predict_true(td->td_realucred == cr)) { | ||||
KASSERT(cr->cr_users > 0, ("%s: users %d not > 0 on cred %p", | KASSERT(cr->cr_users > 0, ("%s: users %d not > 0 on cred %p", | ||||
__func__, cr->cr_users, cr)); | __func__, cr->cr_users, cr)); | ||||
td->td_ucredref--; | td->td_ucredref--; | ||||
return; | return; | ||||
} | } | ||||
mtx_lock(&cr->cr_mtx); | mtx_lock(&cr->cr_mtx); | ||||
KASSERT(cr->cr_users >= 0, ("%s: users %d not >= 0 on cred %p", | KASSERT(cr->cr_users >= 0, ("%s: users %d not >= 0 on cred %p", | ||||
__func__, cr->cr_users, cr)); | __func__, cr->cr_users, cr)); | ||||
cr->cr_ref--; | cr->cr_ref--; | ||||
zleiUnsubmitted Not Done Inline ActionsAnd assert an underflow on decrease. KASSERT(cr->cr_ref > 0, ("will underflow")); zlei: And assert an underflow on decrease.
```
KASSERT(cr->cr_ref > 0, ("will underflow"));
``` | |||||
mjgAuthorUnsubmitted Done Inline Actionssee the above comment why this can't work mjg: see the above comment why this can't work | |||||
if (cr->cr_users > 0) { | if (cr->cr_users > 0) { | ||||
mtx_unlock(&cr->cr_mtx); | mtx_unlock(&cr->cr_mtx); | ||||
return; | return; | ||||
} | } | ||||
KASSERT(cr->cr_ref >= 0, ("%s: ref %d not >= 0 on cred %p", | KASSERT(cr->cr_ref >= 0, ("%s: ref %ld not >= 0 on cred %p", | ||||
__func__, cr->cr_ref, cr)); | __func__, cr->cr_ref, cr)); | ||||
if (cr->cr_ref > 0) { | if (cr->cr_ref > 0) { | ||||
mtx_unlock(&cr->cr_mtx); | mtx_unlock(&cr->cr_mtx); | ||||
return; | return; | ||||
} | } | ||||
crfree_final(cr); | crfree_final(cr); | ||||
} | } | ||||
static void | static void | ||||
crfree_final(struct ucred *cr) | crfree_final(struct ucred *cr) | ||||
{ | { | ||||
KASSERT(cr->cr_users == 0, ("%s: users %d not == 0 on cred %p", | KASSERT(cr->cr_users == 0, ("%s: users %d not == 0 on cred %p", | ||||
__func__, cr->cr_users, cr)); | __func__, cr->cr_users, cr)); | ||||
KASSERT(cr->cr_ref == 0, ("%s: ref %d not == 0 on cred %p", | KASSERT(cr->cr_ref == 0, ("%s: ref %ld not == 0 on cred %p", | ||||
__func__, cr->cr_ref, cr)); | __func__, cr->cr_ref, cr)); | ||||
/* | /* | ||||
* Some callers of crget(), such as nfs_statfs(), allocate a temporary | * Some callers of crget(), such as nfs_statfs(), allocate a temporary | ||||
* credential, but don't allocate a uidinfo structure. | * credential, but don't allocate a uidinfo structure. | ||||
*/ | */ | ||||
if (cr->cr_uidinfo != NULL) | if (cr->cr_uidinfo != NULL) | ||||
uifree(cr->cr_uidinfo); | uifree(cr->cr_uidinfo); | ||||
Show All 21 Lines | |||||
void | void | ||||
crcopy(struct ucred *dest, struct ucred *src) | crcopy(struct ucred *dest, struct ucred *src) | ||||
{ | { | ||||
KASSERT(dest->cr_ref == 1, ("crcopy of shared ucred")); | KASSERT(dest->cr_ref == 1, ("crcopy of shared ucred")); | ||||
bcopy(&src->cr_startcopy, &dest->cr_startcopy, | bcopy(&src->cr_startcopy, &dest->cr_startcopy, | ||||
(unsigned)((caddr_t)&src->cr_endcopy - | (unsigned)((caddr_t)&src->cr_endcopy - | ||||
(caddr_t)&src->cr_startcopy)); | (caddr_t)&src->cr_startcopy)); | ||||
dest->cr_flags = src->cr_flags; | |||||
crsetgroups(dest, src->cr_ngroups, src->cr_groups); | crsetgroups(dest, src->cr_ngroups, src->cr_groups); | ||||
uihold(dest->cr_uidinfo); | uihold(dest->cr_uidinfo); | ||||
uihold(dest->cr_ruidinfo); | uihold(dest->cr_ruidinfo); | ||||
prison_hold(dest->cr_prison); | prison_hold(dest->cr_prison); | ||||
loginclass_hold(dest->cr_loginclass); | loginclass_hold(dest->cr_loginclass); | ||||
#ifdef AUDIT | #ifdef AUDIT | ||||
audit_cred_copy(src, dest); | audit_cred_copy(src, dest); | ||||
#endif | #endif | ||||
▲ Show 20 Lines • Show All 90 Lines • ▼ Show 20 Lines | proc_unset_cred(struct proc *p) | ||||
MPASS(p->p_state == PRS_ZOMBIE || p->p_state == PRS_NEW); | MPASS(p->p_state == PRS_ZOMBIE || p->p_state == PRS_NEW); | ||||
cr = p->p_ucred; | cr = p->p_ucred; | ||||
p->p_ucred = NULL; | p->p_ucred = NULL; | ||||
KASSERT(cr->cr_users > 0, ("%s: users %d not > 0 on cred %p", | KASSERT(cr->cr_users > 0, ("%s: users %d not > 0 on cred %p", | ||||
__func__, cr->cr_users, cr)); | __func__, cr->cr_users, cr)); | ||||
mtx_lock(&cr->cr_mtx); | mtx_lock(&cr->cr_mtx); | ||||
cr->cr_users--; | cr->cr_users--; | ||||
if (cr->cr_users == 0) | if (cr->cr_users == 0) | ||||
KASSERT(cr->cr_ref > 0, ("%s: ref %d not > 0 on cred %p", | KASSERT(cr->cr_ref > 0, ("%s: ref %ld not > 0 on cred %p", | ||||
__func__, cr->cr_ref, cr)); | __func__, cr->cr_ref, cr)); | ||||
mtx_unlock(&cr->cr_mtx); | mtx_unlock(&cr->cr_mtx); | ||||
crfree(cr); | crfree(cr); | ||||
} | } | ||||
struct ucred * | struct ucred * | ||||
crcopysafe(struct proc *p, struct ucred *cr) | crcopysafe(struct proc *p, struct ucred *cr) | ||||
{ | { | ||||
▲ Show 20 Lines • Show All 272 Lines • Show Last 20 Lines |
Please use __SIZEOF_LONG__ or some other similar variable for these; CHERI architectures are neither ILP32 nor LP64 but the native integer size is still 32/64-bit depending on the base architecture (only ever 64-bit in CheriBSD). Though really for the formats I'd be inclined to say just use intmax_t, it's not performance-critical and involves (undocumented) tight coupling between here and ucred.h. Though why not just use long itself?