PERFORCE change 111027 for review

[Todd Miller]

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

Change 111027 by millert at millert_g5tower on 2006/12/04 16:44:41

	Fix locking in mac_vnode_label_associate_fdesc():
	Use fp_lookup() not fdfile macro to get the struct fileproc  
	    for an fd and have it lock the proc fd lock.  
	Use socket, posix sem and shm, and pipe locks.

Affected files ...

.. //depot/projects/trustedbsd/sedarwin8/darwin/xnu/security/mac_vfs.c#21 edit

Differences ...

==== //depot/projects/trustedbsd/sedarwin8/darwin/xnu/security/mac_vfs.c#21 (text+ko) ====

@@ -952,15 +952,18 @@
 mac_vnode_label_associate_fdesc(struct mount *mp, struct fdescnode *fnp,
     struct vnode *vp, vfs_context_t ctx)
 {
-	struct fileglob *fg;
+	struct fileproc *fp;
 	struct pseminfo *psem;
 	struct pshminfo *pshm;
 	struct xsocket xso;
 	struct socket *so;
 	struct pipe *cpipe;
 	struct vnode *fvp;
+	struct proc *p;
 	int error;
 
+	error = 0;
+
 	/*
 	 * If no backing file, let the policy choose which label to use.
 	 */
@@ -970,52 +973,76 @@
 		return (0);
 	}
 
-	fg = (*fdfile(vfs_context_proc(ctx), fnp->fd_fd))->f_fglob;
-	switch (fg->fg_type) {
+	p = vfs_context_proc(ctx);
+	error = fp_lookup(p, fnp->fd_fd, &fp, 0);
+	if (error)
+		return (error);
+
+	if (fp->f_fglob == NULL) {
+		error = EBADF;
+		goto out;
+	}
+
+	switch (fp->f_fglob->fg_type) {
 	case DTYPE_VNODE:
-		fvp = (struct vnode *)fg->fg_data;
+		fvp = (struct vnode *)fp->f_fglob->fg_data;
 		if ((error = vnode_getwithref(fvp)))
-			return (error);
+			goto out;
 		MAC_PERFORM(vnode_label_copy, fvp->v_label, vp->v_label);
 		(void)vnode_put(fvp);
 		break;
 	case DTYPE_SOCKET:
-		so = (struct socket *)fg->fg_data;
+		so = (struct socket *)fp->f_fglob->fg_data;
+		SOCK_LOCK(so);
 		sotoxsocket(so, &xso);
 		MAC_PERFORM(vnode_label_associate_socket,
 		    vfs_context_ucred(ctx), &xso, so->so_label,
 		    vp, vp->v_label);
+		SOCK_UNLOCK(so);
 		break;
 	case DTYPE_PSXSHM:
-		/* XXX: should hold the PSHM_SUBSYS lock. */
-		pshm = pshmnodeinfo((struct pshmnode *)fg->fg_data);
-		if (pshm == NULL)
-			return (EINVAL);
-		MAC_PERFORM(vnode_label_associate_posixshm,
-		    vfs_context_ucred(ctx), pshm, pshm->pshm_label,
-		    vp, vp->v_label);
+		PSHM_SUBSYS_LOCK();
+		pshm = pshmnodeinfo((struct pshmnode *)fp->f_fglob->fg_data);
+		if (pshm != NULL) {
+			MAC_PERFORM(vnode_label_associate_posixshm,
+			    vfs_context_ucred(ctx), pshm, pshm->pshm_label,
+			    vp, vp->v_label);
+		} else
+			error = EINVAL;
+		PSHM_SUBSYS_UNLOCK();
 		break;
 	case DTYPE_PSXSEM:
-		/* XXX: should hold the PSEM_SUBSYS lock. */
-		psem = psemnodeinfo((struct psemnode *)fg->fg_data);
-		if (psem == NULL)
-			return (EINVAL);
-		MAC_PERFORM(vnode_label_associate_posixsem,
-		    vfs_context_ucred(ctx), psem, psem->psem_label,
-		    vp, vp->v_label);
+		PSEM_SUBSYS_LOCK();
+		psem = psemnodeinfo((struct psemnode *)fp->f_fglob->fg_data);
+		if (psem != NULL) {
+			MAC_PERFORM(vnode_label_associate_posixsem,
+			    vfs_context_ucred(ctx), psem, psem->psem_label,
+			    vp, vp->v_label);
+		} else
+			error = EINVAL;
+		PSEM_SUBSYS_UNLOCK();
 		break;
 	case DTYPE_PIPE:
-		/* XXX: should PIPE_LOCK */
-		cpipe = (struct pipe *)fg->fg_data;
+		cpipe = (struct pipe *)fp->f_fglob->fg_data;
+		/* kern/sys_pipe.c:pipe_select() suggests this test. */
+		if (cpipe == (struct pipe *)-1) {
+			error = EINVAL;
+			goto out;
+		}
+		PIPE_LOCK(cpipe);
 		MAC_PERFORM(vnode_label_associate_pipe, vfs_context_ucred(ctx),
 		    cpipe, cpipe->pipe_label, vp, vp->v_label);
+		PIPE_UNLOCK(cpipe);
 		break;
 	case DTYPE_KQUEUE:
 	case DTYPE_FSEVENTS:
 	default:
 		MAC_PERFORM(vnode_label_associate_file, vfs_context_ucred(ctx),
-		    mp, mp->mnt_mntlabel, fg, fg->fg_label, vp, vp->v_label);
+		    mp, mp->mnt_mntlabel, fp->f_fglob, fp->f_fglob->fg_label,
+		    vp, vp->v_label);
 		break;
 	}
-	return (0);
+out:
+	fp_drop(p, fnp->fd_fd, fp, 0);
+	return (error);
 }