Refactor some duplicated file-system code.

Proposed and received no objections on tech-kern@:

	http://mail-index.netbsd.org/tech-kern/2009/04/18/msg004843.html

sys/fs/ptyfs/ptyfs_vnops.c

@@ -1,4 +1,4 @@
-/*	$NetBSD: ptyfs_vnops.c,v 1.27 2008/01/02 11:48:43 ad Exp $	*/
+/*	$NetBSD: ptyfs_vnops.c,v 1.28 2009/04/20 18:06:27 elad Exp $	*/
 
 /*
  * Copyright (c) 1993, 1995
@@ -76,7 +76,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: ptyfs_vnops.c,v 1.27 2008/01/02 11:48:43 ad Exp $");
+__KERNEL_RCSID(0, "$NetBSD: ptyfs_vnops.c,v 1.28 2009/04/20 18:06:27 elad Exp $");
 
 #include <sys/param.h>
 #include <sys/systm.h>
@@ -464,10 +464,11 @@ ptyfs_chmod(struct vnode *vp, mode_t mode, kauth_cred_t cred, struct lwp *l)
 	struct ptyfsnode *ptyfs = VTOPTYFS(vp);
 	int error;
 
-	if (kauth_cred_geteuid(cred) != ptyfs->ptyfs_uid &&
+	error = common_chmod_allowed(cred, vp, ptyfs->ptyfs_uid,
-	    (error = kauth_authorize_generic(cred, KAUTH_GENERIC_ISSUSER,
+	    ptyfs->ptyfs_gid, mode);
-	    NULL)) != 0)
+	if (error)
-		return error;
+		return (error);
+
 	ptyfs->ptyfs_mode &= ~ALLPERMS;
 	ptyfs->ptyfs_mode |= (mode & ALLPERMS);
 	return 0;
@@ -482,25 +483,17 @@ ptyfs_chown(struct vnode *vp, uid_t uid, gid_t gid, kauth_cred_t cred,
     struct lwp *l)
 {
 	struct ptyfsnode *ptyfs = VTOPTYFS(vp);
-	int		error, ismember = 0;
+	int error;
 
 	if (uid == (uid_t)VNOVAL)
 		uid = ptyfs->ptyfs_uid;
 	if (gid == (gid_t)VNOVAL)
 		gid = ptyfs->ptyfs_gid;
-	/*
+
-	 * If we don't own the file, are trying to change the owner
+	error = common_chown_allowed(cred, ptyfs->ptyfs_uid, ptyfs->ptyfs_gid,
-	 * of the file, or are not a member of the target group,
+	    uid, gid);
-	 * the caller's credentials must imply super-user privilege
+	if (error)
-	 * or the call fails.
+		return (error);
-	 */
-	if ((kauth_cred_geteuid(cred) != ptyfs->ptyfs_uid || uid != ptyfs->ptyfs_uid ||
-	    (gid != ptyfs->ptyfs_gid &&
-	    !(kauth_cred_getegid(cred) == gid ||
-	    (kauth_cred_ismember_gid(cred, gid, &ismember) == 0 && ismember)))) &&
-	    ((error = kauth_authorize_generic(cred, KAUTH_GENERIC_ISSUSER,
-	    NULL)) != 0))
-		return error;
 
 	ptyfs->ptyfs_gid = gid;
 	ptyfs->ptyfs_uid = uid;

sys/fs/tmpfs/tmpfs_subr.c

@@ -1,4 +1,4 @@
-/*	$NetBSD: tmpfs_subr.c,v 1.50 2009/04/11 11:59:04 markd Exp $	*/
+/*	$NetBSD: tmpfs_subr.c,v 1.51 2009/04/20 18:06:27 elad Exp $	*/
 
 /*
  * Copyright (c) 2005, 2006, 2007 The NetBSD Foundation, Inc.
@@ -35,7 +35,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: tmpfs_subr.c,v 1.50 2009/04/11 11:59:04 markd Exp $");
+__KERNEL_RCSID(0, "$NetBSD: tmpfs_subr.c,v 1.51 2009/04/20 18:06:27 elad Exp $");
 
 #include <sys/param.h>
 #include <sys/dirent.h>
@@ -1018,7 +1018,7 @@ tmpfs_chflags(struct vnode *vp, int flags, kauth_cred_t cred, struct lwp *l)
 int
 tmpfs_chmod(struct vnode *vp, mode_t mode, kauth_cred_t cred, struct lwp *l)
 {
-	int error, ismember = 0;
+	int error;
 	struct tmpfs_node *node;
 
 	KASSERT(VOP_ISLOCKED(vp));
@@ -1033,21 +1033,10 @@ tmpfs_chmod(struct vnode *vp, mode_t mode, kauth_cred_t cred, struct lwp *l)
 	if (node->tn_flags & (IMMUTABLE | APPEND))
 		return EPERM;
 
-	/* XXX: The following comes from UFS code, and can be found in
+	error = common_chmod_allowed(cred, vp, node->tn_uid, node->tn_gid,
-	 * several other file systems.  Shouldn't this be centralized
+	    mode);
-	 * somewhere? */
+	if (error)
-	if (kauth_cred_geteuid(cred) != node->tn_uid &&
+		return (error);
-	    (error = kauth_authorize_generic(cred, KAUTH_GENERIC_ISSUSER,
-	    NULL)))
-		return error;
-	if (kauth_authorize_generic(cred, KAUTH_GENERIC_ISSUSER, NULL) != 0) {
-		if (vp->v_type != VDIR && (mode & S_ISTXT))
-			return EFTYPE;
-
-		if ((kauth_cred_ismember_gid(cred, node->tn_gid,
-		    &ismember) != 0 || !ismember) && (mode & S_ISGID))
-			return EPERM;
-	}
 
 	node->tn_mode = (mode & ALLPERMS);
 
@@ -1072,7 +1061,7 @@ int
 tmpfs_chown(struct vnode *vp, uid_t uid, gid_t gid, kauth_cred_t cred,
     struct lwp *l)
 {
-	int error, ismember = 0;
+	int error;
 	struct tmpfs_node *node;
 
 	KASSERT(VOP_ISLOCKED(vp));
@@ -1095,15 +1084,10 @@ tmpfs_chown(struct vnode *vp, uid_t uid, gid_t gid, kauth_cred_t cred,
 	if (node->tn_flags & (IMMUTABLE | APPEND))
 		return EPERM;
 
-	/* XXX: The following comes from UFS code, and can be found in
+	error = common_chown_allowed(cred, node->tn_uid, node->tn_gid, uid,
-	 * several other file systems.  Shouldn't this be centralized
+	    gid);
-	 * somewhere? */
+	if (error)
-	if ((kauth_cred_geteuid(cred) != node->tn_uid || uid != node->tn_uid ||
+		return (error);
-	    (gid != node->tn_gid && !(kauth_cred_getegid(cred) == gid ||
-	    (kauth_cred_ismember_gid(cred, gid, &ismember) == 0 && ismember)))) &&
-	    ((error = kauth_authorize_generic(cred, KAUTH_GENERIC_ISSUSER,
-	    NULL)) != 0))
-		return error;
 
 	node->tn_uid = uid;
 	node->tn_gid = gid;

sys/fs/udf/udf_vnops.c

@@ -1,4 +1,4 @@
-/* $NetBSD: udf_vnops.c,v 1.38 2009/03/20 23:06:52 reinoud Exp $ */
+/* $NetBSD: udf_vnops.c,v 1.39 2009/04/20 18:06:26 elad Exp $ */
 
 /*
  * Copyright (c) 2006, 2008 Reinoud Zandijk
@@ -32,7 +32,7 @@
 
 #include <sys/cdefs.h>
 #ifndef lint
-__KERNEL_RCSID(0, "$NetBSD: udf_vnops.c,v 1.38 2009/03/20 23:06:52 reinoud Exp $");
+__KERNEL_RCSID(0, "$NetBSD: udf_vnops.c,v 1.39 2009/04/20 18:06:26 elad Exp $");
 #endif /* not lint */
 
 
@@ -935,9 +935,8 @@ udf_chown(struct vnode *vp, uid_t new_uid, gid_t new_gid,
 	  kauth_cred_t cred)
 {
 	struct udf_node  *udf_node = VTOI(vp);
-	uid_t euid, uid;
+	uid_t uid;
-	gid_t egid, gid;
+	gid_t gid;
-	int issuperuser, ismember;
 	int error;
 
 #ifdef notyet
@@ -965,26 +964,10 @@ udf_chown(struct vnode *vp, uid_t new_uid, gid_t new_gid,
 	if ((gid_t) ((uint32_t) gid) != gid)
 		return EINVAL;
 
-	/*
-	 * If we don't own the file, are trying to change the owner of the
-	 * file, or are not a member of the target group, the caller's
-	 * credentials must imply super-user privilege or the call fails.
-	 */
-
 	/* check permissions */
-	euid  = kauth_cred_geteuid(cred);
+	error = common_chown_allowed(cred, uid, gid, new_uid, new_gid);
-	egid  = kauth_cred_getegid(cred);
+	if (error)
-	if ((error = kauth_cred_ismember_gid(cred, new_gid, &ismember)))
+		return (error);
-		return error;
-	error = kauth_authorize_generic(cred, KAUTH_GENERIC_ISSUSER, NULL);
-	issuperuser = (error == 0);
-
-	if (!issuperuser) {
-		if ((new_uid != uid) || (euid != uid))
-			return EPERM;
-		if ((new_gid != gid) && !(egid == new_gid || ismember)) 
-			return EPERM;
-	}
 
 	/* change the ownership */
 	udf_setownership(udf_node, new_uid, new_gid);
@@ -1000,9 +983,8 @@ static int
 udf_chmod(struct vnode *vp, mode_t mode, kauth_cred_t cred)
 {
 	struct udf_node  *udf_node = VTOI(vp);
-	uid_t euid, uid;
+	uid_t uid;
-	gid_t egid, gid;
+	gid_t gid;
-	int issuperuser, ismember;
 	int error;
 
 #ifdef notyet
@@ -1019,22 +1001,9 @@ udf_chmod(struct vnode *vp, mode_t mode, kauth_cred_t cred)
 	udf_getownership(udf_node, &uid, &gid);
 
 	/* check permissions */
-	euid  = kauth_cred_geteuid(cred);
+	error = common_chmod_allowed(cred, vp, uid, gid, mode);
-	egid  = kauth_cred_getegid(cred);
+	if (error)
-	if ((error = kauth_cred_ismember_gid(cred, gid, &ismember)))
+		return (error);
-		return error;
-	error = kauth_authorize_generic(cred, KAUTH_GENERIC_ISSUSER, NULL);
-	issuperuser = (error == 0);
-
-	if ((euid != uid) && !issuperuser)
-		return EPERM;
-	if (euid != 0) {
-		if (vp->v_type != VDIR && (mode & S_ISTXT))
-			return EFTYPE;
-
-		if ((!ismember) && (mode & S_ISGID))
-			return EPERM;
-	}
 
 	/* change mode */
 	udf_setaccessmode(udf_node, mode);

sys/kern/vfs_subr.c

@@ -1,4 +1,4 @@
-/*	$NetBSD: vfs_subr.c,v 1.371 2009/04/17 20:22:52 dyoung Exp $	*/
+/*	$NetBSD: vfs_subr.c,v 1.372 2009/04/20 18:06:26 elad Exp $	*/
 
 /*-
  * Copyright (c) 1997, 1998, 2004, 2005, 2007, 2008 The NetBSD Foundation, Inc.
@@ -81,7 +81,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: vfs_subr.c,v 1.371 2009/04/17 20:22:52 dyoung Exp $");
+__KERNEL_RCSID(0, "$NetBSD: vfs_subr.c,v 1.372 2009/04/20 18:06:26 elad Exp $");
 
 #include "opt_ddb.h"
 #include "opt_compat_netbsd.h"
@@ -3206,3 +3206,147 @@ vfs_mount_print(struct mount *mp, int full, void (*pr)(const char *, ...))
 	}
 }
 #endif /* DDB */
+
+/*
+ * Common routine to check if chmod() is allowed.
+ *
+ * Policy:
+ *   - You must be root, or
+ *   - You must own the file, and
+ *     - You must not set the "sticky" bit (meaningless, see chmod(2))
+ *     - You must be a member of the group if you're trying to set the
+ *       SGIDf bit
+ *
+ * cred - credentials of the invoker
+ * vp - vnode of the file-system object
+ * cur_uid, cur_gid - current uid/gid of the file-system object
+ * new_mode - new mode for the file-system object
+ *
+ * Returns 0 if the change is allowed, or an error value otherwise.
+ */
+int
+common_chmod_allowed(kauth_cred_t cred, struct vnode *vp, uid_t cur_uid,
+    gid_t cur_gid, mode_t new_mode)
+{
+	int error;
+
+	/* Superuser can always change mode. */
+	error = kauth_authorize_generic(cred, KAUTH_GENERIC_ISSUSER,
+	    NULL);
+	if (!error) {
+		goto out;
+	}
+
+	/* Otherwise, user must own the file. */
+	if (kauth_cred_geteuid(cred) != cur_uid) {
+		goto out;
+	}
+
+	/*
+	 * Non-root users can't set the sticky bit on files.
+	 */
+	if ((vp->v_type != VDIR) && (new_mode & S_ISTXT)) {
+		error = EFTYPE;
+		goto out;
+	}
+
+	/*
+	 * If the invoker is trying to set the SGID bit on the file,
+	 * check group membership.
+	 */
+	if (new_mode & S_ISGID) {
+		int ismember;
+
+		error = kauth_cred_ismember_gid(cred, cur_gid,
+		    &ismember);
+		if (error)
+			goto out;
+
+		if (!ismember) {
+			error = EPERM;
+			goto out;
+		}
+	}
+
+	error = 0;
+
+ out:
+	return (error);
+}
+
+/*
+ * Common routine to check if chown() is allowed.
+ *
+ * Policy:
+ *   - You must be root, or
+ *   - You must own the file, and
+ *     - You must not try to change ownership, and
+ *     - You must be member of the new group
+ *
+ * cred - credentials of the invoker
+ * cur_uid, cur_gid - current uid/gid of the file-system object
+ * new_uid, new_gid - target uid/gid of the file-system object
+ *
+ * Returns 0 if the change is allowed, or an error value otherwise.
+ */
+int	
+common_chown_allowed(kauth_cred_t cred, uid_t cur_uid, gid_t cur_gid,
+    uid_t new_uid, gid_t new_gid)
+{
+	int error, ismember;
+
+	/*
+	 * You can only change ownership of a file if:
+	 * You are the superuser, or...
+	 */
+	error = kauth_authorize_generic(cred, KAUTH_GENERIC_ISSUSER,
+	    NULL);
+	if (!error) {
+		goto out;
+	}
+
+	/*
+	 * You own the file and...
+	 */
+	if (kauth_cred_geteuid(cred) == cur_uid) {
+		/*
+		 * You don't try to change ownership, and...
+		 */
+		 if (new_uid != cur_uid) {
+			goto out;
+		}
+
+		/*
+		 * You don't try to change group (no-op), or...
+		 */
+		if (new_gid == cur_gid) {
+			error = 0;
+			goto out;
+		}
+
+		/*
+		 * Your effective gid is the new gid, or...
+		 */
+		if (kauth_cred_getegid(cred) == new_gid) {
+			error = 0;
+			goto out;
+		}
+
+		/*
+		 * The new gid is one you're a member of.
+		 */
+		ismember = 0;
+		error = kauth_cred_ismember_gid(cred, new_gid,
+		    &ismember);
+		if (error || !ismember) {
+			error = EPERM;
+			goto out;
+		}
+
+		error = 0;
+	}
+
+ out:
+	return (error);
+}
+

sys/sys/vnode.h

@@ -1,4 +1,4 @@
-/*	$NetBSD: vnode.h,v 1.203 2009/03/19 09:07:54 pooka Exp $	*/
+/*	$NetBSD: vnode.h,v 1.204 2009/04/20 18:06:26 elad Exp $	*/
 
 /*-
  * Copyright (c) 2008 The NetBSD Foundation, Inc.
@@ -663,6 +663,10 @@ void	vfs_timestamp(struct timespec *);
 void	vfs_vnode_print(struct vnode *, int, void (*)(const char *, ...));
 void	vfs_mount_print(struct mount *, int, void (*)(const char *, ...));
 #endif /* DDB */
+
+int	common_chmod_allowed(kauth_cred_t, struct vnode *, uid_t, gid_t,
+	    mode_t);
+int	common_chown_allowed(kauth_cred_t, uid_t, gid_t, uid_t, gid_t);
 #endif /* _KERNEL */
 
 #endif /* !_SYS_VNODE_H_ */

sys/ufs/ext2fs/ext2fs_vnops.c

@@ -1,4 +1,4 @@
-/*	$NetBSD: ext2fs_vnops.c,v 1.83 2008/11/23 10:09:25 mrg Exp $	*/
+/*	$NetBSD: ext2fs_vnops.c,v 1.84 2009/04/20 18:06:27 elad Exp $	*/
 
 /*
  * Copyright (c) 1982, 1986, 1989, 1993
@@ -70,7 +70,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: ext2fs_vnops.c,v 1.83 2008/11/23 10:09:25 mrg Exp $");
+__KERNEL_RCSID(0, "$NetBSD: ext2fs_vnops.c,v 1.84 2009/04/20 18:06:27 elad Exp $");
 
 #include <sys/param.h>
 #include <sys/systm.h>
@@ -442,19 +442,12 @@ static int
 ext2fs_chmod(struct vnode *vp, int mode, kauth_cred_t cred, struct lwp *l)
 {
 	struct inode *ip = VTOI(vp);
-	int error, ismember = 0;
+	int error;
 
-	if (kauth_cred_geteuid(cred) != ip->i_uid &&
+	error = common_chmod_allowed(cred, vp, ip->i_uid, ip->i_gid, mode);
-	    (error = kauth_authorize_generic(cred, KAUTH_GENERIC_ISSUSER,
+	if (error)
-	    NULL)))
 		return (error);
-	if (kauth_authorize_generic(cred, KAUTH_GENERIC_ISSUSER, NULL)) {
+
-		if (vp->v_type != VDIR && (mode & S_ISTXT))
-			return (EFTYPE);
-		if ((kauth_cred_ismember_gid(cred, ip->i_gid, &ismember) != 0 ||
-		    !ismember) && (mode & ISGID))
-			return (EPERM);
-	}
 	ip->i_e2fs_mode &= ~ALLPERMS;
 	ip->i_e2fs_mode |= (mode & ALLPERMS);
 	ip->i_flag |= IN_CHANGE;
@@ -472,23 +465,17 @@ ext2fs_chown(struct vnode *vp, uid_t uid, gid_t gid, kauth_cred_t cred,
 	struct inode *ip = VTOI(vp);
 	uid_t ouid;
 	gid_t ogid;
-	int error = 0, ismember = 0;
+	int error;
 
 	if (uid == (uid_t)VNOVAL)
 		uid = ip->i_uid;
 	if (gid == (gid_t)VNOVAL)
 		gid = ip->i_gid;
-	/*
+
-	 * If we don't own the file, are trying to change the owner
+	error = common_chown_allowed(cred, ip->i_uid, ip->i_gid, uid, gid);
-	 * of the file, or are not a member of the target group,
+	if (error)
-	 * the caller must be superuser or the call fails.
-	 */
-	if ((kauth_cred_geteuid(cred) != ip->i_uid || uid != ip->i_uid ||
- 	    (gid != ip->i_gid &&
-	    !(kauth_cred_getegid(cred) == gid ||
-	    (kauth_cred_ismember_gid(cred, gid, &ismember) == 0 && ismember)))) &&
-	    (error = kauth_authorize_generic(cred, KAUTH_GENERIC_ISSUSER, NULL)))
 		return (error);
+
 	ogid = ip->i_gid;
 	ouid = ip->i_uid;
 

sys/ufs/ufs/ufs_vnops.c

@@ -1,4 +1,4 @@
-/*	$NetBSD: ufs_vnops.c,v 1.173 2009/02/22 20:28:07 ad Exp $	*/
+/*	$NetBSD: ufs_vnops.c,v 1.174 2009/04/20 18:06:27 elad Exp $	*/
 
 /*-
  * Copyright (c) 2008 The NetBSD Foundation, Inc.
@@ -66,7 +66,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: ufs_vnops.c,v 1.173 2009/02/22 20:28:07 ad Exp $");
+__KERNEL_RCSID(0, "$NetBSD: ufs_vnops.c,v 1.174 2009/04/20 18:06:27 elad Exp $");
 
 #if defined(_KERNEL_OPT)
 #include "opt_ffs.h"
@@ -640,21 +640,16 @@ static int
 ufs_chmod(struct vnode *vp, int mode, kauth_cred_t cred, struct lwp *l)
 {
 	struct inode	*ip;
-	int		error, ismember = 0;
+	int		error;
 
 	UFS_WAPBL_JLOCK_ASSERT(vp->v_mount);
 
 	ip = VTOI(vp);
-	if (kauth_cred_geteuid(cred) != ip->i_uid &&
+
-	    (error = kauth_authorize_generic(cred, KAUTH_GENERIC_ISSUSER, NULL)))
+	error = common_chmod_allowed(cred, vp, ip->i_uid, ip->i_gid, mode);
+	if (error)
 		return (error);
-	if (kauth_authorize_generic(cred, KAUTH_GENERIC_ISSUSER, NULL)) {
+
-		if (vp->v_type != VDIR && (mode & S_ISTXT))
-			return (EFTYPE);
-		if ((kauth_cred_ismember_gid(cred, ip->i_gid, &ismember) != 0 ||
-		    !ismember) && (mode & ISGID))
-			return (EPERM);
-	}
 	ip->i_mode &= ~ALLPERMS;
 	ip->i_mode |= (mode & ALLPERMS);
 	ip->i_flag |= IN_CHANGE;
@@ -672,7 +667,7 @@ ufs_chown(struct vnode *vp, uid_t uid, gid_t gid, kauth_cred_t cred,
     	struct lwp *l)
 {
 	struct inode	*ip;
-	int		error, ismember = 0;
+	int		error = 0;
 #ifdef QUOTA
 	uid_t		ouid;
 	gid_t		ogid;
@@ -685,19 +680,9 @@ ufs_chown(struct vnode *vp, uid_t uid, gid_t gid, kauth_cred_t cred,
 		uid = ip->i_uid;
 	if (gid == (gid_t)VNOVAL)
 		gid = ip->i_gid;
-	/*
+
-	 * If we don't own the file, are trying to change the owner
+	error = common_chown_allowed(cred, ip->i_uid, ip->i_gid, uid, gid);
-	 * of the file, or are not a member of the target group,
+	if (error)
-	 * the caller's credentials must imply super-user privilege
-	 * or the call fails.
-	 */
-	if ((kauth_cred_geteuid(cred) != ip->i_uid || uid != ip->i_uid ||
-	    (gid != ip->i_gid &&
-	    !(kauth_cred_getegid(cred) == gid ||
-	    (kauth_cred_ismember_gid(cred, gid, &ismember) == 0 &&
-	    ismember)))) &&
-	    ((error = kauth_authorize_generic(cred, KAUTH_GENERIC_ISSUSER,
-	    NULL)) != 0))
 		return (error);
 
 #ifdef QUOTA