PERFORCE change 164527 for review

[Edward Tomasz Napierala]

http://perforce.freebsd.org/chv.cgi?CH=164527

Change 164527 by trasz at trasz_victim on 2009/06/16 19:32:38

	Plug a memory ('struct gidinfo') leak introduced in a previous
	commit and fix another LOR.

Affected files ...

.. //depot/projects/soc2009/trasz_limits/sys/kern/kern_prot.c#8 edit

Differences ...

==== //depot/projects/soc2009/trasz_limits/sys/kern/kern_prot.c#8 (text+ko) ====

@@ -733,6 +733,7 @@
 
 fail:
 	PROC_UNLOCK(p);
+	gifree(gip);
 	crfree(newcred);
 	return (error);
 }
@@ -783,6 +784,7 @@
 
 fail:
 	PROC_UNLOCK(p);
+	gifree(egip);
 	crfree(newcred);
 	return (error);
 }
@@ -813,12 +815,15 @@
 {
 	struct proc *p = td->td_proc;
 	struct ucred *newcred, *oldcred;
-	int i, error;
+	struct gidinfo *gidinfos[NGROUPS], *oldgidinfos[NGROUPS];
+	int i, error, oldngroups = 0;
 
 	if (ngrp > NGROUPS)
 		return (EINVAL);
 	AUDIT_ARG(groupset, groups, ngrp);
 	newcred = crget();
+	for (i = 0; i < ngrp; i++)
+		gidinfos[i] = gifind(groups[i]);
 	PROC_LOCK(p);
 	oldcred = p->p_ucred;
 
@@ -844,26 +849,39 @@
 		 * have the egid in the groups[0]).  We risk security holes
 		 * when running non-BSD software if we do not do the same.
 		 */
+		oldngroups = newcred->cr_ngroups - 1;
+		for (i = 0; i < oldngroups; i++)
+			oldgidinfos[i] = newcred->cr_gidinfos[i + 1];
 		newcred->cr_ngroups = 1;
-		for (i = 1; i < newcred->cr_ngroups; i++)
-			gifree(newcred->cr_gidinfos[i]);
 	} else {
-		for (i = 0; i < newcred->cr_ngroups; i++)
-			gifree(newcred->cr_gidinfos[i]);
+		oldngroups = newcred->cr_ngroups;
+		for (i = 0; i < oldngroups; i++)
+			oldgidinfos[i] = newcred->cr_gidinfos[i];
 		bcopy(groups, newcred->cr_groups, ngrp * sizeof(gid_t));
 		newcred->cr_ngroups = ngrp;
 		for (i = 0; i < newcred->cr_ngroups; i++)
-			newcred->cr_gidinfos[i] = gifind(newcred->cr_groups[i]);
+			newcred->cr_gidinfos[i] = gidinfos[i];
 	}
 	setsugid(p);
 	p->p_ucred = newcred;
 	PROC_UNLOCK(p);
+	for (i = 0; i < oldngroups; i++)
+		gifree(oldgidinfos[i]);
+	/* Don't free gidinfos[]. */
 	crfree(oldcred);
+	for (i = 0; i < newcred->cr_ngroups; i++)
+		KASSERT(newcred->cr_gidinfos[i]->gi_gid == newcred->cr_groups[i], ("Whoops."));
 	return (0);
 
 fail:
 	PROC_UNLOCK(p);
+	for (i = 0; i < oldngroups; i++)
+		gifree(oldgidinfos[i]);
+	for (i = 0; i < ngrp; i++)
+		gifree(gidinfos[i]);
 	crfree(newcred);
+	for (i = 0; i < newcred->cr_ngroups; i++)
+		KASSERT(newcred->cr_gidinfos[i]->gi_gid == newcred->cr_groups[i], ("Whoops."));
 	return (error);
 }
 
@@ -995,6 +1013,7 @@
 
 fail:
 	PROC_UNLOCK(p);
+	gifree(egip);
 	crfree(newcred);
 	return (error);
 }
@@ -1150,6 +1169,7 @@
 
 fail:
 	PROC_UNLOCK(p);
+	gifree(egip);
 	crfree(newcred);
 	return (error);
 }