Subject: Re: kern/33630: pool corrupt (pretty reproducable for me)
To: None <kern-bug-people@netbsd.org, gnats-admin@netbsd.org,>
From: Martin Husemann <martin@duskware.de>
List: netbsd-bugs
Date: 06/27/2006 16:35:02
The following reply was made to PR kern/33630; it has been noted by GNATS.
From: Martin Husemann <martin@duskware.de>
To: gnats-bugs@NetBSD.org
Cc:
Subject: Re: kern/33630: pool corrupt (pretty reproducable for me)
Date: Tue, 27 Jun 2006 18:32:17 +0200
--sdtB3X0nJg68CQEu
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
After seeing the free list corruption I considered something external
overwriting neighbour items - so I tried to realy make sure nothing outside
the kauth module could access the pointers. I changed kauth_cred_t into an
integer, so pointers to the real data would only be known to kern_auth.c,
and I (again) added rigid consistency checks, but this time including
all kauth_creds, not only the currently changed one.
With below patch (and lots of boring, mechanic changes to cope with the
integer instead of a pointer) I could not trigger the problem.
IMHO this leaves only the pool code as possible culprit.
Martin
--sdtB3X0nJg68CQEu
Content-Type: text/plain; charset=us-ascii
Content-Disposition: attachment; filename=patch
Index: sys/kauth.h
===================================================================
RCS file: /cvsroot/src/sys/sys/kauth.h,v
retrieving revision 1.3
diff -u -p -r1.3 kauth.h
--- sys/kauth.h 28 May 2006 06:49:27 -0000 1.3
+++ sys/kauth.h 27 Jun 2006 16:23:14 -0000
@@ -75,8 +75,8 @@ typedef int (*kauth_scope_callback_t)(ka
*/
#define KAUTH_GENERIC_ISSUSER 1 /* check for super-user */
-#define NOCRED ((kauth_cred_t)-1) /* no credential available */
-#define FSCRED ((kauth_cred_t)-2) /* filesystem credential */
+#define NOCRED (-1) /* no credential available */
+#define FSCRED (-2) /* filesystem credential */
/*
* Prototypes.
Index: kern/kern_auth.c
===================================================================
RCS file: /cvsroot/src/sys/kern/kern_auth.c,v
retrieving revision 1.8
diff -u -p -r1.8 kern_auth.c
--- kern/kern_auth.c 13 Jun 2006 22:56:46 -0000 1.8
+++ kern/kern_auth.c 27 Jun 2006 16:23:14 -0000
@@ -50,6 +50,7 @@
* Credentials.
*/
struct kauth_cred {
+ u_int32_t magic1;
struct simplelock cr_lock; /* lock */
uint32_t cr_refcnt; /* reference count */
uid_t cr_uid; /* user id */
@@ -60,6 +61,7 @@ struct kauth_cred {
gid_t cr_svgid; /* saved effective group id */
uint16_t cr_ngroups; /* number of groups */
gid_t cr_groups[NGROUPS]; /* group memberships */
+ u_int32_t magic2;
};
/*
@@ -98,51 +100,146 @@ static struct simplelock scopes_lock;
static kauth_scope_t kauth_builtin_scope_generic;
static kauth_scope_t kauth_builtin_scope_process;
+#define KCMAGIC1 0x05040311
+#define KCMAGIC2 0x11568322
+#define FREE_KCMAGIC1 0xdead0001
+#define FREE_KCMAGIC2 0xdead0002
+
+int kauth_cred_verify1(kauth_cred_t c, int alive);
+void kauth_cred_verify(kauth_cred_t c);
+
+struct kauth_cred all_creds[500];
+
+int
+kauth_cred_verify1(int ndx, int alive)
+{
+ int p = 0;
+
+ if (ndx < 0 || ndx >= 500) {
+ printf("kauth_cred_t out of bounds: %d\n", ndx);
+ return 0;
+ }
+ if (!alive) {
+ /* this may be a free item - check alternate magics */
+ if (all_creds[ndx].magic1 == FREE_KCMAGIC1 &&
+ all_creds[ndx].magic2 == FREE_KCMAGIC2)
+ return 1;
+ }
+ if (all_creds[ndx].cr_refcnt <= 0) {
+ p = 1;
+ printf("cred %d, refcount = %d\n", ndx, all_creds[ndx].cr_refcnt);
+ }
+ if (all_creds[ndx].magic1 != KCMAGIC1) {
+ p = 1;
+ printf("cred %d, magic1: 0x%x (should be 0x%x)\n", ndx, all_creds[ndx].magic1, KCMAGIC1);
+ }
+ if (all_creds[ndx].magic2 != KCMAGIC2) {
+ p = 1;
+ printf("cred %d, magic2: 0x%x (should be 0x%x)\n", ndx, all_creds[ndx].magic2, KCMAGIC2);
+ }
+
+ return !p;
+}
+
+void
+kauth_cred_verify(kauth_cred_t c)
+{
+ int i, p = 0;
+
+ for (i = 0; i < 500; i++) {
+ if (!kauth_cred_verify1(i, i==(c-1)))
+ p = 1;
+ }
+ if (p)
+ panic("kauth_cred storage inconsitency found while checking #%d", c);
+}
+
/* Allocate new, empty kauth credentials. */
kauth_cred_t
kauth_cred_alloc(void)
{
- kauth_cred_t cred;
+ static int init_done = 0;
+ struct kauth_cred *cred;
+ int i;
- cred = pool_get(&kauth_cred_pool, PR_WAITOK);
+ if (!init_done) {
+ for (i = 0; i < 500; i++) {
+ all_creds[i].magic1 = FREE_KCMAGIC1;
+ all_creds[i].magic2 = FREE_KCMAGIC2;
+ }
+ init_done = 1;
+ }
+
+ for (i = 0; i < 500; i++) {
+ if (all_creds[i].magic1 == FREE_KCMAGIC1 &&
+ all_creds[i].magic2 == FREE_KCMAGIC2)
+ break;
+ }
+ if (i >= 500)
+ panic("out of kauth_cred items");
+
+ /* cred = pool_get(&kauth_cred_pool, PR_WAITOK); */
+ cred = &all_creds[i];
memset(cred, 0, sizeof(*cred));
simple_lock_init(&cred->cr_lock);
cred->cr_refcnt = 1;
+ cred->magic1 = KCMAGIC1;
+ cred->magic2 = KCMAGIC2;
- return (cred);
+ kauth_cred_verify(i+1);
+
+ return (i+1);
}
+#define CREDPTR(NDX, RES) \
+ if ((NDX) <= 0 || (NDX) > 500) panic("invalid kauth_cred: %d", (NDX)); \
+ RES = &all_creds[(NDX)-1]
+
/* Increment reference count to cred. */
void
-kauth_cred_hold(kauth_cred_t cred)
+kauth_cred_hold(kauth_cred_t c)
{
- KASSERT(cred != NULL);
+ struct kauth_cred *cred;
+ CREDPTR(c, cred);
+
+ kauth_cred_verify(c);
simple_lock(&cred->cr_lock);
cred->cr_refcnt++;
simple_unlock(&cred->cr_lock);
+ kauth_cred_verify(c);
}
/* Decrease reference count to cred. If reached zero, free it. */
void
-kauth_cred_free(kauth_cred_t cred)
+kauth_cred_free(kauth_cred_t c)
{
- KASSERT(cred != NULL);
+ struct kauth_cred *cred;
+
+ CREDPTR(c, cred);
+ kauth_cred_verify(c);
simple_lock(&cred->cr_lock);
cred->cr_refcnt--;
simple_unlock(&cred->cr_lock);
- if (cred->cr_refcnt == 0)
- pool_put(&kauth_cred_pool, cred);
+ if (cred->cr_refcnt == 0) {
+ cred->magic1 = FREE_KCMAGIC1;
+ cred->magic2 = FREE_KCMAGIC2;
+ /* pool_put(&kauth_cred_pool, cred); */
+ }
}
void
-kauth_cred_clone(kauth_cred_t from, kauth_cred_t to)
+kauth_cred_clone(kauth_cred_t f, kauth_cred_t t)
{
- KASSERT(from != NULL);
- KASSERT(to != NULL);
+ struct kauth_cred *from, *to;
+ CREDPTR(f, from);
+ CREDPTR(t, to);
+
+ kauth_cred_verify(f);
+ kauth_cred_verify(t);
to->cr_uid = from->cr_uid;
to->cr_euid = from->cr_euid;
to->cr_svuid = from->cr_svuid;
@@ -152,21 +249,27 @@ kauth_cred_clone(kauth_cred_t from, kaut
to->cr_ngroups = from->cr_ngroups;
memcpy(to->cr_groups, from->cr_groups,
sizeof(to->cr_groups));
+ kauth_cred_verify(f);
+ kauth_cred_verify(t);
}
/*
* Duplicate cred and return a new kauth_cred_t.
*/
kauth_cred_t
-kauth_cred_dup(kauth_cred_t cred)
+kauth_cred_dup(kauth_cred_t c)
{
+ struct kauth_cred *cred;
kauth_cred_t new_cred;
- KASSERT(cred != NULL);
+ CREDPTR(c, cred);
+ kauth_cred_verify(c);
new_cred = kauth_cred_alloc();
- kauth_cred_clone(cred, new_cred);
+ kauth_cred_clone(c, new_cred);
+ kauth_cred_verify(c);
+ kauth_cred_verify(new_cred);
return (new_cred);
}
@@ -176,146 +279,182 @@ kauth_cred_dup(kauth_cred_t cred)
* XXX: Is this even needed? [kauth_cred_copy]
*/
kauth_cred_t
-kauth_cred_copy(kauth_cred_t cred)
+kauth_cred_copy(kauth_cred_t c)
{
+ struct kauth_cred *cred;
kauth_cred_t new_cred;
- KASSERT(cred != NULL);
+ CREDPTR(c, cred);
+ kauth_cred_verify(c);
/* If the provided credentials already have one reference, use them. */
if (cred->cr_refcnt == 1)
- return (cred);
+ return (c);
new_cred = kauth_cred_alloc();
- kauth_cred_clone(cred, new_cred);
+ kauth_cred_clone(c, new_cred);
- kauth_cred_free(cred);
+ kauth_cred_free(c);
+ kauth_cred_verify(new_cred);
return (new_cred);
}
uid_t
-kauth_cred_getuid(kauth_cred_t cred)
+kauth_cred_getuid(kauth_cred_t c)
{
- KASSERT(cred != NULL);
+ struct kauth_cred *cred;
+
+ CREDPTR(c, cred);
+ kauth_cred_verify(c);
return (cred->cr_uid);
}
uid_t
-kauth_cred_geteuid(kauth_cred_t cred)
+kauth_cred_geteuid(kauth_cred_t c)
{
- KASSERT(cred != NULL);
+ struct kauth_cred *cred;
+ CREDPTR(c, cred);
+ kauth_cred_verify(c);
return (cred->cr_euid);
}
uid_t
-kauth_cred_getsvuid(kauth_cred_t cred)
+kauth_cred_getsvuid(kauth_cred_t c)
{
- KASSERT(cred != NULL);
+ struct kauth_cred *cred;
+ CREDPTR(c, cred);
+
+ kauth_cred_verify(c);
return (cred->cr_svuid);
}
gid_t
-kauth_cred_getgid(kauth_cred_t cred)
+kauth_cred_getgid(kauth_cred_t c)
{
- KASSERT(cred != NULL);
+ struct kauth_cred *cred;
+
+ CREDPTR(c, cred);
+ kauth_cred_verify(c);
return (cred->cr_gid);
}
gid_t
-kauth_cred_getegid(kauth_cred_t cred)
+kauth_cred_getegid(kauth_cred_t c)
{
- KASSERT(cred != NULL);
+ struct kauth_cred *cred;
+ CREDPTR(c, cred);
+ kauth_cred_verify(c);
return (cred->cr_egid);
}
gid_t
-kauth_cred_getsvgid(kauth_cred_t cred)
+kauth_cred_getsvgid(kauth_cred_t c)
{
- KASSERT(cred != NULL);
+ struct kauth_cred *cred;
+ CREDPTR(c, cred);
+ kauth_cred_verify(c);
return (cred->cr_svgid);
}
void
-kauth_cred_setuid(kauth_cred_t cred, uid_t uid)
+kauth_cred_setuid(kauth_cred_t c, uid_t uid)
{
- KASSERT(cred != NULL);
+ struct kauth_cred *cred;
+ CREDPTR(c, cred);
+ kauth_cred_verify(c);
cred->cr_uid = uid;
}
void
-kauth_cred_seteuid(kauth_cred_t cred, uid_t uid)
+kauth_cred_seteuid(kauth_cred_t c, uid_t uid)
{
- KASSERT(cred != NULL);
+ struct kauth_cred *cred;
+ CREDPTR(c, cred);
+ kauth_cred_verify(c);
cred->cr_euid = uid;
}
void
-kauth_cred_setsvuid(kauth_cred_t cred, uid_t uid)
+kauth_cred_setsvuid(kauth_cred_t c, uid_t uid)
{
- KASSERT(cred != NULL);
+ struct kauth_cred *cred;
+ CREDPTR(c, cred);
+ kauth_cred_verify(c);
cred->cr_svuid = uid;
}
void
-kauth_cred_setgid(kauth_cred_t cred, gid_t gid)
+kauth_cred_setgid(kauth_cred_t c, gid_t gid)
{
- KASSERT(cred != NULL);
+ struct kauth_cred *cred;
+ CREDPTR(c, cred);
+ kauth_cred_verify(c);
cred->cr_gid = gid;
}
void
-kauth_cred_setegid(kauth_cred_t cred, gid_t gid)
+kauth_cred_setegid(kauth_cred_t c, gid_t gid)
{
- KASSERT(cred != NULL);
+ struct kauth_cred *cred;
+ CREDPTR(c, cred);
+ kauth_cred_verify(c);
cred->cr_egid = gid;
}
void
-kauth_cred_setsvgid(kauth_cred_t cred, gid_t gid)
+kauth_cred_setsvgid(kauth_cred_t c, gid_t gid)
{
- KASSERT(cred != NULL);
+ struct kauth_cred *cred;
+ CREDPTR(c, cred);
+ kauth_cred_verify(c);
cred->cr_svgid = gid;
+ kauth_cred_verify(c);
}
/* Checks if gid is a member of the groups in cred. */
int
-kauth_cred_ismember_gid(kauth_cred_t cred, gid_t gid, int *resultp)
+kauth_cred_ismember_gid(kauth_cred_t c, gid_t gid, int *resultp)
{
+ struct kauth_cred *cred;
int i;
- KASSERT(cred != NULL);
+ CREDPTR(c, cred);
KASSERT(resultp != NULL);
*resultp = 0;
+ kauth_cred_verify(c);
for (i = 0; i < cred->cr_ngroups; i++)
if (cred->cr_groups[i] == gid) {
*resultp = 1;
break;
}
+ kauth_cred_verify(c);
return (0);
}
uint16_t
-kauth_cred_ngroups(kauth_cred_t cred)
+kauth_cred_ngroups(kauth_cred_t c)
{
- KASSERT(cred != NULL);
+ struct kauth_cred *cred;
+ CREDPTR(c, cred);
+ kauth_cred_verify(c);
return (cred->cr_ngroups);
}
@@ -323,21 +462,26 @@ kauth_cred_ngroups(kauth_cred_t cred)
* Return the group at index idx from the groups in cred.
*/
gid_t
-kauth_cred_group(kauth_cred_t cred, uint16_t idx)
+kauth_cred_group(kauth_cred_t c, uint16_t idx)
{
- KASSERT(cred != NULL);
+ struct kauth_cred *cred;
+ CREDPTR(c, cred);
+
KASSERT(idx < cred->cr_ngroups);
+ kauth_cred_verify(c);
return (cred->cr_groups[idx]);
}
/* XXX elad: gmuid is unused for now. */
int
-kauth_cred_setgroups(kauth_cred_t cred, gid_t *grbuf, size_t len, uid_t gmuid)
+kauth_cred_setgroups(kauth_cred_t c, gid_t *grbuf, size_t len, uid_t gmuid)
{
- KASSERT(cred != NULL);
+ struct kauth_cred *cred;
+ CREDPTR(c, cred);
KASSERT(len < sizeof(cred->cr_groups) / sizeof(cred->cr_groups[0]));
+ kauth_cred_verify(c);
simple_lock(&cred->cr_lock);
if (len)
@@ -348,20 +492,24 @@ kauth_cred_setgroups(kauth_cred_t cred,
cred->cr_ngroups = len;
simple_unlock(&cred->cr_lock);
+ kauth_cred_verify(c);
return (0);
}
int
-kauth_cred_getgroups(kauth_cred_t cred, gid_t *grbuf, size_t len)
+kauth_cred_getgroups(kauth_cred_t c, gid_t *grbuf, size_t len)
{
- KASSERT(cred != NULL);
+ struct kauth_cred *cred;
+ CREDPTR(c, cred);
KASSERT(len <= cred->cr_ngroups);
+ kauth_cred_verify(c);
memset(grbuf, 0xff, sizeof(*grbuf) * len);
simple_lock(&cred->cr_lock);
memcpy(grbuf, cred->cr_groups, sizeof(*grbuf) * len);
simple_unlock(&cred->cr_lock);
+ kauth_cred_verify(c);
return (0);
}
@@ -372,11 +520,14 @@ kauth_cred_getgroups(kauth_cred_t cred,
* XXX: root bypasses this!
*/
static int
-kauth_cred_uidmatch(kauth_cred_t cred1, kauth_cred_t cred2)
+kauth_cred_uidmatch(kauth_cred_t c1, kauth_cred_t c2)
{
- KASSERT(cred1 != NULL);
- KASSERT(cred2 != NULL);
+ struct kauth_cred *cred1, *cred2;
+ CREDPTR(c1, cred1);
+ CREDPTR(c2, cred2);
+ kauth_cred_verify(c1);
+ kauth_cred_verify(c2);
/* Are we root? */
if (cred1->cr_euid == 0)
return (1);
@@ -391,10 +542,12 @@ kauth_cred_uidmatch(kauth_cred_t cred1,
}
uint32_t
-kauth_cred_getrefcnt(kauth_cred_t cred)
+kauth_cred_getrefcnt(kauth_cred_t c)
{
- KASSERT(cred != NULL);
+ struct kauth_cred *cred;
+ CREDPTR(c, cred);
+ kauth_cred_verify(c);
return (cred->cr_refcnt);
}
@@ -403,11 +556,13 @@ kauth_cred_getrefcnt(kauth_cred_t cred)
* XXX: For NFS code.
*/
void
-kauth_cred_uucvt(kauth_cred_t cred, const struct uucred *uuc)
+kauth_cred_uucvt(kauth_cred_t c, const struct uucred *uuc)
{
- KASSERT(cred != NULL);
+ struct kauth_cred *cred;
+ CREDPTR(c, cred);
KASSERT(uuc != NULL);
+ kauth_cred_verify(c);
cred->cr_refcnt = 1;
cred->cr_uid = uuc->cr_uid;
cred->cr_euid = uuc->cr_uid;
@@ -416,8 +571,9 @@ kauth_cred_uucvt(kauth_cred_t cred, cons
cred->cr_egid = uuc->cr_gid;
cred->cr_svgid = uuc->cr_gid;
cred->cr_ngroups = min(uuc->cr_ngroups, NGROUPS);
- kauth_cred_setgroups(cred, __UNCONST(uuc->cr_groups),
+ kauth_cred_setgroups(c, __UNCONST(uuc->cr_groups),
cred->cr_ngroups, -1);
+ kauth_cred_verify(c);
}
/*
@@ -425,11 +581,13 @@ kauth_cred_uucvt(kauth_cred_t cred, cons
* XXX: Modelled after crcmp() for NFS.
*/
int
-kauth_cred_uucmp(kauth_cred_t cred, const struct uucred *uuc)
+kauth_cred_uucmp(kauth_cred_t c, const struct uucred *uuc)
{
- KASSERT(cred != NULL);
+ struct kauth_cred *cred;
+ CREDPTR(c, cred);
KASSERT(uuc != NULL);
+ kauth_cred_verify(c);
if (cred->cr_euid == uuc->cr_uid &&
cred->cr_egid == uuc->cr_gid &&
cred->cr_ngroups == uuc->cr_ngroups) {
@@ -440,7 +598,7 @@ kauth_cred_uucmp(kauth_cred_t cred, cons
int ismember;
ismember = 0;
- if (kauth_cred_ismember_gid(cred, uuc->cr_groups[i],
+ if (kauth_cred_ismember_gid(c, uuc->cr_groups[i],
&ismember) != 0 || !ismember)
return (1);
}
@@ -456,17 +614,20 @@ kauth_cred_uucmp(kauth_cred_t cred, cons
* XXX: For sysctl.
*/
void
-kauth_cred_toucred(kauth_cred_t cred, struct ucred *uc)
+kauth_cred_toucred(kauth_cred_t c, struct ucred *uc)
{
- KASSERT(cred != NULL);
+ struct kauth_cred *cred;
+ CREDPTR(c, cred);
KASSERT(uc != NULL);
+ kauth_cred_verify(c);
uc->cr_uid = cred->cr_euid;
uc->cr_gid = cred->cr_egid;
uc->cr_ngroups = min(cred->cr_ngroups,
sizeof(uc->cr_groups) / sizeof(uc->cr_groups[0]));
memcpy(uc->cr_groups, cred->cr_groups,
uc->cr_ngroups * sizeof(uc->cr_groups[0]));
+ kauth_cred_verify(c);
}
/*
@@ -474,17 +635,20 @@ kauth_cred_toucred(kauth_cred_t cred, st
* XXX: For sysctl.
*/
void
-kauth_cred_topcred(kauth_cred_t cred, struct pcred *pc)
+kauth_cred_topcred(kauth_cred_t c, struct pcred *pc)
{
- KASSERT(cred != NULL);
+ struct kauth_cred *cred;
+ CREDPTR(c, cred);
KASSERT(pc != NULL);
+ kauth_cred_verify(c);
pc->pc_ucred = (struct ucred *)cred; /* XXX this is just wrong */
pc->p_ruid = cred->cr_uid;
pc->p_svuid = cred->cr_svuid;
pc->p_rgid = cred->cr_gid;
pc->p_svgid = cred->cr_svgid;
pc->p_refcnt = cred->cr_refcnt;
+ kauth_cred_verify(c);
}
/*
@@ -493,6 +657,7 @@ kauth_cred_topcred(kauth_cred_t cred, st
kauth_cred_t
kauth_cred_get(void)
{
+ kauth_cred_verify(curproc->p_cred);
return (curproc->p_cred);
}
@@ -680,7 +845,7 @@ kauth_authorize_action(kauth_scope_t sco
int error, allow, fail;
/* Sanitize input */
- if (scope == NULL || cred == NULL)
+ if (scope == NULL || cred == 0)
return (EFAULT);
if (!action)
return (EINVAL);
@@ -709,10 +874,12 @@ kauth_authorize_action(kauth_scope_t sco
* Generic scope default callback.
*/
int
-kauth_authorize_cb_generic(kauth_cred_t cred, kauth_action_t action,
+kauth_authorize_cb_generic(kauth_cred_t c, kauth_action_t action,
void *cookie, void *arg0, void *arg1, void *arg2,
void *arg3)
{
+ struct kauth_cred *cred;
+ CREDPTR(c, cred);
int error;
error = KAUTH_RESULT_DEFER;
@@ -805,6 +972,8 @@ int
kauth_authorize_process(kauth_cred_t cred, kauth_action_t action,
struct proc *p, void *arg1, void *arg2, void *arg3)
{
+ kauth_cred_verify(cred);
+
return (kauth_authorize_action(kauth_builtin_scope_process, cred,
action, p, arg1, arg2, arg3));
}
--sdtB3X0nJg68CQEu--