Skip site navigation (1)Skip section navigation (2) 
Date:      Thu, 29 Jun 2006 14:16:51 GMT
From:      John Baldwin <jhb@FreeBSD.org>
To:        Perforce Change Reviews <perforce@freebsd.org>
Subject:   PERFORCE change 100288 for review
Message-ID:  <200606291416.k5TEGp5N068674@repoman.freebsd.org>
next in thread | raw e-mail | index | archive | help 
http://perforce.freebsd.org/chv.cgi?CH=100288

Change 100288 by jhb@jhb_mutex on 2006/06/29 14:16:06

	Make kern_getgroups() saner by pusing the copyout() to the caller
	and assuming UIO_SYSSPACE.  Also, the caller now has to set
	td->td_retval[0].  I think this should be a general rule that
	kern_*() should not be setting td->td_retval.  Will need to audit
	that eventually.

Affected files ...

.. //depot/projects/smpng/sys/i386/ibcs2/ibcs2_misc.c#25 edit
.. //depot/projects/smpng/sys/kern/kern_prot.c#94 edit
.. //depot/projects/smpng/sys/sys/syscallsubr.h#38 edit

Differences ...

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

@@ -653,21 +653,22 @@
 {
 	ibcs2_gid_t iset[NGROUPS_MAX];
 	gid_t gp[NGROUPS_MAX];
-	int error, i;
+	int error, i, ngrp;
 
 	if (uap->gidsetsize < 0)
 		return (EINVAL);
-	if (uap->gidsetsize > NGROUPS_MAX)
-		uap->gidsetsize = NGROUPS_MAX;
-
-	error = kern_getgroups(td, uap->gidsetsize, gp, UIO_SYSSPACE);
-	if (error || uap->gidsetsize == 0 || td->td_retval[0] == 0)
+	ngrp = MIN(uap->gidsetsize, NGROUPS_MAX);
+	error = kern_getgroups(td, &ngrp, gp);
+	if (error)
 		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)));
+	if (uap->gidsetsize > 0) {
+		for (i = 0; i < ngrp; i++)
+			iset[i] = (ibcs2_gid_t)gp[i];
+		error = copyout(iset, uap->gidset, ngrp * sizeof(ibcs2_gid_t));
+	}
+	if (error == 0)
+		td->td_retval[0] = ngrp;
+	return (error);
 }
 
 int

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

@@ -299,35 +299,36 @@
 int
 getgroups(struct thread *td, register struct getgroups_args *uap)
 {
-	return (kern_getgroups(td, uap->gidsetsize, uap->gidset,
-	    UIO_USERSPACE));
+	gid_t groups[NGROUPS];
+	int error, ngrp;
+
+        ngrp = MIN(uap->gidsetsize, NGROUPS);
+	error = kern_getgroups(td, &ngrp, groups);
+	if (error)
+		return (error);
+	if (uap->gidsetsize > 0)
+		error = copyout(groups, uap->gidset, ngrp * sizeof(gid_t));
+	if (error == 0)
+		td->td_retval[0] = ngrp;
+	return (error);
 }
 
 int
-kern_getgroups(struct thread *td, u_int gidsetsize, gid_t *gidset,
-    enum uio_seg gidsetseg)
+kern_getgroups(struct thread *td, u_int *ngrp, gid_t *groups)
 {
 	struct ucred *cred;
-	u_int ngrp;
 	int error;
 
 	cred = td->td_ucred;
-	if ((ngrp = gidsetsize) == 0) {
-		td->td_retval[0] = cred->cr_ngroups;
+	if (*ngrp == 0) {
+		*ngrp = cred->cr_ngroups;
 		return (0);
 	}
-	if (ngrp < cred->cr_ngroups)
+	if (*ngrp < cred->cr_ngroups)
 		return (EINVAL);
-	ngrp = cred->cr_ngroups;
-	if (gidsetseg == UIO_USERSPACE)
-		error = copyout(cred->cr_groups, gidset, ngrp * sizeof(gid_t));
-	else {
-		bcopy(cred->cr_groups, gidset, ngrp * sizeof(gid_t));
-		error = 0;
-	}
-	if (error == 0)
-		td->td_retval[0] = ngrp;
-	return (error);
+	*ngrp = cred->cr_ngroups;
+	bcopy(cred->cr_groups, gidset, ngrp * sizeof(gid_t));
+	return (0);
 }
 
 #ifndef _SYS_SYSPROTO_H_

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

@@ -83,8 +83,7 @@
 	    enum uio_seg tptrseg);
 int	kern_getfsstat(struct thread *td, struct statfs **buf, size_t bufsize,
 	    enum uio_seg bufseg, int flags);
-int	kern_getgroups(struct thread *td, u_int gidsetsize, gid_t *gidset,
-	    enum uio_seg gidsetseg);
+int	kern_getgroups(struct thread *td, u_int *ngrp, gid_t *groups);
 int	kern_getitimer(struct thread *, u_int, struct itimerval *);
 int	kern_getpeername(struct thread *td, int fd, struct sockaddr **sa,
 	    socklen_t *alen);

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