GIT: unionfs2-2.6.27.y: Unionfs: bugfix when mounting readonly exported NFS volumes (was: nfsro)

Erez Zadok ezk at fsl.cs.sunysb.edu
Thu Aug 12 23:16:51 EDT 2010


commit 157bec38ff6b6cb101813a714d388b2e59c692cc
Author: Erez_Zadok <ezk at cs.sunysb.edu>
Date:   Thu Jul 26 00:04:58 2007 -0400

    Unionfs: bugfix when mounting readonly exported NFS volumes (was: nfsro)
    
    When stacking on top of readonly exported NFS volumes, which are mounted
    locally read-write, attempting to create a new file yields the proper EROFS.
    But attempting to modify an existing file returns an EACCES, which
    interferes with unionfs's copyup policy: only EROFS triggers a copyup,
    whereas EACCES does not (and shouldn't -- that'd be a security hole).  The
    old unionfs 1.x attempted to workaround this EACCES condition by supporting
    a special unionfs mount option called 'nfsro'; support for this option was
    left in the latest unionfs 2.0, but the mount option was not made available
    until we could properly investigate this issue with the latest NFS code.
    
    This patch removes all remnants of this 'nfsro' support.  It is no longer
    needed.  Instead, users can use the existing per-branch 'ro' unionfs mount
    option, which would properly return the appropriate status conditions back
    from unionfs_permission.  These return conditions result in a copyup if and
    only if needed, even for readonly exported NFS volumes.  In effect, unionfs
    per-branch 'ro' option now simulates a true readonly localhost mount.
    
    Signed-off-by: Erez Zadok <ezk at cs.sunysb.edu>

diff --git a/fs/unionfs/inode.c b/fs/unionfs/inode.c
index 401c62b..53d373a 100644
--- a/fs/unionfs/inode.c
+++ b/fs/unionfs/inode.c
@@ -967,26 +967,32 @@ static void unionfs_put_link(struct dentry *dentry, struct nameidata *nd,
  * Basically copied from the kernel vfs permission(), but we've changed
  * the following:
  *   (1) the IS_RDONLY check is skipped, and
- *   (2) if you set the mount option `mode=nfsro', we assume that -EACCES
- *   means that the export is read-only and we should check standard Unix
- *   permissions.  This means that NFS ACL checks (or other advanced
- *   permission features) are bypassed. Note however, that we do call
- *   security_inode_permission, and therefore security inside SELinux, etc.
- *   are performed.
+ *   (2) We return 0 (success) if the non-leftmost branch is mounted
+ *       readonly, to allow copyup to work.
+ *   (3) we do call security_inode_permission, and therefore security inside
+ *       SELinux, etc. are performed.
  */
-static int inode_permission(struct inode *inode, int mask,
+static int inode_permission(struct super_block *sb, struct inode *inode, int mask,
 			    struct nameidata *nd, int bindex)
 {
 	int retval, submask;
 
 	if (mask & MAY_WRITE) {
+		umode_t mode = inode->i_mode;
 		/* The first branch is allowed to be really readonly. */
-		if (bindex == 0) {
-			umode_t mode = inode->i_mode;
-			if (IS_RDONLY(inode) &&
-			    (S_ISREG(mode) || S_ISDIR(mode) || S_ISLNK(mode)))
-				return -EROFS;
-		}
+		if (bindex == 0 &&
+		    IS_RDONLY(inode) &&
+		    (S_ISREG(mode) || S_ISDIR(mode) || S_ISLNK(mode)))
+			return -EROFS;
+		/*
+		 * For all other branches than the first one, we ignore
+		 * EROFS or if the branch is mounted as readonly, to let
+		 * copyup take place.
+		 */
+		if (bindex > 0 &&
+		    is_robranch_super(sb, bindex) &&
+		    (S_ISREG(mode) || S_ISDIR(mode) || S_ISLNK(mode)))
+			return 0;
 		/*
 		 * Nobody gets write access to an immutable file.
 		 */
@@ -996,18 +1002,9 @@ static int inode_permission(struct inode *inode, int mask,
 
 	/* Ordinary permission routines do not understand MAY_APPEND. */
 	submask = mask & ~MAY_APPEND;
-	if (inode->i_op && inode->i_op->permission) {
+	if (inode->i_op && inode->i_op->permission)
 		retval = inode->i_op->permission(inode, submask, nd);
-		if ((retval == -EACCES) && (submask & MAY_WRITE) &&
-		    (!strcmp("nfs", (inode)->i_sb->s_type->name)) &&
-		    (nd) && (nd->mnt) && (nd->mnt->mnt_sb)) {
-			int perms;
-			perms = branchperms(nd->mnt->mnt_sb, bindex);
-			if (perms & MAY_NFSRO)
-				retval = generic_permission(inode, submask,
-							    NULL);
-		}
-	} else
+	else
 		retval = generic_permission(inode, submask, NULL);
 
 	if (retval && retval != -EROFS)	/* ignore EROFS */
@@ -1065,7 +1062,7 @@ static int unionfs_permission(struct inode *inode, int mask,
 		 * We use our own special version of permission, such that
 		 * only the first branch returns -EROFS.
 		 */
-		err = inode_permission(lower_inode, mask, nd, bindex);
+		err = inode_permission(inode->i_sb, lower_inode, mask, nd, bindex);
 
 		/*
 		 * The permissions are an intersection of the overall directory
diff --git a/include/linux/union_fs.h b/include/linux/union_fs.h
index 223ccab..9bc4e3b 100644
--- a/include/linux/union_fs.h
+++ b/include/linux/union_fs.h
@@ -22,8 +22,5 @@
 /* We don't support normal remount, but unionctl uses it. */
 # define UNIONFS_REMOUNT_MAGIC		0x4a5a4380
 
-/* should be at least LAST_USED_UNIONFS_PERMISSION<<1 */
-#define MAY_NFSRO			16
-
 #endif /* _LINUX_UNIONFS_H */
 


More information about the unionfs-cvs mailing list