Skip site navigation (1)Skip section navigation (2)
Date:      Sun, 7 Jun 2009 20:15:44 GMT
From:      Robert Watson <rwatson@FreeBSD.org>
To:        Perforce Change Reviews <perforce@freebsd.org>
Subject:   PERFORCE change 163733 for review
Message-ID:  <200906072015.n57KFiZM079558@repoman.freebsd.org>

next in thread | raw e-mail | index | archive | help
http://perforce.freebsd.org/chv.cgi?CH=163733

Change 163733 by rwatson@rwatson_freebsd_capabilities on 2009/06/07 20:15:15

	Continue update for lockless file descriptor reference
	acquisition by adding use of cap_fextract(), for select/poll,
	tty hook registration, socket system calls, and vnode system
	calls.
	
	Use getvnode_cap() for close() auditing and UFS bgfsck rather
	than getvnode, and remove getvnode to prevent more from
	sneaking in un-capabilitized.

Affected files ...

.. //depot/projects/trustedbsd/capabilities/src/sys/kern/sys_generic.c#9 edit
.. //depot/projects/trustedbsd/capabilities/src/sys/kern/tty.c#9 edit
.. //depot/projects/trustedbsd/capabilities/src/sys/kern/uipc_syscalls.c#11 edit
.. //depot/projects/trustedbsd/capabilities/src/sys/kern/vfs_syscalls.c#14 edit
.. //depot/projects/trustedbsd/capabilities/src/sys/security/audit/audit_arg.c#7 edit
.. //depot/projects/trustedbsd/capabilities/src/sys/ufs/ffs/ffs_alloc.c#6 edit

Differences ...

==== //depot/projects/trustedbsd/capabilities/src/sys/kern/sys_generic.c#9 (text+ko) ====

@@ -37,6 +37,7 @@
 #include <sys/cdefs.h>
 __FBSDID("$FreeBSD: src/sys/kern/sys_generic.c,v 1.172 2009/05/14 03:24:22 jeff Exp $");
 
+#include "opt_capabilities.h"
 #include "opt_compat.h"
 #include "opt_ktrace.h"
 
@@ -970,6 +971,37 @@
 	return (n);
 }
 
+static __inline int
+getselfd_cap(struct filedesc *fdp, int fd, struct file **fpp)
+{
+	struct file *fp;
+#ifdef CAPABILITIES
+	struct file *fp_fromcap;
+	int error;
+#endif
+
+	if ((fp = fget_unlocked(fdp, fd)) == NULL)
+		return (EBADF);
+#ifdef CAPABILITIES
+	/*
+	 * If the file descriptor is for a capability, test righst and use
+	 * the file descriptor referenced by the capability.
+	 */
+	error = cap_fextract(fp, CAP_EVENT, &fp_fromcap);
+	if (error) {
+		fdrop(fp, curthread);
+		return (error);
+	}
+	if (fp != fp_fromcap) {
+		fhold(fp_fromcap);
+		fdrop(fp, curthread);
+		fp = fp_fromcap;
+	}
+#endif /* CAPABILITIES */
+	*fpp = fp;
+	return (0);
+}
+
 /*
  * Traverse the list of fds attached to this thread's seltd and check for
  * completion.
@@ -985,6 +1017,7 @@
 	struct file *fp;
 	fd_mask bit;
 	int fd, ev, n, idx;
+	int error;
 
 	fdp = td->td_proc->p_fd;
 	stp = td->td_sel;
@@ -996,8 +1029,9 @@
 		/* If the selinfo wasn't cleared the event didn't fire. */
 		if (si != NULL)
 			continue;
-		if ((fp = fget_unlocked(fdp, fd)) == NULL)
-			return (EBADF);
+		error = getselfd_cap(fdp, fd, &fp);
+		if (error)
+			return (error);
 		idx = fd / NFDBITS;
 		bit = (fd_mask)1 << (fd % NFDBITS);
 		ev = fo_poll(fp, selflags(ibits, idx, bit), td->td_ucred, td);
@@ -1025,6 +1059,7 @@
 	fd_mask bit;
 	int ev, flags, end, fd;
 	int n, idx;
+	int error;
 
 	fdp = td->td_proc->p_fd;
 	n = 0;
@@ -1035,8 +1070,9 @@
 			flags = selflags(ibits, idx, bit);
 			if (flags == 0)
 				continue;
-			if ((fp = fget_unlocked(fdp, fd)) == NULL)
-				return (EBADF);
+			error = getselfd_cap(fdp, fd, &fp);
+			if (error)
+				return (error);
 			selfdalloc(td, (void *)(uintptr_t)fd);
 			ev = fo_poll(fp, flags, td->td_ucred, td);
 			fdrop(fp, td);

==== //depot/projects/trustedbsd/capabilities/src/sys/kern/tty.c#9 (text+ko) ====

@@ -30,9 +30,11 @@
 #include <sys/cdefs.h>
 __FBSDID("$FreeBSD: src/sys/kern/tty.c,v 1.317 2009/05/29 06:41:23 ed Exp $");
 
+#include "opt_capabilities.h"
 #include "opt_compat.h"
 
 #include <sys/param.h>
+#include <sys/capability.h>
 #include <sys/conf.h>
 #include <sys/cons.h>
 #include <sys/fcntl.h>
@@ -1727,20 +1729,12 @@
 	struct file *fp;
 	struct cdev *dev;
 	struct cdevsw *cdp;
-	struct filedesc *fdp;
 	int error;
 
 	/* Validate the file descriptor. */
-	if ((fdp = p->p_fd) == NULL)
-		return (EBADF);
-
-	fp = fget_unlocked(fdp, fd);
-	if (fp == NULL)
-		return (EBADF);
-	if (fp->f_ops == &badfileops) {
-		error = EBADF;
-		goto done1;
-	}
+	error = fget(curthread, fd, CAP_TTYHOOK, &fp);
+	if (error)
+		return (error);
 	
 	/* Make sure the vnode is bound to a character device. */
 	error = EINVAL;

==== //depot/projects/trustedbsd/capabilities/src/sys/kern/uipc_syscalls.c#11 (text+ko) ====

@@ -108,38 +108,6 @@
 SYSCTL_INT(_kern_ipc, OID_AUTO, nsfbufsused, CTLFLAG_RD, &nsfbufsused, 0,
     "Number of sendfile(2) sf_bufs in use");
 
-#if 0
-/*
- * Convert a user file descriptor to a kernel file entry.  A reference on the
- * file entry is held upon returning.  This is lighter weight than
- * fgetsock(), which bumps the socket reference drops the file reference
- * count instead, as this approach avoids several additional mutex operations
- * associated with the additional reference count.  If requested, return the
- * open file flags.
- */
-static int
-getsock(struct filedesc *fdp, int fd, struct file **fpp, u_int *fflagp)
-{
-	struct file *fp;
-	int error;
-
-	fp = NULL;
-	if (fdp == NULL || (fp = fget_unlocked(fdp, fd)) == NULL) {
-		error = EBADF;
-	} else if (fp->f_type != DTYPE_SOCKET) {
-		fdrop(fp, curthread);
-		fp = NULL;
-		error = ENOTSOCK;
-	} else {
-		if (fflagp != NULL)
-			*fflagp = fp->f_flag;
-		error = 0;
-	}
-	*fpp = fp;
-	return (error);
-}
-#endif
-
 /*
  * Convert a user file descriptor to a kernel file entry and check that, if
  * it is a capability, the right rights are present.  A reference on the file
@@ -150,45 +118,38 @@
     struct file **fpp, u_int *fflagp)
 {
 	struct file *fp;
+#ifdef CAPABILITIES
+	struct file *fp_fromcap;
+#endif
 	int error;
 
 	fp = NULL;
-	if (fdp == NULL) {
-		*fpp = NULL;
+	if (fdp == NULL || (fp = fget_unlocked(fdp, fd)) == NULL)
 		return (EBADF);
-	}
-	FILEDESC_SLOCK(fdp);
-	fp = fget_locked(fdp, fd);
-	if (fp == NULL) {
-		error = EBADF;
-		goto out;
-	}
-
+#ifdef CAPABILITIES
 	/*
 	 * If the file descriptor is for a capability, test rights and use
 	 * the file descriptor referenced by the capability.
 	 */
-#ifdef CAPABILITIES
-	error = cap_fextract(fp, rights, &fp);
+	error = cap_fextract(fp, rights, &fp_fromcap);
 	if (error) {
-		fp = NULL;
-		goto out;
+		fdrop(fp, curthread);
+		return (error);
+	}
+	if (fp != fp_fromcap) {
+		fhold(fp_fromcap);
+		fdrop(fp, curthread);
+		fp = fp_fromcap;
 	}
 #endif /* CAPABILITIES */
 	if (fp->f_type != DTYPE_SOCKET) {
-		error = ENOTSOCK;
-		fp = NULL;
-		goto out;
-	} else {
-		fhold(fp);
-		if (fflagp != NULL)
-			*fflagp = fp->f_flag;
-		error = 0;
+		fdrop(fp, curthread);
+		return (ENOTSOCK);
 	}
-out:
-	FILEDESC_SUNLOCK(fdp);
+	if (fflagp != NULL)
+		*fflagp = fp->f_flag;
 	*fpp = fp;
-	return (error);
+	return (0);
 }
 
 /*

==== //depot/projects/trustedbsd/capabilities/src/sys/kern/vfs_syscalls.c#14 (text+ko) ====

@@ -117,31 +117,6 @@
 #endif
 
 /*
- * Convert a user file descriptor to a kernel file entry.
- * A reference on the file entry is held upon returning.
- */
-int
-getvnode(fdp, fd, fpp)
-	struct filedesc *fdp;
-	int fd;
-	struct file **fpp;
-{
-	int error;
-	struct file *fp;
-
-	error = 0;
-	fp = NULL;
-	if (fdp == NULL || (fp = fget_unlocked(fdp, fd)) == NULL)
-		error = EBADF;
-	else if (fp->f_vnode == NULL) {
-		error = EINVAL;
-		fdrop(fp, curthread);
-	}
-	*fpp = fp;
-	return (error);
-}
-
-/*
  * Convert a user file descriptor to a kernel file entry and check that, if
  * it is a capability, the right rights are present.  A reference on the file
  * entry is held upon returning.
@@ -151,6 +126,9 @@
     struct file **fpp)
 {
 	struct file *fp;
+#ifdef CAPABILITIES
+	struct file *fp_fromcap;
+#endif
 	int error;
 
 	error = 0;
@@ -162,18 +140,23 @@
 	 * If the file descriptor is for a capability, test rights and use
 	 * the file descriptor referenced by the capability.
 	 */
-	error = cap_fextract(fp, rights, &fp);
+	error = cap_fextract(fp, rights, &fp_fromcap);
 	if (error) {
 		fdrop(fp, curthread);
 		return (error);
 	}
+	if (fp != fp_fromcap) {
+		fhold(fp_fromcap);
+		fdrop(fp, curthread);
+		fp = fp_fromcap;
+	}
 #endif /* CAPABILITIES */
 	if (fp->f_vnode == NULL) {
 		fdrop(fp, curthread);
 		return (EINVAL);
 	}
 	*fpp = fp;
-	return (error);
+	return (0);
 }
 
 /*
@@ -807,7 +790,7 @@
 	int error;
 
 	AUDIT_ARG(fd, uap->fd);
-	if ((error = getvnode(fdp, uap->fd, &fp)) != 0)
+	if ((error = getvnode_cap(fdp, uap->fd, CAP_FCHDIR, &fp)) != 0)
 		return (error);
 	vp = fp->f_vnode;
 	VREF(vp);

==== //depot/projects/trustedbsd/capabilities/src/sys/security/audit/audit_arg.c#7 (text) ====

@@ -859,7 +859,7 @@
 
 	audit_arg_fd(fd);
 
-	if (getvnode(td->td_proc->p_fd, fd, &fp) != 0)
+	if (getvnode_cap(td->td_proc->p_fd, fd, 0, &fp) != 0)
 		return;
 
 	vp = fp->f_vnode;

==== //depot/projects/trustedbsd/capabilities/src/sys/ufs/ffs/ffs_alloc.c#6 (text+ko) ====

@@ -62,9 +62,11 @@
 #include <sys/cdefs.h>
 __FBSDID("$FreeBSD: src/sys/ufs/ffs/ffs_alloc.c,v 1.153 2009/05/17 20:26:00 alc Exp $");
 
+#include "opt_capabilities.h"
 #include "opt_quota.h"
 
 #include <sys/param.h>
+#include <sys/capability.h>
 #include <sys/systm.h>
 #include <sys/bio.h>
 #include <sys/buf.h>
@@ -2411,7 +2413,8 @@
 		return (error);
 	if (cmd.version != FFS_CMD_VERSION)
 		return (ERPCMISMATCH);
-	if ((error = getvnode(curproc->p_fd, cmd.handle, &fp)) != 0)
+	if ((error = getvnode_cap(curproc->p_fd, cmd.handle, CAP_FSCK, &fp))
+	    != 0)
 		return (error);
 	vn_start_write(fp->f_data, &mp, V_WAIT);
 	if (mp == 0 || strncmp(mp->mnt_stat.f_fstypename, "ufs", MFSNAMELEN)) {



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