linux-fsdevel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] generic acl support for ->permission
@ 2004-09-01 15:24 Christoph Hellwig
  2004-09-01 16:00 ` Christoph Hellwig
                   ` (2 more replies)
  0 siblings, 3 replies; 5+ messages in thread
From: Christoph Hellwig @ 2004-09-01 15:24 UTC (permalink / raw)
  To: linux-fsdevel; +Cc: agruen, shaggy, jeffm

Currently we every filesystem with Posix ACLs has it's own
reimplemtation of the generic permission checking code with additonal
ACL support.  This patch

 - adds an optional callback to vfs_permission that filesystems can use
   for ACL support (and renames it to generic_permission because the
   old name was wrong - it wasn't like the other vfs_* functions at all)
 - uses it in ext2, ext3, jfs and reiserfs.  XFS will follow a little
   later as it's permission checking is burried under several layers of
   abtractions.


--- 1.132/fs/exec.c	2004-08-27 09:02:34 +02:00
+++ edited/fs/exec.c	2004-09-01 14:02:43 +02:00
@@ -893,7 +893,7 @@
 	mode = inode->i_mode;
 	/*
 	 * Check execute perms again - if the caller has CAP_DAC_OVERRIDE,
-	 * vfs_permission lets a non-executable through
+	 * generic_permission lets a non-executable through
 	 */
 	if (!(mode & 0111))	/* with at least _one_ execute bit set */
 		return -EACCES;
--- 1.107/fs/namei.c	2004-09-01 01:00:48 +02:00
+++ edited/fs/namei.c	2004-09-01 14:42:21 +02:00
@@ -151,15 +151,19 @@
 	return result;
 }
 
-/*
- *	vfs_permission()
+/**
+ * generic_permission  -  check for access rights on a Posix-like filesystem
+ * @inode:	inode to check access rights for
+ * @mask:	right to check for (%MAY_READ, %MAY_WRITE, %MAY_EXEC)
+ * @check_acl:	optional callback to check for Posix ACLs
  *
- * is used to check for read/write/execute permissions on a file.
+ * Used to check for read/write/execute permissions on a file.
  * We use "fsuid" for this, letting us set arbitrary permissions
  * for filesystem access without changing the "normal" uids which
  * are used for other things..
  */
-int vfs_permission(struct inode * inode, int mask)
+int generic_permission(struct inode *inode, int mask,
+		int (*check_acl)(struct inode *inode, int mask))
 {
 	umode_t			mode = inode->i_mode;
 
@@ -180,8 +184,18 @@
 
 	if (current->fsuid == inode->i_uid)
 		mode >>= 6;
-	else if (in_group_p(inode->i_gid))
-		mode >>= 3;
+	else {
+		if (IS_POSIXACL(inode) && (mode & S_IRWXG) && check_acl) {
+			int error = check_acl(inode, mask);
+			if (error == -EACCES)
+				goto check_capabilities;
+			else if (error != -EAGAIN)
+				return error;
+		}
+
+		if (in_group_p(inode->i_gid))
+			mode >>= 3;
+	}
 
 	/*
 	 * If the DACs are ok we don't need any capability check.
@@ -189,6 +203,7 @@
 	if (((mode & mask & (MAY_READ|MAY_WRITE|MAY_EXEC)) == mask))
 		return 0;
 
+ check_capabilities:
 	/*
 	 * Read/write DACs are always overridable.
 	 * Executable DACs are overridable if at least one exec bit is set.
@@ -219,7 +234,7 @@
 	if (inode->i_op && inode->i_op->permission)
 		retval = inode->i_op->permission(inode, submask, nd);
 	else
-		retval = vfs_permission(inode, submask);
+		retval = generic_permission(inode, submask, NULL);
 	if (retval)
 		return retval;
 
@@ -314,7 +329,7 @@
 /*
  * Short-cut version of permission(), for calling by
  * path_walk(), when dcache lock is held.  Combines parts
- * of permission() and vfs_permission(), and tests ONLY for
+ * of permission() and generic_permission(), and tests ONLY for
  * MAY_EXEC permission.
  *
  * If appropriate, check DAC only.  If not appropriate, or
@@ -2405,7 +2420,7 @@
 EXPORT_SYMBOL(vfs_link);
 EXPORT_SYMBOL(vfs_mkdir);
 EXPORT_SYMBOL(vfs_mknod);
-EXPORT_SYMBOL(vfs_permission);
+EXPORT_SYMBOL(generic_permission);
 EXPORT_SYMBOL(vfs_readlink);
 EXPORT_SYMBOL(vfs_rename);
 EXPORT_SYMBOL(vfs_rmdir);
--- 1.97/fs/cifs/CHANGES	2004-08-27 06:51:14 +02:00
+++ edited/fs/cifs/CHANGES	2004-09-01 14:02:16 +02:00
@@ -7,8 +7,8 @@
 
 Version 1.21
 ------------
-Add new mount parm to control whether mode check (vfs_permission) is done on
-the client.  If Unix extensions are enabled and the uids on the client
+Add new mount parm to control whether mode check (generic_permission) is done
+on the client.  If Unix extensions are enabled and the uids on the client
 and server do not match, client permission checks are meaningless on
 server uids that do not exist on the client (this does not affect the
 normal ACL check which occurs on the server).  Fix default uid
--- 1.68/fs/cifs/cifsfs.c	2004-09-01 01:10:31 +02:00
+++ edited/fs/cifs/cifsfs.c	2004-09-01 13:57:31 +02:00
@@ -200,7 +200,7 @@
 		on the client (above and beyond ACL on servers) for  
 		servers which do not support setting and viewing mode bits,
 		so allowing client to check permissions is useful */ 
-		return vfs_permission(inode, mask);
+		return generic_permission(inode, mask, NULL);
 }
 
 static kmem_cache_t *cifs_inode_cachep;
--- 1.12/fs/ext2/acl.c	2004-07-12 10:00:54 +02:00
+++ edited/fs/ext2/acl.c	2004-09-01 14:09:16 +02:00
@@ -280,60 +280,24 @@
 	return error;
 }
 
-/*
- * Inode operation permission().
- *
- * inode->i_sem: don't care
- */
-int
-ext2_permission(struct inode *inode, int mask, struct nameidata *nd)
+static int
+ext2_check_acl(struct inode *inode, int mask)
 {
-	int mode = inode->i_mode;
+	struct posix_acl *acl = ext2_get_acl(inode, ACL_TYPE_ACCESS);
 
-	/* Nobody gets write access to a read-only fs */
-	if ((mask & MAY_WRITE) && IS_RDONLY(inode) &&
-	    (S_ISREG(mode) || S_ISDIR(mode) || S_ISLNK(mode)))
-		return -EROFS;
-	/* Nobody gets write access to an immutable file */
-	if ((mask & MAY_WRITE) && IS_IMMUTABLE(inode))
-	    return -EACCES;
-	if (current->fsuid == inode->i_uid) {
-		mode >>= 6;
-	} else if (test_opt(inode->i_sb, POSIX_ACL)) {
-		struct posix_acl *acl;
-
-		/* The access ACL cannot grant access if the group class
-		   permission bits don't contain all requested permissions. */
-		if (((mode >> 3) & mask & S_IRWXO) != mask)
-			goto check_groups;
-		acl = ext2_get_acl(inode, ACL_TYPE_ACCESS);
-		if (acl) {
-			int error = posix_acl_permission(inode, acl, mask);
-			posix_acl_release(acl);
-			if (error == -EACCES)
-				goto check_capabilities;
-			return error;
-		} else
-			goto check_groups;
-	} else {
-check_groups:
-		if (in_group_p(inode->i_gid))
-			mode >>= 3;
+	if (acl) {
+		int error = posix_acl_permission(inode, acl, mask);
+		posix_acl_release(acl);
+		return error;
 	}
-	if ((mode & mask & S_IRWXO) == mask)
-		return 0;
 
-check_capabilities:
-	/* Allowed to override Discretionary Access Control? */
-	if (!(mask & MAY_EXEC) ||
-	    (inode->i_mode & S_IXUGO) || S_ISDIR(inode->i_mode))
-		if (capable(CAP_DAC_OVERRIDE))
-			return 0;
-	/* Read and search granted if capable(CAP_DAC_READ_SEARCH) */
-	if (capable(CAP_DAC_READ_SEARCH) && ((mask == MAY_READ) ||
-	    (S_ISDIR(inode->i_mode) && !(mask & MAY_WRITE))))
-		return 0;
-	return -EACCES;
+	return -EAGAIN;
+}
+
+int
+ext2_permission(struct inode *inode, int mask, struct nameidata *nd)
+{
+	return generic_permission(inode, mask, ext2_check_acl);
 }
 
 /*
--- 1.18/fs/ext3/acl.c	2004-07-12 10:00:54 +02:00
+++ edited/fs/ext3/acl.c	2004-09-01 14:09:13 +02:00
@@ -285,60 +285,24 @@
 	return error;
 }
 
-/*
- * Inode operation permission().
- *
- * inode->i_sem: don't care
- */
-int
-ext3_permission(struct inode *inode, int mask, struct nameidata *nd)
+static int
+ext3_check_acl(struct inode *inode, int mask)
 {
-	int mode = inode->i_mode;
+	struct posix_acl *acl = ext3_get_acl(inode, ACL_TYPE_ACCESS);
 
-	/* Nobody gets write access to a read-only fs */
-	if ((mask & MAY_WRITE) && IS_RDONLY(inode) &&
-	    (S_ISREG(mode) || S_ISDIR(mode) || S_ISLNK(mode)))
-		return -EROFS;
-	/* Nobody gets write access to an immutable file */
-	if ((mask & MAY_WRITE) && IS_IMMUTABLE(inode))
-	    return -EACCES;
-	if (current->fsuid == inode->i_uid) {
-		mode >>= 6;
-	} else if (test_opt(inode->i_sb, POSIX_ACL)) {
-		struct posix_acl *acl;
-
-		/* The access ACL cannot grant access if the group class
-		   permission bits don't contain all requested permissions. */
-		if (((mode >> 3) & mask & S_IRWXO) != mask)
-			goto check_groups;
-		acl = ext3_get_acl(inode, ACL_TYPE_ACCESS);
-		if (acl) {
-			int error = posix_acl_permission(inode, acl, mask);
-			posix_acl_release(acl);
-			if (error == -EACCES)
-				goto check_capabilities;
-			return error;
-		} else
-			goto check_groups;
-	} else {
-check_groups:
-		if (in_group_p(inode->i_gid))
-			mode >>= 3;
+	if (acl) {
+		int error = posix_acl_permission(inode, acl, mask);
+		posix_acl_release(acl);
+		return error;
 	}
-	if ((mode & mask & S_IRWXO) == mask)
-		return 0;
 
-check_capabilities:
-	/* Allowed to override Discretionary Access Control? */
-	if (!(mask & MAY_EXEC) ||
-	    (inode->i_mode & S_IXUGO) || S_ISDIR(inode->i_mode))
-		if (capable(CAP_DAC_OVERRIDE))
-			return 0;
-	/* Read and search granted if capable(CAP_DAC_READ_SEARCH) */
-	if (capable(CAP_DAC_READ_SEARCH) && ((mask == MAY_READ) ||
-	    (S_ISDIR(inode->i_mode) && !(mask & MAY_WRITE))))
-		return 0;
-	return -EACCES;
+	return -EAGAIN;
+}
+
+int
+ext3_permission(struct inode *inode, int mask, struct nameidata *nd)
+{
+	return generic_permission(inode, mask, ext3_check_acl);
 }
 
 /*
--- 1.21/fs/fat/dir.c	2003-09-29 03:11:39 +02:00
+++ edited/fs/fat/dir.c	2004-09-01 11:14:11 +02:00
@@ -93,14 +93,6 @@
 }
 #endif
 
-static inline unsigned char
-fat_tolower(struct nls_table *t, unsigned char c)
-{
-	unsigned char nc = t->charset2lower[c];
-
-	return nc ? nc : c;
-}
-
 static inline int
 fat_short2uni(struct nls_table *t, unsigned char *c, int clen, wchar_t *uni)
 {
@@ -140,17 +132,6 @@
 	return charlen;
 }
 
-static int
-fat_strnicmp(struct nls_table *t, const unsigned char *s1,
-					const unsigned char *s2, int len)
-{
-	while(len--)
-		if (fat_tolower(t, *s1++) != fat_tolower(t, *s2++))
-			return 1;
-
-	return 0;
-}
-
 static inline int
 fat_shortname2uni(struct nls_table *nls, unsigned char *buf, int buf_size,
 		  wchar_t *uni_buf, unsigned short opt, int lower)
@@ -311,7 +292,7 @@
 			:uni16_to_x8(bufname, bufuname, uni_xlate, nls_io);
 		if (xlate_len == name_len)
 			if ((!anycase && !memcmp(name, bufname, xlate_len)) ||
-			    (anycase && !fat_strnicmp(nls_io, name, bufname,
+			    (anycase && !nls_strnicmp(nls_io, name, bufname,
 								xlate_len)))
 				goto Found;
 
@@ -322,7 +303,7 @@
 			if (xlate_len != name_len)
 				continue;
 			if ((!anycase && !memcmp(name, bufname, xlate_len)) ||
-			    (anycase && !fat_strnicmp(nls_io, name, bufname,
+			    (anycase && !nls_strnicmp(nls_io, name, bufname,
 								xlate_len)))
 				goto Found;
 		}
--- 1.17/fs/hfs/inode.c	2004-08-31 10:09:38 +02:00
+++ edited/fs/hfs/inode.c	2004-09-01 13:57:44 +02:00
@@ -516,7 +516,7 @@
 {
 	if (S_ISREG(inode->i_mode) && mask & MAY_EXEC)
 		return 0;
-	return vfs_permission(inode, mask);
+	return generic_permission(inode, mask, NULL);
 }
 
 static int hfs_file_open(struct inode *inode, struct file *file)
--- 1.6/fs/hfsplus/inode.c	2004-08-31 10:09:38 +02:00
+++ edited/fs/hfsplus/inode.c	2004-09-01 13:59:54 +02:00
@@ -260,7 +260,7 @@
 	 */
 	if (S_ISREG(inode->i_mode) && mask & MAY_EXEC && !(inode->i_mode & 0111))
 		return 0;
-	return vfs_permission(inode, mask);
+	return generic_permission(inode, mask, NULL);
 }
 
 
--- 1.3/fs/hostfs/hostfs_kern.c	2004-08-24 11:08:24 +02:00
+++ edited/fs/hostfs/hostfs_kern.c	2004-09-01 13:58:45 +02:00
@@ -802,7 +802,7 @@
 	if(name == NULL) return(-ENOMEM);
 	err = access_file(name, r, w, x);
 	kfree(name);
-	if(!err) err = vfs_permission(ino, desired);
+	if(!err) err = generic_permission(ino, desired, NULL);
 	return(err);
 }
 
--- 1.7/fs/jfs/acl.c	2004-08-18 21:32:19 +02:00
+++ edited/fs/jfs/acl.c	2004-09-01 14:17:33 +02:00
@@ -122,89 +122,26 @@
 	}
 	return rc;
 }
-
-/*
- *	jfs_permission()
- *
- * modified vfs_permission to check posix acl
- */
-int jfs_permission(struct inode * inode, int mask, struct nameidata *nd)
+	
+static int jfs_check_acl(struct inode *inode, int mask)
 {
-	umode_t mode = inode->i_mode;
 	struct jfs_inode_info *ji = JFS_IP(inode);
 
-	if (mask & MAY_WRITE) {
-		/*
-		 * Nobody gets write access to a read-only fs.
-		 */
-		if (IS_RDONLY(inode) &&
-		    (S_ISREG(mode) || S_ISDIR(mode) || S_ISLNK(mode)))
-			return -EROFS;
-
-		/*
-		 * Nobody gets write access to an immutable file.
-		 */
-		if (IS_IMMUTABLE(inode))
-			return -EACCES;
-	}
-
-	if (current->fsuid == inode->i_uid) {
-		mode >>= 6;
-		goto check_mode;
-	}
-	/*
-	 * ACL can't contain additional permissions if the ACL_MASK entry
-	 * is zero.
-	 */
-	if (!(mode & S_IRWXG))
-		goto check_groups;
-
 	if (ji->i_acl == JFS_ACL_NOT_CACHED) {
-		struct posix_acl *acl;
-
-		acl = jfs_get_acl(inode, ACL_TYPE_ACCESS);
-
+		struct posix_acl *acl = jfs_get_acl(inode, ACL_TYPE_ACCESS);
 		if (IS_ERR(acl))
 			return PTR_ERR(acl);
 		posix_acl_release(acl);
 	}
 
-	if (ji->i_acl) {
-		int rc = posix_acl_permission(inode, ji->i_acl, mask);
-		if (rc == -EACCES)
-			goto check_capabilities;
-		return rc;
-	}
-
-check_groups:
-	if (in_group_p(inode->i_gid))
-		mode >>= 3;
-
-check_mode:
-	/*
-	 * If the DACs are ok we don't need any capability check.
-	 */
-	if (((mode & mask & (MAY_READ|MAY_WRITE|MAY_EXEC)) == mask))
-		return 0;
+	if (ji->i_acl)
+		return posix_acl_permission(inode, ji->i_acl, mask);
+	return -EAGAIN;
+}
 
-check_capabilities:
-	/*
-	 * Read/write DACs are always overridable.
-	 * Executable DACs are overridable if at least one exec bit is set.
-	 */
-	if (!(mask & MAY_EXEC) ||
-	    (inode->i_mode & S_IXUGO) || S_ISDIR(inode->i_mode))
-		if (capable(CAP_DAC_OVERRIDE))
-			return 0;
-
-	/*
-	 * Searching includes executable on directories, else just read.
-	 */
-	if (mask == MAY_READ || (S_ISDIR(inode->i_mode) && !(mask & MAY_WRITE)))
-		if (capable(CAP_DAC_READ_SEARCH))
-			return 0;
-
-	return -EACCES;
+int jfs_permission(struct inode *inode, int mask, struct nameidata *nd)
+{
+	return generic_permission(inode, mask, jfs_check_acl);
 }
 
 int jfs_init_acl(struct inode *inode, struct inode *dir)
--- 1.78/fs/nfs/dir.c	2004-06-15 06:01:27 +02:00
+++ edited/fs/nfs/dir.c	2004-09-01 13:58:19 +02:00
@@ -1589,7 +1589,7 @@
 	return res;
 out_notsup:
 	nfs_revalidate_inode(NFS_SERVER(inode), inode);
-	res = vfs_permission(inode, mask);
+	res = generic_permission(inode, mask, NULL);
 	unlock_kernel();
 	return res;
 }
--- 1.77/fs/proc/base.c	2004-08-27 08:31:38 +02:00
+++ edited/fs/proc/base.c	2004-09-01 13:58:31 +02:00
@@ -471,7 +471,7 @@
 
 static int proc_permission(struct inode *inode, int mask, struct nameidata *nd)
 {
-	if (vfs_permission(inode, mask) != 0)
+	if (generic_permission(inode, mask, NULL) != 0)
 		return -EACCES;
 	return proc_check_root(inode);
 }
===== fs/reiserfs/xattr.c 1.9 vs edited =====
--- 1.9/fs/reiserfs/xattr.c	2004-08-24 11:08:56 +02:00
+++ edited/fs/reiserfs/xattr.c	2004-09-01 14:24:53 +02:00
@@ -1339,110 +1339,63 @@
     return err;
 }
 
+#ifdef CONFIG_REISERFS_FS_POSIX_ACL
 static int
-__reiserfs_permission (struct inode *inode, int mask, struct nameidata *nd,
-                       int need_lock)
+__reiserfs_check_acl(struct inode *inode, int mask, int need_lock)
 {
-	umode_t			mode = inode->i_mode;
+	struct posix_acl *acl;
 
-	if (mask & MAY_WRITE) {
-		/*
-		 * Nobody gets write access to a read-only fs.
-		 */
-		if (IS_RDONLY(inode) &&
-		    (S_ISREG(mode) || S_ISDIR(mode) || S_ISLNK(mode)))
-			return -EROFS;
-
-		/*
-		 * Nobody gets write access to an immutable file.
-		 */
-		if (IS_IMMUTABLE(inode))
-			return -EACCES;
-	}
+	if (get_inode_sd_version (inode) != STAT_DATA_V1)
+		return -EAGAIN;
 
-	/* We don't do permission checks on the internal objects.
-	* Permissions are determined by the "owning" object. */
-        if (is_reiserfs_priv_object (inode))
-		return 0;
+	if (need_lock) {
+		reiserfs_read_lock_xattr_i(inode);
+		reiserfs_read_lock_xattrs(inode->i_sb);
+		acl = reiserfs_get_acl(inode, ACL_TYPE_ACCESS);
+		reiserfs_read_unlock_xattrs(inode->i_sb);
+		reiserfs_read_unlock_xattr_i(inode);
+	} else
+		acl = reiserfs_get_acl (inode, ACL_TYPE_ACCESS);
+
+	if (IS_ERR(acl)) {
+		if (PTR_ERR(acl) == -ENODATA)
+			return -EAGAIN;
+		return PTR_ERR(acl);
+	}
 
-	if (current->fsuid == inode->i_uid) {
-		mode >>= 6;
-#ifdef CONFIG_REISERFS_FS_POSIX_ACL
-	} else if (reiserfs_posixacl(inode->i_sb) &&
-                   get_inode_sd_version (inode) != STAT_DATA_V1) {
-                struct posix_acl *acl;
-
-		/* ACL can't contain additional permissions if
-		   the ACL_MASK entry is 0 */
-		if (!(mode & S_IRWXG))
-			goto check_groups;
-
-                if (need_lock) {
-		    reiserfs_read_lock_xattr_i (inode);
-                    reiserfs_read_lock_xattrs (inode->i_sb);
-		}
-                acl = reiserfs_get_acl (inode, ACL_TYPE_ACCESS);
-                if (need_lock) {
-                    reiserfs_read_unlock_xattrs (inode->i_sb);
-		    reiserfs_read_unlock_xattr_i (inode);
-		}
-                if (IS_ERR (acl)) {
-                    if (PTR_ERR (acl) == -ENODATA)
-                        goto check_groups;
-                    return PTR_ERR (acl);
-                }
-
-                if (acl) {
-                    int err = posix_acl_permission (inode, acl, mask);
-                    posix_acl_release (acl);
-                    if (err == -EACCES) {
-                        goto check_capabilities;
-                    }
-                    return err;
-		} else {
-			goto check_groups;
-                }
-#endif
-	} else {
-check_groups:
-		if (in_group_p(inode->i_gid))
-			mode >>= 3;
+	if (acl) {
+		int error = posix_acl_permission(inode, acl, mask);
+		posix_acl_release(acl);
+		return error;
 	}
 
-	/*
-	 * If the DACs are ok we don't need any capability check.
-	 */
-	if (((mode & mask & (MAY_READ|MAY_WRITE|MAY_EXEC)) == mask))
-		return 0;
-
-check_capabilities:
-	/*
-	 * Read/write DACs are always overridable.
-	 * Executable DACs are overridable if at least one exec bit is set.
-	 */
-	if (!(mask & MAY_EXEC) ||
-	    (inode->i_mode & S_IXUGO) || S_ISDIR(inode->i_mode))
-		if (capable(CAP_DAC_OVERRIDE))
-			return 0;
-
-	/*
-	 * Searching includes executable on directories, else just read.
-	 */
-	if (mask == MAY_READ || (S_ISDIR(inode->i_mode) && !(mask & MAY_WRITE)))
-		if (capable(CAP_DAC_READ_SEARCH))
-			return 0;
+	return -EAGAIN;
+}
 
-	return -EACCES;
+static int
+reiserfs_check_acl(struct inode *inode, int mask)
+{
+	return __reiserfs_check_acl(inode, mask, 1);
+}
+
+static int
+reiserfs_check_acl_locked(struct inode *inode, int mask)
+{
+	return __reiserfs_check_acl(inode, mask, 0);
 }
+#else
+# define reiserfs_check_acl		NULL
+# define reiserfs_check_acl_locked	NULL
+#endif
 
 int
-reiserfs_permission (struct inode *inode, int mask, struct nameidata *nd)
+reiserfs_permission(struct inode *inode, int mask, struct nameidata *nd)
 {
-    return __reiserfs_permission (inode, mask, nd, 1);
+	return generic_permission(inode, mask, reiserfs_check_acl);
 }
 
 int
-reiserfs_permission_locked (struct inode *inode, int mask, struct nameidata *nd)
+reiserfs_permission_locked(struct inode *inode, int mask, struct nameidata *nd)
 {
-    return __reiserfs_permission (inode, mask, nd, 0);
+	return generic_permission(inode, mask, reiserfs_check_acl_locked);
 }
--- 1.343/include/linux/fs.h	2004-08-27 08:30:32 +02:00
+++ edited/include/linux/fs.h	2004-09-01 14:51:12 +02:00
@@ -1336,7 +1336,9 @@
 extern int setattr_mask(unsigned int);
 extern int notify_change(struct dentry *, struct iattr *);
 extern int permission(struct inode *, int, struct nameidata *);
-extern int vfs_permission(struct inode *, int);
+extern int generic_permission(struct inode *, int,
+		int (*check_acl)(struct inode *, int));
+
 extern int get_write_access(struct inode *);
 extern int deny_write_access(struct file *);
 static inline void put_write_access(struct inode * inode)

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] generic acl support for ->permission
  2004-09-01 15:24 [PATCH] generic acl support for ->permission Christoph Hellwig
@ 2004-09-01 16:00 ` Christoph Hellwig
  2004-09-01 17:40 ` Dave Kleikamp
  2004-09-01 19:20 ` Andreas Gruenbacher
  2 siblings, 0 replies; 5+ messages in thread
From: Christoph Hellwig @ 2004-09-01 16:00 UTC (permalink / raw)
  To: linux-fsdevel; +Cc: agruen, shaggy, jeffm

On Wed, Sep 01, 2004 at 05:24:39PM +0200, Christoph Hellwig wrote:
> Currently we every filesystem with Posix ACLs has it's own
> reimplemtation of the generic permission checking code with additonal
> ACL support.  This patch
> 
>  - adds an optional callback to vfs_permission that filesystems can use
>    for ACL support (and renames it to generic_permission because the
>    old name was wrong - it wasn't like the other vfs_* functions at all)
>  - uses it in ext2, ext3, jfs and reiserfs.  XFS will follow a little
>    later as it's permission checking is burried under several layers of
>    abtractions.

willy notices this patch had a stale hunk of the nls bits in.  Clean
patch below:

--- 1.132/fs/exec.c	2004-08-27 09:02:34 +02:00
+++ edited/fs/exec.c	2004-09-01 14:02:43 +02:00
@@ -893,7 +893,7 @@
 	mode = inode->i_mode;
 	/*
 	 * Check execute perms again - if the caller has CAP_DAC_OVERRIDE,
-	 * vfs_permission lets a non-executable through
+	 * generic_permission lets a non-executable through
 	 */
 	if (!(mode & 0111))	/* with at least _one_ execute bit set */
 		return -EACCES;
--- 1.107/fs/namei.c	2004-09-01 01:00:48 +02:00
+++ edited/fs/namei.c	2004-09-01 14:42:21 +02:00
@@ -151,15 +151,19 @@
 	return result;
 }
 
-/*
- *	vfs_permission()
+/**
+ * generic_permission  -  check for access rights on a Posix-like filesystem
+ * @inode:	inode to check access rights for
+ * @mask:	right to check for (%MAY_READ, %MAY_WRITE, %MAY_EXEC)
+ * @check_acl:	optional callback to check for Posix ACLs
  *
- * is used to check for read/write/execute permissions on a file.
+ * Used to check for read/write/execute permissions on a file.
  * We use "fsuid" for this, letting us set arbitrary permissions
  * for filesystem access without changing the "normal" uids which
  * are used for other things..
  */
-int vfs_permission(struct inode * inode, int mask)
+int generic_permission(struct inode *inode, int mask,
+		int (*check_acl)(struct inode *inode, int mask))
 {
 	umode_t			mode = inode->i_mode;
 
@@ -180,8 +184,18 @@
 
 	if (current->fsuid == inode->i_uid)
 		mode >>= 6;
-	else if (in_group_p(inode->i_gid))
-		mode >>= 3;
+	else {
+		if (IS_POSIXACL(inode) && (mode & S_IRWXG) && check_acl) {
+			int error = check_acl(inode, mask);
+			if (error == -EACCES)
+				goto check_capabilities;
+			else if (error != -EAGAIN)
+				return error;
+		}
+
+		if (in_group_p(inode->i_gid))
+			mode >>= 3;
+	}
 
 	/*
 	 * If the DACs are ok we don't need any capability check.
@@ -189,6 +203,7 @@
 	if (((mode & mask & (MAY_READ|MAY_WRITE|MAY_EXEC)) == mask))
 		return 0;
 
+ check_capabilities:
 	/*
 	 * Read/write DACs are always overridable.
 	 * Executable DACs are overridable if at least one exec bit is set.
@@ -219,7 +234,7 @@
 	if (inode->i_op && inode->i_op->permission)
 		retval = inode->i_op->permission(inode, submask, nd);
 	else
-		retval = vfs_permission(inode, submask);
+		retval = generic_permission(inode, submask, NULL);
 	if (retval)
 		return retval;
 
@@ -314,7 +329,7 @@
 /*
  * Short-cut version of permission(), for calling by
  * path_walk(), when dcache lock is held.  Combines parts
- * of permission() and vfs_permission(), and tests ONLY for
+ * of permission() and generic_permission(), and tests ONLY for
  * MAY_EXEC permission.
  *
  * If appropriate, check DAC only.  If not appropriate, or
@@ -2405,7 +2420,7 @@
 EXPORT_SYMBOL(vfs_link);
 EXPORT_SYMBOL(vfs_mkdir);
 EXPORT_SYMBOL(vfs_mknod);
-EXPORT_SYMBOL(vfs_permission);
+EXPORT_SYMBOL(generic_permission);
 EXPORT_SYMBOL(vfs_readlink);
 EXPORT_SYMBOL(vfs_rename);
 EXPORT_SYMBOL(vfs_rmdir);
--- 1.97/fs/cifs/CHANGES	2004-08-27 06:51:14 +02:00
+++ edited/fs/cifs/CHANGES	2004-09-01 14:02:16 +02:00
@@ -7,8 +7,8 @@
 
 Version 1.21
 ------------
-Add new mount parm to control whether mode check (vfs_permission) is done on
-the client.  If Unix extensions are enabled and the uids on the client
+Add new mount parm to control whether mode check (generic_permission) is done
+on the client.  If Unix extensions are enabled and the uids on the client
 and server do not match, client permission checks are meaningless on
 server uids that do not exist on the client (this does not affect the
 normal ACL check which occurs on the server).  Fix default uid
--- 1.68/fs/cifs/cifsfs.c	2004-09-01 01:10:31 +02:00
+++ edited/fs/cifs/cifsfs.c	2004-09-01 13:57:31 +02:00
@@ -200,7 +200,7 @@
 		on the client (above and beyond ACL on servers) for  
 		servers which do not support setting and viewing mode bits,
 		so allowing client to check permissions is useful */ 
-		return vfs_permission(inode, mask);
+		return generic_permission(inode, mask, NULL);
 }
 
 static kmem_cache_t *cifs_inode_cachep;
--- 1.12/fs/ext2/acl.c	2004-07-12 10:00:54 +02:00
+++ edited/fs/ext2/acl.c	2004-09-01 14:09:16 +02:00
@@ -280,60 +280,24 @@
 	return error;
 }
 
-/*
- * Inode operation permission().
- *
- * inode->i_sem: don't care
- */
-int
-ext2_permission(struct inode *inode, int mask, struct nameidata *nd)
+static int
+ext2_check_acl(struct inode *inode, int mask)
 {
-	int mode = inode->i_mode;
+	struct posix_acl *acl = ext2_get_acl(inode, ACL_TYPE_ACCESS);
 
-	/* Nobody gets write access to a read-only fs */
-	if ((mask & MAY_WRITE) && IS_RDONLY(inode) &&
-	    (S_ISREG(mode) || S_ISDIR(mode) || S_ISLNK(mode)))
-		return -EROFS;
-	/* Nobody gets write access to an immutable file */
-	if ((mask & MAY_WRITE) && IS_IMMUTABLE(inode))
-	    return -EACCES;
-	if (current->fsuid == inode->i_uid) {
-		mode >>= 6;
-	} else if (test_opt(inode->i_sb, POSIX_ACL)) {
-		struct posix_acl *acl;
-
-		/* The access ACL cannot grant access if the group class
-		   permission bits don't contain all requested permissions. */
-		if (((mode >> 3) & mask & S_IRWXO) != mask)
-			goto check_groups;
-		acl = ext2_get_acl(inode, ACL_TYPE_ACCESS);
-		if (acl) {
-			int error = posix_acl_permission(inode, acl, mask);
-			posix_acl_release(acl);
-			if (error == -EACCES)
-				goto check_capabilities;
-			return error;
-		} else
-			goto check_groups;
-	} else {
-check_groups:
-		if (in_group_p(inode->i_gid))
-			mode >>= 3;
+	if (acl) {
+		int error = posix_acl_permission(inode, acl, mask);
+		posix_acl_release(acl);
+		return error;
 	}
-	if ((mode & mask & S_IRWXO) == mask)
-		return 0;
 
-check_capabilities:
-	/* Allowed to override Discretionary Access Control? */
-	if (!(mask & MAY_EXEC) ||
-	    (inode->i_mode & S_IXUGO) || S_ISDIR(inode->i_mode))
-		if (capable(CAP_DAC_OVERRIDE))
-			return 0;
-	/* Read and search granted if capable(CAP_DAC_READ_SEARCH) */
-	if (capable(CAP_DAC_READ_SEARCH) && ((mask == MAY_READ) ||
-	    (S_ISDIR(inode->i_mode) && !(mask & MAY_WRITE))))
-		return 0;
-	return -EACCES;
+	return -EAGAIN;
+}
+
+int
+ext2_permission(struct inode *inode, int mask, struct nameidata *nd)
+{
+	return generic_permission(inode, mask, ext2_check_acl);
 }
 
 /*
--- 1.18/fs/ext3/acl.c	2004-07-12 10:00:54 +02:00
+++ edited/fs/ext3/acl.c	2004-09-01 14:09:13 +02:00
@@ -285,60 +285,24 @@
 	return error;
 }
 
-/*
- * Inode operation permission().
- *
- * inode->i_sem: don't care
- */
-int
-ext3_permission(struct inode *inode, int mask, struct nameidata *nd)
+static int
+ext3_check_acl(struct inode *inode, int mask)
 {
-	int mode = inode->i_mode;
+	struct posix_acl *acl = ext3_get_acl(inode, ACL_TYPE_ACCESS);
 
-	/* Nobody gets write access to a read-only fs */
-	if ((mask & MAY_WRITE) && IS_RDONLY(inode) &&
-	    (S_ISREG(mode) || S_ISDIR(mode) || S_ISLNK(mode)))
-		return -EROFS;
-	/* Nobody gets write access to an immutable file */
-	if ((mask & MAY_WRITE) && IS_IMMUTABLE(inode))
-	    return -EACCES;
-	if (current->fsuid == inode->i_uid) {
-		mode >>= 6;
-	} else if (test_opt(inode->i_sb, POSIX_ACL)) {
-		struct posix_acl *acl;
-
-		/* The access ACL cannot grant access if the group class
-		   permission bits don't contain all requested permissions. */
-		if (((mode >> 3) & mask & S_IRWXO) != mask)
-			goto check_groups;
-		acl = ext3_get_acl(inode, ACL_TYPE_ACCESS);
-		if (acl) {
-			int error = posix_acl_permission(inode, acl, mask);
-			posix_acl_release(acl);
-			if (error == -EACCES)
-				goto check_capabilities;
-			return error;
-		} else
-			goto check_groups;
-	} else {
-check_groups:
-		if (in_group_p(inode->i_gid))
-			mode >>= 3;
+	if (acl) {
+		int error = posix_acl_permission(inode, acl, mask);
+		posix_acl_release(acl);
+		return error;
 	}
-	if ((mode & mask & S_IRWXO) == mask)
-		return 0;
 
-check_capabilities:
-	/* Allowed to override Discretionary Access Control? */
-	if (!(mask & MAY_EXEC) ||
-	    (inode->i_mode & S_IXUGO) || S_ISDIR(inode->i_mode))
-		if (capable(CAP_DAC_OVERRIDE))
-			return 0;
-	/* Read and search granted if capable(CAP_DAC_READ_SEARCH) */
-	if (capable(CAP_DAC_READ_SEARCH) && ((mask == MAY_READ) ||
-	    (S_ISDIR(inode->i_mode) && !(mask & MAY_WRITE))))
-		return 0;
-	return -EACCES;
+	return -EAGAIN;
+}
+
+int
+ext3_permission(struct inode *inode, int mask, struct nameidata *nd)
+{
+	return generic_permission(inode, mask, ext3_check_acl);
 }
 
 /*
--- 1.17/fs/hfs/inode.c	2004-08-31 10:09:38 +02:00
+++ edited/fs/hfs/inode.c	2004-09-01 13:57:44 +02:00
@@ -516,7 +516,7 @@
 {
 	if (S_ISREG(inode->i_mode) && mask & MAY_EXEC)
 		return 0;
-	return vfs_permission(inode, mask);
+	return generic_permission(inode, mask, NULL);
 }
 
 static int hfs_file_open(struct inode *inode, struct file *file)
--- 1.6/fs/hfsplus/inode.c	2004-08-31 10:09:38 +02:00
+++ edited/fs/hfsplus/inode.c	2004-09-01 13:59:54 +02:00
@@ -260,7 +260,7 @@
 	 */
 	if (S_ISREG(inode->i_mode) && mask & MAY_EXEC && !(inode->i_mode & 0111))
 		return 0;
-	return vfs_permission(inode, mask);
+	return generic_permission(inode, mask, NULL);
 }
 
 
--- 1.3/fs/hostfs/hostfs_kern.c	2004-08-24 11:08:24 +02:00
+++ edited/fs/hostfs/hostfs_kern.c	2004-09-01 13:58:45 +02:00
@@ -802,7 +802,7 @@
 	if(name == NULL) return(-ENOMEM);
 	err = access_file(name, r, w, x);
 	kfree(name);
-	if(!err) err = vfs_permission(ino, desired);
+	if(!err) err = generic_permission(ino, desired, NULL);
 	return(err);
 }
 
--- 1.7/fs/jfs/acl.c	2004-08-18 21:32:19 +02:00
+++ edited/fs/jfs/acl.c	2004-09-01 14:17:33 +02:00
@@ -122,89 +122,26 @@
 	}
 	return rc;
 }
-
-/*
- *	jfs_permission()
- *
- * modified vfs_permission to check posix acl
- */
-int jfs_permission(struct inode * inode, int mask, struct nameidata *nd)
+	
+static int jfs_check_acl(struct inode *inode, int mask)
 {
-	umode_t mode = inode->i_mode;
 	struct jfs_inode_info *ji = JFS_IP(inode);
 
-	if (mask & MAY_WRITE) {
-		/*
-		 * Nobody gets write access to a read-only fs.
-		 */
-		if (IS_RDONLY(inode) &&
-		    (S_ISREG(mode) || S_ISDIR(mode) || S_ISLNK(mode)))
-			return -EROFS;
-
-		/*
-		 * Nobody gets write access to an immutable file.
-		 */
-		if (IS_IMMUTABLE(inode))
-			return -EACCES;
-	}
-
-	if (current->fsuid == inode->i_uid) {
-		mode >>= 6;
-		goto check_mode;
-	}
-	/*
-	 * ACL can't contain additional permissions if the ACL_MASK entry
-	 * is zero.
-	 */
-	if (!(mode & S_IRWXG))
-		goto check_groups;
-
 	if (ji->i_acl == JFS_ACL_NOT_CACHED) {
-		struct posix_acl *acl;
-
-		acl = jfs_get_acl(inode, ACL_TYPE_ACCESS);
-
+		struct posix_acl *acl = jfs_get_acl(inode, ACL_TYPE_ACCESS);
 		if (IS_ERR(acl))
 			return PTR_ERR(acl);
 		posix_acl_release(acl);
 	}
 
-	if (ji->i_acl) {
-		int rc = posix_acl_permission(inode, ji->i_acl, mask);
-		if (rc == -EACCES)
-			goto check_capabilities;
-		return rc;
-	}
-
-check_groups:
-	if (in_group_p(inode->i_gid))
-		mode >>= 3;
-
-check_mode:
-	/*
-	 * If the DACs are ok we don't need any capability check.
-	 */
-	if (((mode & mask & (MAY_READ|MAY_WRITE|MAY_EXEC)) == mask))
-		return 0;
+	if (ji->i_acl)
+		return posix_acl_permission(inode, ji->i_acl, mask);
+	return -EAGAIN;
+}
 
-check_capabilities:
-	/*
-	 * Read/write DACs are always overridable.
-	 * Executable DACs are overridable if at least one exec bit is set.
-	 */
-	if (!(mask & MAY_EXEC) ||
-	    (inode->i_mode & S_IXUGO) || S_ISDIR(inode->i_mode))
-		if (capable(CAP_DAC_OVERRIDE))
-			return 0;
-
-	/*
-	 * Searching includes executable on directories, else just read.
-	 */
-	if (mask == MAY_READ || (S_ISDIR(inode->i_mode) && !(mask & MAY_WRITE)))
-		if (capable(CAP_DAC_READ_SEARCH))
-			return 0;
-
-	return -EACCES;
+int jfs_permission(struct inode *inode, int mask, struct nameidata *nd)
+{
+	return generic_permission(inode, mask, jfs_check_acl);
 }
 
 int jfs_init_acl(struct inode *inode, struct inode *dir)
--- 1.78/fs/nfs/dir.c	2004-06-15 06:01:27 +02:00
+++ edited/fs/nfs/dir.c	2004-09-01 13:58:19 +02:00
@@ -1589,7 +1589,7 @@
 	return res;
 out_notsup:
 	nfs_revalidate_inode(NFS_SERVER(inode), inode);
-	res = vfs_permission(inode, mask);
+	res = generic_permission(inode, mask, NULL);
 	unlock_kernel();
 	return res;
 }
--- 1.77/fs/proc/base.c	2004-08-27 08:31:38 +02:00
+++ edited/fs/proc/base.c	2004-09-01 13:58:31 +02:00
@@ -471,7 +471,7 @@
 
 static int proc_permission(struct inode *inode, int mask, struct nameidata *nd)
 {
-	if (vfs_permission(inode, mask) != 0)
+	if (generic_permission(inode, mask, NULL) != 0)
 		return -EACCES;
 	return proc_check_root(inode);
 }
===== fs/reiserfs/xattr.c 1.9 vs edited =====
--- 1.9/fs/reiserfs/xattr.c	2004-08-24 11:08:56 +02:00
+++ edited/fs/reiserfs/xattr.c	2004-09-01 14:24:53 +02:00
@@ -1339,110 +1339,63 @@
     return err;
 }
 
+#ifdef CONFIG_REISERFS_FS_POSIX_ACL
 static int
-__reiserfs_permission (struct inode *inode, int mask, struct nameidata *nd,
-                       int need_lock)
+__reiserfs_check_acl(struct inode *inode, int mask, int need_lock)
 {
-	umode_t			mode = inode->i_mode;
+	struct posix_acl *acl;
 
-	if (mask & MAY_WRITE) {
-		/*
-		 * Nobody gets write access to a read-only fs.
-		 */
-		if (IS_RDONLY(inode) &&
-		    (S_ISREG(mode) || S_ISDIR(mode) || S_ISLNK(mode)))
-			return -EROFS;
-
-		/*
-		 * Nobody gets write access to an immutable file.
-		 */
-		if (IS_IMMUTABLE(inode))
-			return -EACCES;
-	}
+	if (get_inode_sd_version (inode) != STAT_DATA_V1)
+		return -EAGAIN;
 
-	/* We don't do permission checks on the internal objects.
-	* Permissions are determined by the "owning" object. */
-        if (is_reiserfs_priv_object (inode))
-		return 0;
+	if (need_lock) {
+		reiserfs_read_lock_xattr_i(inode);
+		reiserfs_read_lock_xattrs(inode->i_sb);
+		acl = reiserfs_get_acl(inode, ACL_TYPE_ACCESS);
+		reiserfs_read_unlock_xattrs(inode->i_sb);
+		reiserfs_read_unlock_xattr_i(inode);
+	} else
+		acl = reiserfs_get_acl (inode, ACL_TYPE_ACCESS);
+
+	if (IS_ERR(acl)) {
+		if (PTR_ERR(acl) == -ENODATA)
+			return -EAGAIN;
+		return PTR_ERR(acl);
+	}
 
-	if (current->fsuid == inode->i_uid) {
-		mode >>= 6;
-#ifdef CONFIG_REISERFS_FS_POSIX_ACL
-	} else if (reiserfs_posixacl(inode->i_sb) &&
-                   get_inode_sd_version (inode) != STAT_DATA_V1) {
-                struct posix_acl *acl;
-
-		/* ACL can't contain additional permissions if
-		   the ACL_MASK entry is 0 */
-		if (!(mode & S_IRWXG))
-			goto check_groups;
-
-                if (need_lock) {
-		    reiserfs_read_lock_xattr_i (inode);
-                    reiserfs_read_lock_xattrs (inode->i_sb);
-		}
-                acl = reiserfs_get_acl (inode, ACL_TYPE_ACCESS);
-                if (need_lock) {
-                    reiserfs_read_unlock_xattrs (inode->i_sb);
-		    reiserfs_read_unlock_xattr_i (inode);
-		}
-                if (IS_ERR (acl)) {
-                    if (PTR_ERR (acl) == -ENODATA)
-                        goto check_groups;
-                    return PTR_ERR (acl);
-                }
-
-                if (acl) {
-                    int err = posix_acl_permission (inode, acl, mask);
-                    posix_acl_release (acl);
-                    if (err == -EACCES) {
-                        goto check_capabilities;
-                    }
-                    return err;
-		} else {
-			goto check_groups;
-                }
-#endif
-	} else {
-check_groups:
-		if (in_group_p(inode->i_gid))
-			mode >>= 3;
+	if (acl) {
+		int error = posix_acl_permission(inode, acl, mask);
+		posix_acl_release(acl);
+		return error;
 	}
 
-	/*
-	 * If the DACs are ok we don't need any capability check.
-	 */
-	if (((mode & mask & (MAY_READ|MAY_WRITE|MAY_EXEC)) == mask))
-		return 0;
-
-check_capabilities:
-	/*
-	 * Read/write DACs are always overridable.
-	 * Executable DACs are overridable if at least one exec bit is set.
-	 */
-	if (!(mask & MAY_EXEC) ||
-	    (inode->i_mode & S_IXUGO) || S_ISDIR(inode->i_mode))
-		if (capable(CAP_DAC_OVERRIDE))
-			return 0;
-
-	/*
-	 * Searching includes executable on directories, else just read.
-	 */
-	if (mask == MAY_READ || (S_ISDIR(inode->i_mode) && !(mask & MAY_WRITE)))
-		if (capable(CAP_DAC_READ_SEARCH))
-			return 0;
+	return -EAGAIN;
+}
 
-	return -EACCES;
+static int
+reiserfs_check_acl(struct inode *inode, int mask)
+{
+	return __reiserfs_check_acl(inode, mask, 1);
+}
+
+static int
+reiserfs_check_acl_locked(struct inode *inode, int mask)
+{
+	return __reiserfs_check_acl(inode, mask, 0);
 }
+#else
+# define reiserfs_check_acl		NULL
+# define reiserfs_check_acl_locked	NULL
+#endif
 
 int
-reiserfs_permission (struct inode *inode, int mask, struct nameidata *nd)
+reiserfs_permission(struct inode *inode, int mask, struct nameidata *nd)
 {
-    return __reiserfs_permission (inode, mask, nd, 1);
+	return generic_permission(inode, mask, reiserfs_check_acl);
 }
 
 int
-reiserfs_permission_locked (struct inode *inode, int mask, struct nameidata *nd)
+reiserfs_permission_locked(struct inode *inode, int mask, struct nameidata *nd)
 {
-    return __reiserfs_permission (inode, mask, nd, 0);
+	return generic_permission(inode, mask, reiserfs_check_acl_locked);
 }
--- 1.343/include/linux/fs.h	2004-08-27 08:30:32 +02:00
+++ edited/include/linux/fs.h	2004-09-01 14:51:12 +02:00
@@ -1336,7 +1336,9 @@
 extern int setattr_mask(unsigned int);
 extern int notify_change(struct dentry *, struct iattr *);
 extern int permission(struct inode *, int, struct nameidata *);
-extern int vfs_permission(struct inode *, int);
+extern int generic_permission(struct inode *, int,
+		int (*check_acl)(struct inode *, int));
+
 extern int get_write_access(struct inode *);
 extern int deny_write_access(struct file *);
 static inline void put_write_access(struct inode * inode)

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] generic acl support for ->permission
  2004-09-01 15:24 [PATCH] generic acl support for ->permission Christoph Hellwig
  2004-09-01 16:00 ` Christoph Hellwig
@ 2004-09-01 17:40 ` Dave Kleikamp
  2004-09-01 19:20 ` Andreas Gruenbacher
  2 siblings, 0 replies; 5+ messages in thread
From: Dave Kleikamp @ 2004-09-01 17:40 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: fsdevel, agruen, jeffm

On Wed, 2004-09-01 at 10:24, Christoph Hellwig wrote:
> @@ -180,8 +184,18 @@
>  
>  	if (current->fsuid == inode->i_uid)
>  		mode >>= 6;
> -	else if (in_group_p(inode->i_gid))
> -		mode >>= 3;
> +	else {
> +		if (IS_POSIXACL(inode) && (mode & S_IRWXG) && check_acl) {

jfs doesn't currently set MS_POSIXACL (it doesn't require the acl mount
option), so this test would fail here.  The patch below will set it.

> +			int error = check_acl(inode, mask);
> +			if (error == -EACCES)
> +				goto check_capabilities;
> +			else if (error != -EAGAIN)
> +				return error;
> +		}
> +
> +		if (in_group_p(inode->i_gid))
> +			mode >>= 3;
> +	}
>  
>  	/*
>  	 * If the DACs are ok we don't need any capability check.

===== super.c 1.55 vs edited =====
--- 1.55/fs/jfs/super.c	2004-08-24 04:08:54 -05:00
+++ edited/super.c	2004-09-01 12:39:42 -05:00
@@ -403,6 +403,10 @@
 	}
 	sbi->flag = flag;
 
+#ifdef CONFIG_JFS_POSIX_ACL
+	sb->s_flags |= MS_POSIXACL;
+#endif
+
 	if (newLVSize) {
 		printk(KERN_ERR "resize option for remount only\n");
 		return -EINVAL;



^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] generic acl support for ->permission
  2004-09-01 15:24 [PATCH] generic acl support for ->permission Christoph Hellwig
  2004-09-01 16:00 ` Christoph Hellwig
  2004-09-01 17:40 ` Dave Kleikamp
@ 2004-09-01 19:20 ` Andreas Gruenbacher
  2004-09-04 10:46   ` Christoph Hellwig
  2 siblings, 1 reply; 5+ messages in thread
From: Andreas Gruenbacher @ 2004-09-01 19:20 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: linux-fsdevel, shaggy, jeffm

[-- Attachment #1: Type: text/plain, Size: 417 bytes --]

Hello Christoph,

as far as I can see what you propose is very similar to the approach taken in 
the attached patch, with one more indirection. This looks nicer to me, at the 
cost of "polluting" iops instead of vfs_permission. JFS will still need 
Dave's patch, obviously.

The attached patch only covers ext3; not sure if it even compiles.

Cheers,
-- 
Andreas Gruenbacher <agruen@suse.de>
SUSE Labs, SUSE LINUX AG

[-- Attachment #2: other.diff --]
[-- Type: text/x-diff, Size: 6044 bytes --]

Index: linux-2.6.9-rc1-bk8/fs/ext3/file.c
===================================================================
--- linux-2.6.9-rc1-bk8.orig/fs/ext3/file.c
+++ linux-2.6.9-rc1-bk8/fs/ext3/file.c
@@ -136,6 +136,6 @@ struct inode_operations ext3_file_inode_
 	.getxattr	= ext3_getxattr,
 	.listxattr	= ext3_listxattr,
 	.removexattr	= ext3_removexattr,
-	.permission	= ext3_permission,
+	.check_posix_acl = ext3_check_posix_acl,
 };
 
Index: linux-2.6.9-rc1-bk8/fs/ext3/acl.h
===================================================================
--- linux-2.6.9-rc1-bk8.orig/fs/ext3/acl.h
+++ linux-2.6.9-rc1-bk8/fs/ext3/acl.h
@@ -59,7 +59,7 @@ static inline int ext3_acl_count(size_t 
 #define EXT3_ACL_NOT_CACHED ((void *)-1)
 
 /* acl.c */
-extern int ext3_permission (struct inode *, int, struct nameidata *);
+extern int ext3_check_posix_acl (struct inode *, int);
 extern int ext3_acl_chmod (struct inode *);
 extern int ext3_init_acl (handle_t *, struct inode *, struct inode *);
 
@@ -68,7 +68,7 @@ extern void exit_ext3_acl(void);
 
 #else  /* CONFIG_EXT3_FS_POSIX_ACL */
 #include <linux/sched.h>
-#define ext3_permission NULL
+#define ext3_check_posix_acl NULL
 
 static inline int
 ext3_acl_chmod(struct inode *inode)
Index: linux-2.6.9-rc1-bk8/fs/namei.c
===================================================================
--- linux-2.6.9-rc1-bk8.orig/fs/namei.c
+++ linux-2.6.9-rc1-bk8/fs/namei.c
@@ -180,8 +180,19 @@ int vfs_permission(struct inode * inode,
 
 	if (current->fsuid == inode->i_uid)
 		mode >>= 6;
-	else if (in_group_p(inode->i_gid))
-		mode >>= 3;
+	else {
+		if (IS_POSIXACL(inode) && (mode & S_IRWXG) &&
+		    inode->i_op->check_posix_acl) {
+			int error = inode->i_op->check_posix_acl(inode, mask);
+			if (error == -EACCES)
+				goto check_capabilities;
+			else if (error != -EAGAIN)
+				return error;
+		}
+		
+		if (in_group_p(inode->i_gid))
+			mode >>= 3;
+	}
 
 	/*
 	 * If the DACs are ok we don't need any capability check.
@@ -189,6 +200,7 @@ int vfs_permission(struct inode * inode,
 	if (((mode & mask & (MAY_READ|MAY_WRITE|MAY_EXEC)) == mask))
 		return 0;
 
+check_capabilities:
 	/*
 	 * Read/write DACs are always overridable.
 	 * Executable DACs are overridable if at least one exec bit is set.
@@ -208,7 +220,7 @@ int vfs_permission(struct inode * inode,
 	return -EACCES;
 }
 
-int permission(struct inode * inode,int mask, struct nameidata *nd)
+int permission(struct inode * inode, int mask, struct nameidata *nd)
 {
 	int retval;
 	int submask;
Index: linux-2.6.9-rc1-bk8/include/linux/fs.h
===================================================================
--- linux-2.6.9-rc1-bk8.orig/include/linux/fs.h
+++ linux-2.6.9-rc1-bk8/include/linux/fs.h
@@ -930,6 +930,7 @@ struct inode_operations {
 	void (*put_link) (struct dentry *, struct nameidata *);
 	void (*truncate) (struct inode *);
 	int (*permission) (struct inode *, int, struct nameidata *);
+	int (*check_posix_acl) (struct inode *, int);
 	int (*setattr) (struct dentry *, struct iattr *);
 	int (*getattr) (struct vfsmount *mnt, struct dentry *, struct kstat *);
 	int (*setxattr) (struct dentry *, const char *,const void *,size_t,int);
Index: linux-2.6.9-rc1-bk8/fs/ext3/acl.c
===================================================================
--- linux-2.6.9-rc1-bk8.orig/fs/ext3/acl.c
+++ linux-2.6.9-rc1-bk8/fs/ext3/acl.c
@@ -285,60 +285,17 @@ ext3_set_acl(handle_t *handle, struct in
 	return error;
 }
 
-/*
- * Inode operation permission().
- *
- * inode->i_sem: don't care
- */
 int
-ext3_permission(struct inode *inode, int mask, struct nameidata *nd)
+ext3_check_posix_acl(struct inode *inode, int mask)
 {
-	int mode = inode->i_mode;
+	struct posix_acl *acl = ext3_get_acl(inode, ACL_TYPE_ACCESS);
+	int error = -EAGAIN;
 
-	/* Nobody gets write access to a read-only fs */
-	if ((mask & MAY_WRITE) && IS_RDONLY(inode) &&
-	    (S_ISREG(mode) || S_ISDIR(mode) || S_ISLNK(mode)))
-		return -EROFS;
-	/* Nobody gets write access to an immutable file */
-	if ((mask & MAY_WRITE) && IS_IMMUTABLE(inode))
-	    return -EACCES;
-	if (current->fsuid == inode->i_uid) {
-		mode >>= 6;
-	} else if (test_opt(inode->i_sb, POSIX_ACL)) {
-		struct posix_acl *acl;
-
-		/* The access ACL cannot grant access if the group class
-		   permission bits don't contain all requested permissions. */
-		if (((mode >> 3) & mask & S_IRWXO) != mask)
-			goto check_groups;
-		acl = ext3_get_acl(inode, ACL_TYPE_ACCESS);
-		if (acl) {
-			int error = posix_acl_permission(inode, acl, mask);
-			posix_acl_release(acl);
-			if (error == -EACCES)
-				goto check_capabilities;
-			return error;
-		} else
-			goto check_groups;
-	} else {
-check_groups:
-		if (in_group_p(inode->i_gid))
-			mode >>= 3;
+	if (acl) {
+		error = posix_acl_permission(inode, acl, mask);
+		posix_acl_release(acl);
 	}
-	if ((mode & mask & S_IRWXO) == mask)
-		return 0;
-
-check_capabilities:
-	/* Allowed to override Discretionary Access Control? */
-	if (!(mask & MAY_EXEC) ||
-	    (inode->i_mode & S_IXUGO) || S_ISDIR(inode->i_mode))
-		if (capable(CAP_DAC_OVERRIDE))
-			return 0;
-	/* Read and search granted if capable(CAP_DAC_READ_SEARCH) */
-	if (capable(CAP_DAC_READ_SEARCH) && ((mask == MAY_READ) ||
-	    (S_ISDIR(inode->i_mode) && !(mask & MAY_WRITE))))
-		return 0;
-	return -EACCES;
+	return error;
 }
 
 /*
Index: linux-2.6.9-rc1-bk8/fs/ext3/namei.c
===================================================================
--- linux-2.6.9-rc1-bk8.orig/fs/ext3/namei.c
+++ linux-2.6.9-rc1-bk8/fs/ext3/namei.c
@@ -2359,7 +2359,7 @@ struct inode_operations ext3_dir_inode_o
 	.getxattr	= ext3_getxattr,
 	.listxattr	= ext3_listxattr,
 	.removexattr	= ext3_removexattr,
-	.permission	= ext3_permission,
+	.check_posix_acl = ext3_check_posix_acl,
 };
 
 struct inode_operations ext3_special_inode_operations = {
@@ -2368,5 +2368,5 @@ struct inode_operations ext3_special_ino
 	.getxattr	= ext3_getxattr,
 	.listxattr	= ext3_listxattr,
 	.removexattr	= ext3_removexattr,
-	.permission	= ext3_permission,
+	.check_posix_acl = ext3_check_posix_acl,
 }; 

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] generic acl support for ->permission
  2004-09-01 19:20 ` Andreas Gruenbacher
@ 2004-09-04 10:46   ` Christoph Hellwig
  0 siblings, 0 replies; 5+ messages in thread
From: Christoph Hellwig @ 2004-09-04 10:46 UTC (permalink / raw)
  To: Andreas Gruenbacher; +Cc: linux-fsdevel, shaggy, jeffm

On Wed, Sep 01, 2004 at 09:20:52PM +0200, Andreas Gruenbacher wrote:
> Hello Christoph,
> 
> as far as I can see what you propose is very similar to the approach taken in 
> the attached patch, with one more indirection. This looks nicer to me, at the 
> cost of "polluting" iops instead of vfs_permission. JFS will still need 
> Dave's patch, obviously.
> 
> The attached patch only covers ext3; not sure if it even compiles.

I don't like this one conceptually.  checking a posix ACL isn't a
fundamental method for the inode object.  Checking permission is, and
checking the ACL as a subaspect of that should really be a callback and
not a method.


^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2004-09-04 10:47 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2004-09-01 15:24 [PATCH] generic acl support for ->permission Christoph Hellwig
2004-09-01 16:00 ` Christoph Hellwig
2004-09-01 17:40 ` Dave Kleikamp
2004-09-01 19:20 ` Andreas Gruenbacher
2004-09-04 10:46   ` Christoph Hellwig

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).