From 9c630ebefeeee4363ffd29f2f9b18eddafc6479c Mon Sep 17 00:00:00 2001 From: Miklos Szeredi Date: Fri, 29 Jul 2016 12:05:23 +0200 Subject: [PATCH] ovl: simplify permission checking The fact that we always do permission checking on the overlay inode and clear MAY_WRITE for checking access to the lower inode allows cruft to be removed from ovl_permission(). 1) "default_permissions" option effectively did generic_permission() on the overlay inode with i_mode, i_uid and i_gid updated from underlying filesystem. This is what we do by default now. It did the update using vfs_getattr() but that's only needed if the underlying filesystem can change (which is not allowed). We may later introduce a "paranoia_mode" that verifies that mode/uid/gid are not changed. 2) splitting out the IS_RDONLY() check from inode_permission() also becomes unnecessary once we remove the MAY_WRITE from the lower inode check. Signed-off-by: Miklos Szeredi --- fs/overlayfs/inode.c | 46 +--------------------------------------- fs/overlayfs/overlayfs.h | 1 - fs/overlayfs/super.c | 7 ------ 3 files changed, 1 insertion(+), 53 deletions(-) diff --git a/fs/overlayfs/inode.c b/fs/overlayfs/inode.c index 8f7dd547cfb3..66f42f5cf705 100644 --- a/fs/overlayfs/inode.c +++ b/fs/overlayfs/inode.c @@ -124,29 +124,6 @@ int ovl_permission(struct inode *inode, int mask) const struct cred *old_cred; int err; - if (ovl_is_default_permissions(inode)) { - struct kstat stat; - struct path realpath = { .dentry = realdentry }; - - if (mask & MAY_NOT_BLOCK) - return -ECHILD; - - realpath.mnt = ovl_entry_mnt_real(oe, inode, is_upper); - - err = vfs_getattr(&realpath, &stat); - if (err) - return err; - - if ((stat.mode ^ inode->i_mode) & S_IFMT) - return -ESTALE; - - inode->i_mode = stat.mode; - inode->i_uid = stat.uid; - inode->i_gid = stat.gid; - - return generic_permission(inode, mask); - } - /* Careful in RCU walk mode */ realinode = d_inode_rcu(realdentry); if (!realinode) { @@ -154,27 +131,6 @@ int ovl_permission(struct inode *inode, int mask) return -ENOENT; } - if (mask & MAY_WRITE) { - umode_t mode = realinode->i_mode; - - /* - * Writes will always be redirected to upper layer, so - * ignore lower layer being read-only. - * - * If the overlay itself is read-only then proceed - * with the permission check, don't return EROFS. - * This will only happen if this is the lower layer of - * another overlayfs. - * - * If upper fs becomes read-only after the overlay was - * constructed return EROFS to prevent modification of - * upper layer. - */ - if (is_upper && !IS_RDONLY(inode) && IS_RDONLY(realinode) && - (S_ISREG(mode) || S_ISDIR(mode) || S_ISLNK(mode))) - return -EROFS; - } - /* * Check overlay inode with the creds of task and underlying inode * with creds of mounter @@ -186,7 +142,7 @@ int ovl_permission(struct inode *inode, int mask) old_cred = ovl_override_creds(inode->i_sb); if (!is_upper) mask &= ~(MAY_WRITE | MAY_APPEND); - err = __inode_permission(realinode, mask); + err = inode_permission(realinode, mask); revert_creds(old_cred); return err; diff --git a/fs/overlayfs/overlayfs.h b/fs/overlayfs/overlayfs.h index 75128c70aa6a..e8d50da384d5 100644 --- a/fs/overlayfs/overlayfs.h +++ b/fs/overlayfs/overlayfs.h @@ -146,7 +146,6 @@ struct inode *ovl_inode_real(struct inode *inode); struct vfsmount *ovl_entry_mnt_real(struct ovl_entry *oe, struct inode *inode, bool is_upper); struct ovl_dir_cache *ovl_dir_cache(struct dentry *dentry); -bool ovl_is_default_permissions(struct inode *inode); void ovl_set_dir_cache(struct dentry *dentry, struct ovl_dir_cache *cache); struct dentry *ovl_workdir(struct dentry *dentry); int ovl_want_write(struct dentry *dentry); diff --git a/fs/overlayfs/super.c b/fs/overlayfs/super.c index 893d6e0ea1c5..80598912a5d9 100644 --- a/fs/overlayfs/super.c +++ b/fs/overlayfs/super.c @@ -185,13 +185,6 @@ struct ovl_dir_cache *ovl_dir_cache(struct dentry *dentry) return oe->cache; } -bool ovl_is_default_permissions(struct inode *inode) -{ - struct ovl_fs *ofs = inode->i_sb->s_fs_info; - - return ofs->config.default_permissions; -} - void ovl_set_dir_cache(struct dentry *dentry, struct ovl_dir_cache *cache) { struct ovl_entry *oe = dentry->d_fsdata;