linux-fsdevel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Erez Zadok <ezk@cs.sunysb.edu>
To: akpm@linux-foundation.org
Cc: linux-kernel@vger.kernel.org, linux-fsdevel@vger.kernel.org,
	viro@ftp.linux.org.uk, hch@infradead.org,
	Erez Zadok <ezk@cs.sunysb.edu>
Subject: [PATCH 2/8] Unionfs: cleanup permission checking code
Date: Fri,  2 Nov 2007 21:22:43 -0400	[thread overview]
Message-ID: <119405297013-git-send-email-ezk@cs.sunysb.edu> (raw)
In-Reply-To: <11940529692937-git-send-email-ezk@cs.sunysb.edu>

Use vfs helpers and avoid redundant checks performed by the VFS already.

Signed-off-by: Erez Zadok <ezk@cs.sunysb.edu>
---
 fs/unionfs/commonfops.c |    4 ---
 fs/unionfs/inode.c      |   70 +++++++++--------------------------------------
 2 files changed, 13 insertions(+), 61 deletions(-)

diff --git a/fs/unionfs/commonfops.c b/fs/unionfs/commonfops.c
index 7654bcb..50e5775 100644
--- a/fs/unionfs/commonfops.c
+++ b/fs/unionfs/commonfops.c
@@ -669,10 +669,6 @@ static long do_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
 
 	lower_file = unionfs_lower_file(file);
 
-	err = security_file_ioctl(lower_file, cmd, arg);
-	if (err)
-		goto out;
-
 	err = -ENOTTY;
 	if (!lower_file || !lower_file->f_op)
 		goto out;
diff --git a/fs/unionfs/inode.c b/fs/unionfs/inode.c
index f4facf4..169365c 100644
--- a/fs/unionfs/inode.c
+++ b/fs/unionfs/inode.c
@@ -914,59 +914,6 @@ 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) 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.
- *
- * @inode: the lower inode we're checking permission on
- */
-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 &&
-		    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;
-		/*
-		 * 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;
-	}
-
-	/* Ordinary permission routines do not understand MAY_APPEND. */
-	submask = mask & ~MAY_APPEND;
-	if (inode->i_op && inode->i_op->permission)
-		retval = inode->i_op->permission(inode, submask, nd);
-	else
-		retval = generic_permission(inode, submask, NULL);
-
-	if (retval && retval != -EROFS)	/* ignore EROFS */
-		return retval;
-
-	retval = security_inode_permission(inode, mask, nd);
-	return ((retval == -EROFS) ? 0 : retval);	/* ignore EROFS */
-}
-
-/*
  * Don't grab the superblock read-lock in unionfs_permission, which prevents
  * a deadlock with the branch-management "add branch" code (which grabbed
  * the write lock).  It is safe to not grab the read lock here, because even
@@ -1011,11 +958,20 @@ static int unionfs_permission(struct inode *inode, int mask,
 			continue;
 
 		/*
-		 * We use our own special version of permission, such that
-		 * only the first branch returns -EROFS.
+		 * We check basic permissions, but we ignore any conditions
+		 * such as readonly file systems or branches marked as
+		 * readonly, because those conditions should lead to a
+		 * copyup taking place later on.
 		 */
-		err = inode_permission(inode->i_sb, lower_inode, mask, nd,
-				       bindex);
+		err = permission(lower_inode, mask, nd);
+		if (err && bindex > 0) {
+			umode_t mode = lower_inode->i_mode;
+			if (is_robranch_super(inode->i_sb, bindex) &&
+			    (S_ISREG(mode) || S_ISDIR(mode) || S_ISLNK(mode)))
+				err = 0;
+			if (IS_COPYUP_ERR(err))
+				err = 0;
+		}
 
 		/*
 		 * The permissions are an intersection of the overall directory
-- 
1.5.2.2


  parent reply	other threads:[~2007-11-03  1:23 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2007-11-03  1:22 [GIT PULL -mm] 0/8 Unionfs updates/cleanups/fixes Erez Zadok
2007-11-03  1:22 ` [PATCH 1/8] Unionfs: delete whiteouts in sticky directories Erez Zadok
2007-11-03  1:22 ` Erez Zadok [this message]
2007-11-03  1:22 ` [PATCH 3/8] Unionfs: update usage.txt documentation Erez Zadok
2007-11-03  1:22 ` [PATCH 4/8] Unionfs: mmap updates Erez Zadok
2007-11-03  1:22 ` [PATCH 5/8] Unionfs: avoid a deadlock during branch-management on a pivot_root'ed union Erez Zadok
2007-11-03  1:22 ` [PATCH 6/8] Unionfs: don't bother validating inode if it has no lower branches Erez Zadok
2007-11-03  1:22 ` [PATCH 7/8] Unionfs: don't printk an error if it's due to common copyup Erez Zadok
2007-11-03  1:22 ` [PATCH 8/8] Unionfs/VFS: no need to export 2 symbols in security/security.c Erez Zadok

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=119405297013-git-send-email-ezk@cs.sunysb.edu \
    --to=ezk@cs.sunysb.edu \
    --cc=akpm@linux-foundation.org \
    --cc=hch@infradead.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=viro@ftp.linux.org.uk \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).