Skip site navigation (1)Skip section navigation (2) 
Date:      Wed, 28 Jun 2006 21:25:54 GMT
From:      John Baldwin <jhb@FreeBSD.org>
To:        Perforce Change Reviews <perforce@freebsd.org>
Subject:   PERFORCE change 100235 for review
Message-ID:  <200606282125.k5SLPsBI061028@repoman.freebsd.org>
next in thread | raw e-mail | index | archive | help 
http://perforce.freebsd.org/chv.cgi?CH=100235

Change 100235 by jhb@jhb_mutex on 2006/06/28 21:25:03

	- Stick group arrays on the stack, they are tiny.
	- Add a kern_setgroups() for ibcs2.  This actually removes a gross
	  hack from setgroups() where we used to allocate an entire credential
	  just to borrow its groups array.  Now we copy the groups array into
	  an array on the stack and just pass it to kern_setgroups().

Affected files ...

.. //depot/projects/smpng/sys/i386/ibcs2/ibcs2_misc.c#24 edit
.. //depot/projects/smpng/sys/kern/kern_prot.c#93 edit
.. //depot/projects/smpng/sys/sys/syscallsubr.h#37 edit

Differences ...

==== //depot/projects/smpng/sys/i386/ibcs2/ibcs2_misc.c#24 (text+ko) ====

@@ -651,31 +651,23 @@
 	struct thread *td;
 	struct ibcs2_getgroups_args *uap;
 {
-	ibcs2_gid_t *iset;
-	gid_t *gp;
+	ibcs2_gid_t iset[NGROUPS_MAX];
+	gid_t gp[NGROUPS_MAX];
 	int error, i;
 
 	if (uap->gidsetsize < 0)
 		return (EINVAL);
 	if (uap->gidsetsize > NGROUPS_MAX)
 		uap->gidsetsize = NGROUPS_MAX;
-	if (uap->gidsetsize)
-		gp = malloc(uap->gidsetsize * sizeof(gid_t), M_TEMP, M_WAITOK);
-	else
-		gp = NULL;
 
 	error = kern_getgroups(td, uap->gidsetsize, gp, UIO_SYSSPACE);
-	if (error == 0 && gp != NULL && td->td_retval[0] > 0) {
-		iset = malloc(td->td_retval[0] * sizeof(ibcs2_gid_t), M_TEMP,
-		    M_WAITOK);
-		for (i = 0; i < td->td_retval[0]; i++)
-			iset[i] = (ibcs2_gid_t)gp[i];
-		error = copyout(iset, uap->gidset, td->td_retval[0] *
-		    sizeof(ibcs2_gid_t));
-		free(iset, M_TEMP);
-	}
-	free(gp, M_TEMP);
-	return (error);
+	if (error || uap->gidsetsize == 0 || td->td_retval[0] == 0)
+		return (error);
+
+	for (i = 0; i < td->td_retval[0]; i++)
+		iset[i] = (ibcs2_gid_t)gp[i];
+	return (copyout(iset, uap->gidset, td->td_retval[0] *
+	    sizeof(ibcs2_gid_t)));
 }
 
 int
@@ -683,28 +675,21 @@
 	struct thread *td;
 	struct ibcs2_setgroups_args *uap;
 {
+	ibcs2_gid_t iset[NGROUPS_MAX];
+	gid_t gp[NGROUPS_MAX];
 	int error, i;
-	ibcs2_gid_t *iset;
-	struct setgroups_args sa;
-	gid_t *gp;
-	caddr_t sg = stackgap_init();
 
 	if (uap->gidsetsize < 0 || uap->gidsetsize > NGROUPS_MAX)
 		return (EINVAL);
-	sa.gidsetsize = uap->gidsetsize;
-	sa.gidset = stackgap_alloc(&sg, sa.gidsetsize *
-					    sizeof(gid_t *));
-	iset = stackgap_alloc(&sg, sa.gidsetsize *
-			      sizeof(ibcs2_gid_t *));
-	if (sa.gidsetsize) {
-		if ((error = copyin((caddr_t)uap->gidset, (caddr_t)iset, 
-				   sizeof(ibcs2_gid_t *) *
-				   uap->gidsetsize)) != 0)
-			return error;
+	if (uap->gidsetsize && uap->gidset) {
+		error = copyin(uap->gidset, iset, sizeof(ibcs2_gid_t) *
+		    uap->gidsetsize);
+		if (error)
+			return (error);
+		for (i = 0; i < uap->gidsetsize; i++)
+			gp[i] = (gid_t)iset[i];
 	}
-	for (i = 0, gp = sa.gidset; i < sa.gidsetsize; i++)
-		*gp++ = (gid_t)iset[i];
-	return setgroups(td, &sa);
+	return (kern_setgroups(td, uap->gidsetsize, gp));
 }
 
 int

==== //depot/projects/smpng/sys/kern/kern_prot.c#93 (text+ko) ====

@@ -830,28 +830,33 @@
 int
 setgroups(struct thread *td, struct setgroups_args *uap)
 {
+	gid_t groups[NGROUPS];
+	int error;
+
+	if (uap->gidsetsize > NGROUPS)
+		return (EINVAL);
+	error = copyin(uap->gidset, groups, uap->gidsetsize * sizeof(gid_t));
+	if (error)
+		return (error);
+	return (kern_setgroups(td, uap->gidsetsize, groups));
+}
+
+int
+kern_setgroups(struct thread *td, u_int ngrp, gid_t *groups)
+{
 	struct proc *p = td->td_proc;
-	struct ucred *newcred, *tempcred, *oldcred;
-	u_int ngrp;
+	struct ucred *newcred, *oldcred;
 	int error;
 
-	ngrp = uap->gidsetsize;
 	if (ngrp > NGROUPS)
 		return (EINVAL);
-	tempcred = crget();
-	error = copyin(uap->gidset, tempcred->cr_groups, ngrp * sizeof(gid_t));
-	if (error != 0) {
-		crfree(tempcred);
-		return (error);
-	}
-	AUDIT_ARG(groupset, tempcred->cr_groups, ngrp);
+	AUDIT_ARG(groupset, groups, ngrp);
 	newcred = crget();
 	PROC_LOCK(p);
 	oldcred = p->p_ucred;
 
 #ifdef MAC
-	error = mac_check_proc_setgroups(p, oldcred, ngrp,
-	    tempcred->cr_groups);
+	error = mac_check_proc_setgroups(p, oldcred, ngrp, groups);
 	if (error)
 		goto fail;
 #endif
@@ -874,21 +879,18 @@
 		 */
 		newcred->cr_ngroups = 1;
 	} else {
-		bcopy(tempcred->cr_groups, newcred->cr_groups,
-		    ngrp * sizeof(gid_t));
+		bcopy(groups, newcred->cr_groups, ngrp * sizeof(gid_t));
 		newcred->cr_ngroups = ngrp;
 	}
 	setsugid(p);
 	p->p_ucred = newcred;
 	PROC_UNLOCK(p);
-	crfree(tempcred);
 	crfree(oldcred);
 	return (0);
 
 fail:
 	PROC_UNLOCK(p);
 	crfree(newcred);
-	crfree(tempcred);
 	return (error);
 }
 

==== //depot/projects/smpng/sys/sys/syscallsubr.h#37 (text+ko) ====

@@ -141,6 +141,7 @@
 	    struct uio *hdr_uio, struct uio *trl_uio, int compat);
 int	kern_sendit(struct thread *td, int s, struct msghdr *mp, int flags,
 	    struct mbuf *control, enum uio_seg segflg);
+int	kern_setgroups(struct thread *td, u_int ngrp, gid_t *groups);
 int	kern_setitimer(struct thread *, u_int, struct itimerval *,
 	    struct itimerval *);
 int	kern_setrlimit(struct thread *, u_int, struct rlimit *);

Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200606282125.k5SLPsBI061028>