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/12/2006 08:35:01
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: Mon, 12 Jun 2006 10:30:44 +0200

 --bp/iNruPH9dso1Pn
 Content-Type: text/plain; charset=us-ascii
 Content-Disposition: inline
 
 For the record, here is the patch I used.
 I explicitly avoided freeing the cred struct, to catch use after free,
 but the only panics I get are with refcnt and both magics (as well as some
 other space before and after the struct) completely zeroed.
 
 Martin
 
 --bp/iNruPH9dso1Pn
 Content-Type: text/plain; charset=us-ascii
 Content-Disposition: attachment; filename=patch
 
 Index: kern_auth.c
 ===================================================================
 RCS file: /cvsroot/src/sys/kern/kern_auth.c,v
 retrieving revision 1.6
 diff -u -p -r1.6 kern_auth.c
 --- kern_auth.c	28 May 2006 06:52:17 -0000	1.6
 +++ kern_auth.c	12 Jun 2006 08:24:30 -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,6 +100,29 @@ 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
 +
 +void
 +kauth_cred_verify(kauth_cred_t c)
 +{
 +	int p = 0;
 +	if (c->cr_refcnt <= 0) {
 +		p = 1;
 +		printf("\nrefcount = %d\n", c->cr_refcnt);
 +	}
 +	if (c->magic1 != KCMAGIC1) {
 +		p = 1;
 +		printf("\nmagic1: 0x%x (should be 0x%x)\n", c->magic1, KCMAGIC1);
 +	}
 +	if (c->magic2 != KCMAGIC2) {
 +		p = 1;
 +		printf("\nmagic2: 0x%x (should be 0x%x)\n", c->magic2, KCMAGIC2);
 +	}
 +
 +	if (p) panic("kauth_cred bad magic");
 +}
 +
  /* Allocate new, empty kauth credentials. */
  kauth_cred_t
  kauth_cred_alloc(void)
 @@ -108,6 +133,10 @@ kauth_cred_alloc(void)
  	memset(cred, 0, sizeof(*cred));
  	simple_lock_init(&cred->cr_lock);
  	cred->cr_refcnt = 1;
 +	cred->magic1 = KCMAGIC1;
 +	cred->magic2 = KCMAGIC2;
 +
 +	kauth_cred_verify(cred);
  
  	return (cred);
  }
 @@ -118,9 +147,11 @@ kauth_cred_hold(kauth_cred_t cred)
  {
  	KASSERT(cred != NULL);
  
 +	kauth_cred_verify(cred);
          simple_lock(&cred->cr_lock);
          cred->cr_refcnt++;
          simple_unlock(&cred->cr_lock);
 +	kauth_cred_verify(cred);
  }
  
  /* Decrease reference count to cred. If reached zero, free it. */
 @@ -129,12 +160,16 @@ kauth_cred_free(kauth_cred_t cred)
  {
  	KASSERT(cred != NULL);
  
 +	kauth_cred_verify(cred);
  	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 = 0xdeadbeef;
 +		cred->magic2 = 0xcdcdcdcd;
 +		/* pool_put(&kauth_cred_pool, cred); */
 +	}
  }
  
  void
 @@ -143,6 +178,8 @@ kauth_cred_clone(kauth_cred_t from, kaut
  	KASSERT(from != NULL);
  	KASSERT(to != NULL);
  
 +	kauth_cred_verify(from);
 +	kauth_cred_verify(to);
  	to->cr_uid = from->cr_uid;
  	to->cr_euid = from->cr_euid;
  	to->cr_svuid = from->cr_svuid;
 @@ -152,6 +189,8 @@ 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(from);
 +	kauth_cred_verify(to);
  }
  
  /*
 @@ -164,9 +203,12 @@ kauth_cred_dup(kauth_cred_t cred)
  
  	KASSERT(cred != NULL);
  
 +	kauth_cred_verify(cred);
  	new_cred = kauth_cred_alloc();
  
  	kauth_cred_clone(cred, new_cred);
 +	kauth_cred_verify(cred);
 +	kauth_cred_verify(new_cred);
  
  	return (new_cred);
  }
 @@ -181,6 +223,7 @@ kauth_cred_copy(kauth_cred_t cred)
  	kauth_cred_t new_cred;
  
  	KASSERT(cred != NULL);
 +	kauth_cred_verify(cred);
  
  	/* If the provided credentials already have one reference, use them. */
  	if (cred->cr_refcnt == 1)
 @@ -192,6 +235,7 @@ kauth_cred_copy(kauth_cred_t cred)
  
  	kauth_cred_free(cred);
  
 +	kauth_cred_verify(new_cred);
  	return (new_cred);
  }
  
 @@ -200,6 +244,7 @@ kauth_cred_getuid(kauth_cred_t cred)
  {
  	KASSERT(cred != NULL);
  
 +	kauth_cred_verify(cred);
  	return (cred->cr_uid);
  }
  
 @@ -208,6 +253,7 @@ kauth_cred_geteuid(kauth_cred_t cred)
  {
  	KASSERT(cred != NULL);
  
 +	kauth_cred_verify(cred);
  	return (cred->cr_euid);
  }
  
 @@ -216,6 +262,7 @@ kauth_cred_getsvuid(kauth_cred_t cred)
  {
  	KASSERT(cred != NULL);
  
 +	kauth_cred_verify(cred);
  	return (cred->cr_svuid);
  }
  
 @@ -224,6 +271,7 @@ kauth_cred_getgid(kauth_cred_t cred)
  {
  	KASSERT(cred != NULL);
  
 +	kauth_cred_verify(cred);
  	return (cred->cr_gid);
  }
  
 @@ -232,6 +280,7 @@ kauth_cred_getegid(kauth_cred_t cred)
  {
  	KASSERT(cred != NULL);
  
 +	kauth_cred_verify(cred);
  	return (cred->cr_egid);
  }
  
 @@ -240,6 +289,7 @@ kauth_cred_getsvgid(kauth_cred_t cred)
  {
  	KASSERT(cred != NULL);
  
 +	kauth_cred_verify(cred);
  	return (cred->cr_svgid);
  }
  
 @@ -249,6 +299,7 @@ kauth_cred_setuid(kauth_cred_t cred, uid
  	KASSERT(cred != NULL);
  	KASSERT(uid >= 0 && uid <= UID_MAX);
  
 +	kauth_cred_verify(cred);
  	cred->cr_uid = uid;
  }
  
 @@ -258,6 +309,7 @@ kauth_cred_seteuid(kauth_cred_t cred, ui
  	KASSERT(cred != NULL);
  	KASSERT(uid >= 0 && uid <= UID_MAX);
  
 +	kauth_cred_verify(cred);
  	cred->cr_euid = uid;
  }
  
 @@ -267,6 +319,7 @@ kauth_cred_setsvuid(kauth_cred_t cred, u
  	KASSERT(cred != NULL);
  	KASSERT(uid >= 0 && uid <= UID_MAX);
  
 +	kauth_cred_verify(cred);
  	cred->cr_svuid = uid;
  }
  
 @@ -276,6 +329,7 @@ kauth_cred_setgid(kauth_cred_t cred, gid
  	KASSERT(cred != NULL);
  	KASSERT(gid >= 0 && gid <= GID_MAX);
  
 +	kauth_cred_verify(cred);
  	cred->cr_gid = gid;
  }
  
 @@ -285,6 +339,7 @@ kauth_cred_setegid(kauth_cred_t cred, gi
  	KASSERT(cred != NULL);
  	KASSERT(gid >= 0 && gid <= GID_MAX);
  
 +	kauth_cred_verify(cred);
  	cred->cr_egid = gid;
  }
  
 @@ -294,7 +349,9 @@ kauth_cred_setsvgid(kauth_cred_t cred, g
  	KASSERT(cred != NULL);
  	KASSERT(gid >= 0 && gid <= GID_MAX);
  
 +	kauth_cred_verify(cred);
  	cred->cr_svgid = gid;
 +	kauth_cred_verify(cred);
  }
  
  /* Checks if gid is a member of the groups in cred. */
 @@ -309,11 +366,13 @@ kauth_cred_ismember_gid(kauth_cred_t cre
  
  	*resultp = 0;
  
 +	kauth_cred_verify(cred);
  	for (i = 0; i < cred->cr_ngroups; i++)
  		if (cred->cr_groups[i] == gid) {
  			*resultp = 1;
  			break;
  		}
 +	kauth_cred_verify(cred);
  
  	return (0);
  }
 @@ -323,6 +382,7 @@ kauth_cred_ngroups(kauth_cred_t cred)
  {
  	KASSERT(cred != NULL);
  
 +	kauth_cred_verify(cred);
  	return (cred->cr_ngroups);
  }
  
 @@ -335,6 +395,7 @@ kauth_cred_group(kauth_cred_t cred, uint
  	KASSERT(cred != NULL);
  	KASSERT(idx < cred->cr_ngroups);
  
 +	kauth_cred_verify(cred);
  	return (cred->cr_groups[idx]);
  }
  
 @@ -345,6 +406,7 @@ kauth_cred_setgroups(kauth_cred_t cred, 
  	KASSERT(cred != NULL);
  	KASSERT(len < sizeof(cred->cr_groups) / sizeof(cred->cr_groups[0]));
  
 +	kauth_cred_verify(cred);
  	simple_lock(&cred->cr_lock);
  
  	if (len)
 @@ -355,6 +417,7 @@ kauth_cred_setgroups(kauth_cred_t cred, 
  	cred->cr_ngroups = len;
  
  	simple_unlock(&cred->cr_lock);
 +	kauth_cred_verify(cred);
  
  	return (0);
  }
 @@ -365,10 +428,12 @@ kauth_cred_getgroups(kauth_cred_t cred, 
  	KASSERT(cred != NULL);
  	KASSERT(len <= cred->cr_ngroups);
  
 +	kauth_cred_verify(cred);
  	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(cred);
  
  	return (0);
  }
 @@ -384,6 +449,8 @@ kauth_cred_uidmatch(kauth_cred_t cred1, 
  	KASSERT(cred1 != NULL);
  	KASSERT(cred2 != NULL);
  
 +	kauth_cred_verify(cred1);
 +	kauth_cred_verify(cred2);
  	/* Are we root? */
  	if (cred1->cr_euid == 0)
  		return (1);
 @@ -402,6 +469,7 @@ kauth_cred_getrefcnt(kauth_cred_t cred)
  {
  	KASSERT(cred != NULL);
  
 +	kauth_cred_verify(cred);
  	return (cred->cr_refcnt);
  }
  
 @@ -415,6 +483,7 @@ kauth_cred_uucvt(kauth_cred_t cred, cons
  	KASSERT(cred != NULL);
  	KASSERT(uuc != NULL);
  
 +	kauth_cred_verify(cred);
  	cred->cr_refcnt = 1;
  	cred->cr_uid = uuc->cr_uid;
  	cred->cr_euid = uuc->cr_uid;
 @@ -425,6 +494,7 @@ kauth_cred_uucvt(kauth_cred_t cred, cons
  	cred->cr_ngroups = min(uuc->cr_ngroups, NGROUPS);
  	kauth_cred_setgroups(cred, __UNCONST(uuc->cr_groups),
  	    cred->cr_ngroups, -1);
 +	kauth_cred_verify(cred);
  }
  
  /*
 @@ -437,6 +507,7 @@ kauth_cred_uucmp(kauth_cred_t cred, cons
  	KASSERT(cred != NULL);
  	KASSERT(uuc != NULL);
  
 +	kauth_cred_verify(cred);
  	if (cred->cr_euid == uuc->cr_uid &&
  	    cred->cr_egid == uuc->cr_gid &&
  	    cred->cr_ngroups == uuc->cr_ngroups) {
 @@ -468,12 +539,14 @@ kauth_cred_toucred(kauth_cred_t cred, st
  	KASSERT(cred != NULL);
  	KASSERT(uc != NULL);
  
 +	kauth_cred_verify(cred);
  	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(cred);
  }
  
  /*
 @@ -486,12 +559,14 @@ kauth_cred_topcred(kauth_cred_t cred, st
  	KASSERT(cred != NULL);
  	KASSERT(pc != NULL);
  
 +	kauth_cred_verify(cred);
  	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(cred);
  }
  
  /*
 @@ -500,6 +575,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);
  }
  
 @@ -816,6 +892,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));
  }
 
 --bp/iNruPH9dso1Pn--