From c1b2cc1a765aff4df7b22abe6b66014236f73eba Mon Sep 17 00:00:00 2001 From: Miklos Szeredi Date: Fri, 29 Jul 2016 12:05:22 +0200 Subject: [PATCH 01/23] ovl: check mounter creds on underlying lookup The hash salting changes meant that we can no longer reuse the hash in the overlay dentry to look up the underlying dentry. Instead of lookup_hash(), use lookup_one_len_unlocked() and swith to mounter's creds (like we do for all other operations later in the series). Now the lookup_hash() export introduced in 4.6 by 3c9fe8cdff1b ("vfs: add lookup_hash() helper") is unused and can possibly be removed; its usefulness negated by the hash salting and the idea that mounter's creds should be used on operations on underlying filesystems. Signed-off-by: Miklos Szeredi Fixes: 8387ff2577eb ("vfs: make the string hashes salt the hash") --- fs/overlayfs/super.c | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/fs/overlayfs/super.c b/fs/overlayfs/super.c index 5e254b3a8c56..cbfa0398f9da 100644 --- a/fs/overlayfs/super.c +++ b/fs/overlayfs/super.c @@ -423,12 +423,16 @@ static bool ovl_dentry_weird(struct dentry *dentry) DCACHE_OP_COMPARE); } -static inline struct dentry *ovl_lookup_real(struct dentry *dir, +static inline struct dentry *ovl_lookup_real(struct super_block *ovl_sb, + struct dentry *dir, struct qstr *name) { + const struct cred *old_cred; struct dentry *dentry; - dentry = lookup_hash(name, dir); + old_cred = ovl_override_creds(ovl_sb); + dentry = lookup_one_len_unlocked(name->name, dir, name->len); + revert_creds(old_cred); if (IS_ERR(dentry)) { if (PTR_ERR(dentry) == -ENOENT) @@ -481,7 +485,7 @@ struct dentry *ovl_lookup(struct inode *dir, struct dentry *dentry, upperdir = ovl_upperdentry_dereference(poe); if (upperdir) { - this = ovl_lookup_real(upperdir, &dentry->d_name); + this = ovl_lookup_real(dentry->d_sb, upperdir, &dentry->d_name); err = PTR_ERR(this); if (IS_ERR(this)) goto out; @@ -514,7 +518,8 @@ struct dentry *ovl_lookup(struct inode *dir, struct dentry *dentry, bool opaque = false; struct path lowerpath = poe->lowerstack[i]; - this = ovl_lookup_real(lowerpath.dentry, &dentry->d_name); + this = ovl_lookup_real(dentry->d_sb, + lowerpath.dentry, &dentry->d_name); err = PTR_ERR(this); if (IS_ERR(this)) { /* From eead4f2dc4f851a3790c49850e96a1d155bf5451 Mon Sep 17 00:00:00 2001 From: Miklos Szeredi Date: Fri, 29 Jul 2016 12:05:22 +0200 Subject: [PATCH 02/23] ovl: use generic_delete_inode No point in keeping overlay inodes around since they will never be reused. Signed-off-by: Miklos Szeredi --- fs/overlayfs/super.c | 1 + 1 file changed, 1 insertion(+) diff --git a/fs/overlayfs/super.c b/fs/overlayfs/super.c index cbfa0398f9da..5341ca57677c 100644 --- a/fs/overlayfs/super.c +++ b/fs/overlayfs/super.c @@ -691,6 +691,7 @@ static const struct super_operations ovl_super_operations = { .statfs = ovl_statfs, .show_options = ovl_show_options, .remount_fs = ovl_remount, + .drop_inode = generic_delete_inode, }; enum { From 58ed4e70f253d80ed72faba7873dc11603b398bc Mon Sep 17 00:00:00 2001 From: Andreas Gruenbacher Date: Thu, 26 May 2016 02:01:47 +0200 Subject: [PATCH 03/23] ovl: store ovl_entry in inode->i_private for all inodes Previously this was only done for directory inodes. Doing so for all inodes makes for a nice cleanup in ovl_permission at zero cost. Inodes are not shared for hard links on the overlay, so this works fine. Signed-off-by: Miklos Szeredi --- fs/overlayfs/inode.c | 48 ++++++++++---------------------------------- 1 file changed, 11 insertions(+), 37 deletions(-) diff --git a/fs/overlayfs/inode.c b/fs/overlayfs/inode.c index d554e86abbe3..32ae8b49a72c 100644 --- a/fs/overlayfs/inode.c +++ b/fs/overlayfs/inode.c @@ -109,31 +109,12 @@ static int ovl_getattr(struct vfsmount *mnt, struct dentry *dentry, int ovl_permission(struct inode *inode, int mask) { - struct ovl_entry *oe; - struct dentry *alias = NULL; - struct inode *realinode; - struct dentry *realdentry; + struct ovl_entry *oe = inode->i_private; bool is_upper; + struct dentry *realdentry = ovl_entry_real(oe, &is_upper); + struct inode *realinode; int err; - if (S_ISDIR(inode->i_mode)) { - oe = inode->i_private; - } else if (mask & MAY_NOT_BLOCK) { - return -ECHILD; - } else { - /* - * For non-directories find an alias and get the info - * from there. - */ - alias = d_find_any_alias(inode); - if (WARN_ON(!alias)) - return -ENOENT; - - oe = alias->d_fsdata; - } - - realdentry = ovl_entry_real(oe, &is_upper); - if (ovl_is_default_permissions(inode)) { struct kstat stat; struct path realpath = { .dentry = realdentry }; @@ -145,26 +126,23 @@ int ovl_permission(struct inode *inode, int mask) err = vfs_getattr(&realpath, &stat); if (err) - goto out_dput; + return err; - err = -ESTALE; if ((stat.mode ^ inode->i_mode) & S_IFMT) - goto out_dput; + return -ESTALE; inode->i_mode = stat.mode; inode->i_uid = stat.uid; inode->i_gid = stat.gid; - err = generic_permission(inode, mask); - goto out_dput; + return generic_permission(inode, mask); } /* Careful in RCU walk mode */ - realinode = ACCESS_ONCE(realdentry->d_inode); + realinode = d_inode_rcu(realdentry); if (!realinode) { WARN_ON(!(mask & MAY_NOT_BLOCK)); - err = -ENOENT; - goto out_dput; + return -ENOENT; } if (mask & MAY_WRITE) { @@ -183,16 +161,12 @@ int ovl_permission(struct inode *inode, int mask) * constructed return EROFS to prevent modification of * upper layer. */ - err = -EROFS; if (is_upper && !IS_RDONLY(inode) && IS_RDONLY(realinode) && (S_ISREG(mode) || S_ISDIR(mode) || S_ISLNK(mode))) - goto out_dput; + return -EROFS; } - err = __inode_permission(realinode, mask); -out_dput: - dput(alias); - return err; + return __inode_permission(realinode, mask); } static const char *ovl_get_link(struct dentry *dentry, @@ -405,11 +379,11 @@ struct inode *ovl_new_inode(struct super_block *sb, umode_t mode, inode->i_ino = get_next_ino(); inode->i_mode = mode; inode->i_flags |= S_NOATIME | S_NOCMTIME; + inode->i_private = oe; mode &= S_IFMT; switch (mode) { case S_IFDIR: - inode->i_private = oe; inode->i_op = &ovl_dir_inode_operations; inode->i_fop = &ovl_dir_operations; break; From 72e48481815eeca72fc886b3be91301ad87d6aeb Mon Sep 17 00:00:00 2001 From: Vivek Goyal Date: Thu, 16 Jun 2016 10:09:14 -0400 Subject: [PATCH 04/23] ovl: move some common code in a function ovl_create_upper() and ovl_create_over_whiteout() seem to be sharing some common code which can be moved into a separate function. No functionality change. Signed-off-by: Vivek Goyal Signed-off-by: Miklos Szeredi --- fs/overlayfs/dir.c | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/fs/overlayfs/dir.c b/fs/overlayfs/dir.c index 5c9d2d80ff70..d9cdb4747f68 100644 --- a/fs/overlayfs/dir.c +++ b/fs/overlayfs/dir.c @@ -158,6 +158,16 @@ static int ovl_dir_getattr(struct vfsmount *mnt, struct dentry *dentry, return 0; } +/* Common operations required to be done after creation of file on upper */ +static void ovl_instantiate(struct dentry *dentry, struct inode *inode, + struct dentry *newdentry) +{ + ovl_dentry_version_inc(dentry->d_parent); + ovl_dentry_update(dentry, newdentry); + ovl_copyattr(newdentry->d_inode, inode); + d_instantiate(dentry, inode); +} + static int ovl_create_upper(struct dentry *dentry, struct inode *inode, struct kstat *stat, const char *link, struct dentry *hardlink) @@ -177,10 +187,7 @@ static int ovl_create_upper(struct dentry *dentry, struct inode *inode, if (err) goto out_dput; - ovl_dentry_version_inc(dentry->d_parent); - ovl_dentry_update(dentry, newdentry); - ovl_copyattr(newdentry->d_inode, inode); - d_instantiate(dentry, inode); + ovl_instantiate(dentry, inode, newdentry); newdentry = NULL; out_dput: dput(newdentry); @@ -363,10 +370,7 @@ static int ovl_create_over_whiteout(struct dentry *dentry, struct inode *inode, if (err) goto out_cleanup; } - ovl_dentry_version_inc(dentry->d_parent); - ovl_dentry_update(dentry, newdentry); - ovl_copyattr(newdentry->d_inode, inode); - d_instantiate(dentry, inode); + ovl_instantiate(dentry, inode, newdentry); newdentry = NULL; out_dput2: dput(upper); From 39a25b2b37629f65e5a1eba1b353d0b47687c2ca Mon Sep 17 00:00:00 2001 From: Vivek Goyal Date: Fri, 1 Jul 2016 16:34:26 -0400 Subject: [PATCH 05/23] ovl: define ->get_acl() for overlay inodes Now we are planning to do DAC permission checks on overlay inode itself. And to make it work, we will need to make sure we can get acls from underlying inode. So define ->get_acl() for overlay inodes and this in turn calls into underlying filesystem to get acls, if any. Signed-off-by: Vivek Goyal Signed-off-by: Miklos Szeredi --- fs/overlayfs/dir.c | 1 + fs/overlayfs/inode.c | 17 +++++++++++++++++ fs/overlayfs/overlayfs.h | 2 ++ fs/overlayfs/super.c | 8 ++++++++ 4 files changed, 28 insertions(+) diff --git a/fs/overlayfs/dir.c b/fs/overlayfs/dir.c index d9cdb4747f68..aa6320557196 100644 --- a/fs/overlayfs/dir.c +++ b/fs/overlayfs/dir.c @@ -921,4 +921,5 @@ const struct inode_operations ovl_dir_inode_operations = { .getxattr = ovl_getxattr, .listxattr = ovl_listxattr, .removexattr = ovl_removexattr, + .get_acl = ovl_get_acl, }; diff --git a/fs/overlayfs/inode.c b/fs/overlayfs/inode.c index 32ae8b49a72c..a574108f52a8 100644 --- a/fs/overlayfs/inode.c +++ b/fs/overlayfs/inode.c @@ -310,6 +310,22 @@ out: return err; } +struct posix_acl *ovl_get_acl(struct inode *inode, int type) +{ + struct inode *realinode = ovl_inode_real(inode); + + if (!realinode) + return ERR_PTR(-ENOENT); + + if (!IS_POSIXACL(realinode)) + return NULL; + + if (!realinode->i_op->get_acl) + return NULL; + + return realinode->i_op->get_acl(realinode, type); +} + static bool ovl_open_need_copy_up(int flags, enum ovl_path_type type, struct dentry *realdentry) { @@ -354,6 +370,7 @@ static const struct inode_operations ovl_file_inode_operations = { .getxattr = ovl_getxattr, .listxattr = ovl_listxattr, .removexattr = ovl_removexattr, + .get_acl = ovl_get_acl, }; static const struct inode_operations ovl_symlink_inode_operations = { diff --git a/fs/overlayfs/overlayfs.h b/fs/overlayfs/overlayfs.h index 0d3f2ad45708..75128c70aa6a 100644 --- a/fs/overlayfs/overlayfs.h +++ b/fs/overlayfs/overlayfs.h @@ -142,6 +142,7 @@ struct dentry *ovl_dentry_upper(struct dentry *dentry); struct dentry *ovl_dentry_lower(struct dentry *dentry); struct dentry *ovl_dentry_real(struct dentry *dentry); struct dentry *ovl_entry_real(struct ovl_entry *oe, bool *is_upper); +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); @@ -179,6 +180,7 @@ ssize_t ovl_getxattr(struct dentry *dentry, struct inode *inode, const char *name, void *value, size_t size); ssize_t ovl_listxattr(struct dentry *dentry, char *list, size_t size); int ovl_removexattr(struct dentry *dentry, const char *name); +struct posix_acl *ovl_get_acl(struct inode *inode, int type); int ovl_open_maybe_copy_up(struct dentry *dentry, unsigned int file_flags); struct inode *ovl_new_inode(struct super_block *sb, umode_t mode, diff --git a/fs/overlayfs/super.c b/fs/overlayfs/super.c index 5341ca57677c..893d6e0ea1c5 100644 --- a/fs/overlayfs/super.c +++ b/fs/overlayfs/super.c @@ -159,6 +159,13 @@ struct dentry *ovl_entry_real(struct ovl_entry *oe, bool *is_upper) return realdentry; } +struct inode *ovl_inode_real(struct inode *inode) +{ + bool tmp; + + return d_inode(ovl_entry_real(inode->i_private, &tmp)); +} + struct vfsmount *ovl_entry_mnt_real(struct ovl_entry *oe, struct inode *inode, bool is_upper) { @@ -1172,6 +1179,7 @@ static int ovl_fill_super(struct super_block *sb, void *data, int silent) sb->s_op = &ovl_super_operations; sb->s_root = root_dentry; sb->s_fs_info = ufs; + sb->s_flags |= MS_POSIXACL; return 0; From c0ca3d70e8d3cf81e2255a217f7ca402f5ed0862 Mon Sep 17 00:00:00 2001 From: Vivek Goyal Date: Fri, 1 Jul 2016 16:34:27 -0400 Subject: [PATCH 06/23] ovl: modify ovl_permission() to do checks on two inodes Right now ovl_permission() calls __inode_permission(realinode), to do permission checks on real inode and no checks are done on overlay inode. Modify it to do checks both on overlay inode as well as underlying inode. Checks on overlay inode will be done with the creds of calling task while checks on underlying inode will be done with the creds of mounter. Signed-off-by: Vivek Goyal Signed-off-by: Miklos Szeredi --- fs/overlayfs/inode.c | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/fs/overlayfs/inode.c b/fs/overlayfs/inode.c index a574108f52a8..f84492ff505d 100644 --- a/fs/overlayfs/inode.c +++ b/fs/overlayfs/inode.c @@ -113,6 +113,7 @@ int ovl_permission(struct inode *inode, int mask) bool is_upper; struct dentry *realdentry = ovl_entry_real(oe, &is_upper); struct inode *realinode; + const struct cred *old_cred; int err; if (ovl_is_default_permissions(inode)) { @@ -166,7 +167,19 @@ int ovl_permission(struct inode *inode, int mask) return -EROFS; } - return __inode_permission(realinode, mask); + /* + * Check overlay inode with the creds of task and underlying inode + * with creds of mounter + */ + err = generic_permission(inode, mask); + if (err) + return err; + + old_cred = ovl_override_creds(inode->i_sb); + err = __inode_permission(realinode, mask); + revert_creds(old_cred); + + return err; } static const char *ovl_get_link(struct dentry *dentry, @@ -314,9 +327,6 @@ struct posix_acl *ovl_get_acl(struct inode *inode, int type) { struct inode *realinode = ovl_inode_real(inode); - if (!realinode) - return ERR_PTR(-ENOENT); - if (!IS_POSIXACL(realinode)) return NULL; From 1175b6b8d96331676f1d436b089b965807f23b4a Mon Sep 17 00:00:00 2001 From: Vivek Goyal Date: Fri, 1 Jul 2016 16:34:28 -0400 Subject: [PATCH 07/23] ovl: do operations on underlying file system in mounter's context Given we are now doing checks both on overlay inode as well underlying inode, we should be able to do checks and operations on underlying file system using mounter's context. So modify all operations to do checks/operations on underlying dentry/inode in the context of mounter. Signed-off-by: Vivek Goyal Signed-off-by: Miklos Szeredi --- fs/overlayfs/dir.c | 58 +++++++++++++++++++++----------------------- fs/overlayfs/inode.c | 49 ++++++++++++++++++++++++++++++++----- 2 files changed, 71 insertions(+), 36 deletions(-) diff --git a/fs/overlayfs/dir.c b/fs/overlayfs/dir.c index aa6320557196..7195306e9f84 100644 --- a/fs/overlayfs/dir.c +++ b/fs/overlayfs/dir.c @@ -138,9 +138,12 @@ static int ovl_dir_getattr(struct vfsmount *mnt, struct dentry *dentry, int err; enum ovl_path_type type; struct path realpath; + const struct cred *old_cred; type = ovl_path_real(dentry, &realpath); + old_cred = ovl_override_creds(dentry->d_sb); err = vfs_getattr(&realpath, stat); + revert_creds(old_cred); if (err) return err; @@ -391,6 +394,8 @@ static int ovl_create_or_link(struct dentry *dentry, int mode, dev_t rdev, { int err; struct inode *inode; + const struct cred *old_cred; + struct cred *override_cred; struct kstat stat = { .mode = mode, .rdev = rdev, @@ -405,28 +410,23 @@ static int ovl_create_or_link(struct dentry *dentry, int mode, dev_t rdev, if (err) goto out_iput; - if (!ovl_dentry_is_opaque(dentry)) { - err = ovl_create_upper(dentry, inode, &stat, link, hardlink); - } else { - const struct cred *old_cred; - struct cred *override_cred; - - old_cred = ovl_override_creds(dentry->d_sb); - - err = -ENOMEM; - override_cred = prepare_creds(); - if (override_cred) { - override_cred->fsuid = old_cred->fsuid; - override_cred->fsgid = old_cred->fsgid; - put_cred(override_creds(override_cred)); - put_cred(override_cred); + old_cred = ovl_override_creds(dentry->d_sb); + err = -ENOMEM; + override_cred = prepare_creds(); + if (override_cred) { + override_cred->fsuid = old_cred->fsuid; + override_cred->fsgid = old_cred->fsgid; + put_cred(override_creds(override_cred)); + put_cred(override_cred); + if (!ovl_dentry_is_opaque(dentry)) + err = ovl_create_upper(dentry, inode, &stat, link, + hardlink); + else err = ovl_create_over_whiteout(dentry, inode, &stat, - link, hardlink); - } - revert_creds(old_cred); + link, hardlink); } - + revert_creds(old_cred); if (!err) inode = NULL; out_iput: @@ -637,6 +637,8 @@ static int ovl_do_remove(struct dentry *dentry, bool is_dir) { enum ovl_path_type type; int err; + const struct cred *old_cred; + err = ovl_check_sticky(dentry); if (err) @@ -651,15 +653,13 @@ static int ovl_do_remove(struct dentry *dentry, bool is_dir) goto out_drop_write; type = ovl_path_type(dentry); - if (OVL_TYPE_PURE_UPPER(type)) { + + old_cred = ovl_override_creds(dentry->d_sb); + if (OVL_TYPE_PURE_UPPER(type)) err = ovl_remove_upper(dentry, is_dir); - } else { - const struct cred *old_cred = ovl_override_creds(dentry->d_sb); - + else err = ovl_remove_and_whiteout(dentry, is_dir); - - revert_creds(old_cred); - } + revert_creds(old_cred); out_drop_write: ovl_drop_write(dentry); out: @@ -764,8 +764,7 @@ static int ovl_rename2(struct inode *olddir, struct dentry *old, old_opaque = !OVL_TYPE_PURE_UPPER(old_type); new_opaque = !OVL_TYPE_PURE_UPPER(new_type); - if (old_opaque || new_opaque) - old_cred = ovl_override_creds(old->d_sb); + old_cred = ovl_override_creds(old->d_sb); if (overwrite && OVL_TYPE_MERGE_OR_LOWER(new_type) && new_is_dir) { opaquedir = ovl_check_empty_and_clear(new); @@ -895,8 +894,7 @@ out_dput_old: out_unlock: unlock_rename(new_upperdir, old_upperdir); out_revert_creds: - if (old_opaque || new_opaque) - revert_creds(old_cred); + revert_creds(old_cred); out_drop_write: ovl_drop_write(old); out: diff --git a/fs/overlayfs/inode.c b/fs/overlayfs/inode.c index f84492ff505d..5becbaf1cec7 100644 --- a/fs/overlayfs/inode.c +++ b/fs/overlayfs/inode.c @@ -41,6 +41,7 @@ int ovl_setattr(struct dentry *dentry, struct iattr *attr) { int err; struct dentry *upperdentry; + const struct cred *old_cred; /* * Check for permissions before trying to copy-up. This is redundant @@ -84,7 +85,9 @@ int ovl_setattr(struct dentry *dentry, struct iattr *attr) attr->ia_valid &= ~ATTR_MODE; inode_lock(upperdentry->d_inode); + old_cred = ovl_override_creds(dentry->d_sb); err = notify_change(upperdentry, attr, NULL); + revert_creds(old_cred); if (!err) ovl_copyattr(upperdentry->d_inode, dentry->d_inode); inode_unlock(upperdentry->d_inode); @@ -102,9 +105,14 @@ static int ovl_getattr(struct vfsmount *mnt, struct dentry *dentry, struct kstat *stat) { struct path realpath; + const struct cred *old_cred; + int err; ovl_path_real(dentry, &realpath); - return vfs_getattr(&realpath, stat); + old_cred = ovl_override_creds(dentry->d_sb); + err = vfs_getattr(&realpath, stat); + revert_creds(old_cred); + return err; } int ovl_permission(struct inode *inode, int mask) @@ -188,6 +196,8 @@ static const char *ovl_get_link(struct dentry *dentry, { struct dentry *realdentry; struct inode *realinode; + const struct cred *old_cred; + const char *p; if (!dentry) return ERR_PTR(-ECHILD); @@ -198,13 +208,18 @@ static const char *ovl_get_link(struct dentry *dentry, if (WARN_ON(!realinode->i_op->get_link)) return ERR_PTR(-EPERM); - return realinode->i_op->get_link(realdentry, realinode, done); + old_cred = ovl_override_creds(dentry->d_sb); + p = realinode->i_op->get_link(realdentry, realinode, done); + revert_creds(old_cred); + return p; } static int ovl_readlink(struct dentry *dentry, char __user *buf, int bufsiz) { struct path realpath; struct inode *realinode; + const struct cred *old_cred; + int err; ovl_path_real(dentry, &realpath); realinode = realpath.dentry->d_inode; @@ -214,10 +229,12 @@ static int ovl_readlink(struct dentry *dentry, char __user *buf, int bufsiz) touch_atime(&realpath); - return realinode->i_op->readlink(realpath.dentry, buf, bufsiz); + old_cred = ovl_override_creds(dentry->d_sb); + err = realinode->i_op->readlink(realpath.dentry, buf, bufsiz); + revert_creds(old_cred); + return err; } - static bool ovl_is_private_xattr(const char *name) { return strncmp(name, OVL_XATTR_PRE_NAME, OVL_XATTR_PRE_LEN) == 0; @@ -229,6 +246,7 @@ int ovl_setxattr(struct dentry *dentry, struct inode *inode, { int err; struct dentry *upperdentry; + const struct cred *old_cred; err = ovl_want_write(dentry); if (err) @@ -243,7 +261,9 @@ int ovl_setxattr(struct dentry *dentry, struct inode *inode, goto out_drop_write; upperdentry = ovl_dentry_upper(dentry); + old_cred = ovl_override_creds(dentry->d_sb); err = vfs_setxattr(upperdentry, name, value, size, flags); + revert_creds(old_cred); out_drop_write: ovl_drop_write(dentry); @@ -255,11 +275,16 @@ ssize_t ovl_getxattr(struct dentry *dentry, struct inode *inode, const char *name, void *value, size_t size) { struct dentry *realdentry = ovl_dentry_real(dentry); + ssize_t res; + const struct cred *old_cred; if (ovl_is_private_xattr(name)) return -ENODATA; - return vfs_getxattr(realdentry, name, value, size); + old_cred = ovl_override_creds(dentry->d_sb); + res = vfs_getxattr(realdentry, name, value, size); + revert_creds(old_cred); + return res; } ssize_t ovl_listxattr(struct dentry *dentry, char *list, size_t size) @@ -267,8 +292,11 @@ ssize_t ovl_listxattr(struct dentry *dentry, char *list, size_t size) struct dentry *realdentry = ovl_dentry_real(dentry); ssize_t res; int off; + const struct cred *old_cred; + old_cred = ovl_override_creds(dentry->d_sb); res = vfs_listxattr(realdentry, list, size); + revert_creds(old_cred); if (res <= 0 || size == 0) return res; @@ -295,6 +323,7 @@ int ovl_removexattr(struct dentry *dentry, const char *name) int err; struct path realpath; enum ovl_path_type type = ovl_path_real(dentry, &realpath); + const struct cred *old_cred; err = ovl_want_write(dentry); if (err) @@ -316,7 +345,9 @@ int ovl_removexattr(struct dentry *dentry, const char *name) ovl_path_upper(dentry, &realpath); } + old_cred = ovl_override_creds(dentry->d_sb); err = vfs_removexattr(realpath.dentry, name); + revert_creds(old_cred); out_drop_write: ovl_drop_write(dentry); out: @@ -326,6 +357,8 @@ out: struct posix_acl *ovl_get_acl(struct inode *inode, int type) { struct inode *realinode = ovl_inode_real(inode); + const struct cred *old_cred; + struct posix_acl *acl; if (!IS_POSIXACL(realinode)) return NULL; @@ -333,7 +366,11 @@ struct posix_acl *ovl_get_acl(struct inode *inode, int type) if (!realinode->i_op->get_acl) return NULL; - return realinode->i_op->get_acl(realinode, type); + old_cred = ovl_override_creds(inode->i_sb); + acl = realinode->i_op->get_acl(realinode, type); + revert_creds(old_cred); + + return acl; } static bool ovl_open_need_copy_up(int flags, enum ovl_path_type type, From 754f8cb72b42a3a6100d2bbb1cb885361a7310dd Mon Sep 17 00:00:00 2001 From: Vivek Goyal Date: Fri, 1 Jul 2016 16:34:29 -0400 Subject: [PATCH 08/23] ovl: do not require mounter to have MAY_WRITE on lower Now we have two levels of checks in ovl_permission(). overlay inode is checked with the creds of task while underlying inode is checked with the creds of mounter. Looks like mounter does not have to have WRITE access to files on lower/. So remove the MAY_WRITE from access mask for checks on underlying lower inode. This means task should still have the MAY_WRITE permission on lower inode and mounter is not required to have MAY_WRITE. It also solves the problem of read only NFS mounts being used as lower. If __inode_permission(lower_inode, MAY_WRITE) is called on read only NFS, it fails. By resetting MAY_WRITE, check succeeds and case of read only NFS shold work with overlay without having to specify any special mount options (default permission). Signed-off-by: Vivek Goyal Signed-off-by: Miklos Szeredi --- fs/overlayfs/inode.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/fs/overlayfs/inode.c b/fs/overlayfs/inode.c index 5becbaf1cec7..8f7dd547cfb3 100644 --- a/fs/overlayfs/inode.c +++ b/fs/overlayfs/inode.c @@ -184,6 +184,8 @@ int ovl_permission(struct inode *inode, int mask) return err; old_cred = ovl_override_creds(inode->i_sb); + if (!is_upper) + mask &= ~(MAY_WRITE | MAY_APPEND); err = __inode_permission(realinode, mask); revert_creds(old_cred); From 9c630ebefeeee4363ffd29f2f9b18eddafc6479c Mon Sep 17 00:00:00 2001 From: Miklos Szeredi Date: Fri, 29 Jul 2016 12:05:23 +0200 Subject: [PATCH 09/23] 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; From bb0d2b8ad29630b580ac903f989e704e23462357 Mon Sep 17 00:00:00 2001 From: Miklos Szeredi Date: Fri, 29 Jul 2016 12:05:23 +0200 Subject: [PATCH 10/23] ovl: fix sgid on directory When creating directory in workdir, the group/sgid inheritance from the parent dir was omitted completely. Fix this by calling inode_init_owner() on overlay inode and using the resulting uid/gid/mode to create the file. Unfortunately the sgid bit can be stripped off due to umask, so need to reset the mode in this case in workdir before moving the directory in place. Reported-by: Eryu Guan Signed-off-by: Miklos Szeredi --- fs/overlayfs/dir.c | 31 +++++++++++++++++++++++++++---- 1 file changed, 27 insertions(+), 4 deletions(-) diff --git a/fs/overlayfs/dir.c b/fs/overlayfs/dir.c index 7195306e9f84..8beeed34dad6 100644 --- a/fs/overlayfs/dir.c +++ b/fs/overlayfs/dir.c @@ -357,6 +357,21 @@ static int ovl_create_over_whiteout(struct dentry *dentry, struct inode *inode, if (err) goto out_dput2; + /* + * mode could have been mutilated due to umask (e.g. sgid directory) + */ + if (!S_ISLNK(stat->mode) && newdentry->d_inode->i_mode != stat->mode) { + struct iattr attr = { + .ia_valid = ATTR_MODE, + .ia_mode = stat->mode, + }; + inode_lock(newdentry->d_inode); + err = notify_change(newdentry, &attr, NULL); + inode_unlock(newdentry->d_inode); + if (err) + goto out_cleanup; + } + if (S_ISDIR(stat->mode)) { err = ovl_set_opaque(newdentry); if (err) @@ -397,7 +412,6 @@ static int ovl_create_or_link(struct dentry *dentry, int mode, dev_t rdev, const struct cred *old_cred; struct cred *override_cred; struct kstat stat = { - .mode = mode, .rdev = rdev, }; @@ -410,12 +424,15 @@ static int ovl_create_or_link(struct dentry *dentry, int mode, dev_t rdev, if (err) goto out_iput; + inode_init_owner(inode, dentry->d_parent->d_inode, mode); + stat.mode = inode->i_mode; + old_cred = ovl_override_creds(dentry->d_sb); err = -ENOMEM; override_cred = prepare_creds(); if (override_cred) { - override_cred->fsuid = old_cred->fsuid; - override_cred->fsgid = old_cred->fsgid; + override_cred->fsuid = inode->i_uid; + override_cred->fsgid = inode->i_gid; put_cred(override_creds(override_cred)); put_cred(override_cred); @@ -427,8 +444,14 @@ static int ovl_create_or_link(struct dentry *dentry, int mode, dev_t rdev, link, hardlink); } revert_creds(old_cred); - if (!err) + if (!err) { + struct inode *realinode = d_inode(ovl_dentry_upper(dentry)); + + WARN_ON(inode->i_mode != realinode->i_mode); + WARN_ON(!uid_eq(inode->i_uid, realinode->i_uid)); + WARN_ON(!gid_eq(inode->i_gid, realinode->i_gid)); inode = NULL; + } out_iput: iput(inode); out: From d719e8f268fa4f9944b24b60814da9017dfb7787 Mon Sep 17 00:00:00 2001 From: Miklos Szeredi Date: Fri, 29 Jul 2016 12:05:23 +0200 Subject: [PATCH 11/23] ovl: update atime on upper Fix atime update logic in overlayfs. This patch adds an i_op->update_time() handler to overlayfs inodes. This forwards atime updates to the upper layer only. No atime updates are done on lower layers. Remove implicit atime updates to underlying files and directories with O_NOATIME. Remove explicit atime update in ovl_readlink(). Clear atime related mnt flags from cloned upper mount. This means atime updates are controlled purely by overlayfs mount options. Reported-by: Konstantin Khlebnikov Signed-off-by: Miklos Szeredi --- fs/overlayfs/dir.c | 1 + fs/overlayfs/inode.c | 29 ++++++++++++++++++++++++++--- fs/overlayfs/overlayfs.h | 4 ++++ fs/overlayfs/super.c | 8 ++++++-- 4 files changed, 37 insertions(+), 5 deletions(-) diff --git a/fs/overlayfs/dir.c b/fs/overlayfs/dir.c index 8beeed34dad6..b4eac8173f93 100644 --- a/fs/overlayfs/dir.c +++ b/fs/overlayfs/dir.c @@ -943,4 +943,5 @@ const struct inode_operations ovl_dir_inode_operations = { .listxattr = ovl_listxattr, .removexattr = ovl_removexattr, .get_acl = ovl_get_acl, + .update_time = ovl_update_time, }; diff --git a/fs/overlayfs/inode.c b/fs/overlayfs/inode.c index 66f42f5cf705..041db9c6621c 100644 --- a/fs/overlayfs/inode.c +++ b/fs/overlayfs/inode.c @@ -185,8 +185,6 @@ static int ovl_readlink(struct dentry *dentry, char __user *buf, int bufsiz) if (!realinode->i_op->readlink) return -EINVAL; - touch_atime(&realpath); - old_cred = ovl_override_creds(dentry->d_sb); err = realinode->i_op->readlink(realpath.dentry, buf, bufsiz); revert_creds(old_cred); @@ -367,6 +365,29 @@ int ovl_open_maybe_copy_up(struct dentry *dentry, unsigned int file_flags) return err; } +int ovl_update_time(struct inode *inode, struct timespec *ts, int flags) +{ + struct dentry *alias; + struct path upperpath; + + if (!(flags & S_ATIME)) + return 0; + + alias = d_find_any_alias(inode); + if (!alias) + return 0; + + ovl_path_upper(alias, &upperpath); + if (upperpath.dentry) { + touch_atime(&upperpath); + inode->i_atime = d_inode(upperpath.dentry)->i_atime; + } + + dput(alias); + + return 0; +} + static const struct inode_operations ovl_file_inode_operations = { .setattr = ovl_setattr, .permission = ovl_permission, @@ -376,6 +397,7 @@ static const struct inode_operations ovl_file_inode_operations = { .listxattr = ovl_listxattr, .removexattr = ovl_removexattr, .get_acl = ovl_get_acl, + .update_time = ovl_update_time, }; static const struct inode_operations ovl_symlink_inode_operations = { @@ -387,6 +409,7 @@ static const struct inode_operations ovl_symlink_inode_operations = { .getxattr = ovl_getxattr, .listxattr = ovl_listxattr, .removexattr = ovl_removexattr, + .update_time = ovl_update_time, }; struct inode *ovl_new_inode(struct super_block *sb, umode_t mode, @@ -400,7 +423,7 @@ struct inode *ovl_new_inode(struct super_block *sb, umode_t mode, inode->i_ino = get_next_ino(); inode->i_mode = mode; - inode->i_flags |= S_NOATIME | S_NOCMTIME; + inode->i_flags |= S_NOCMTIME; inode->i_private = oe; mode &= S_IFMT; diff --git a/fs/overlayfs/overlayfs.h b/fs/overlayfs/overlayfs.h index e8d50da384d5..fb73c09a84e7 100644 --- a/fs/overlayfs/overlayfs.h +++ b/fs/overlayfs/overlayfs.h @@ -181,6 +181,7 @@ ssize_t ovl_listxattr(struct dentry *dentry, char *list, size_t size); int ovl_removexattr(struct dentry *dentry, const char *name); struct posix_acl *ovl_get_acl(struct inode *inode, int type); int ovl_open_maybe_copy_up(struct dentry *dentry, unsigned int file_flags); +int ovl_update_time(struct inode *inode, struct timespec *ts, int flags); struct inode *ovl_new_inode(struct super_block *sb, umode_t mode, struct ovl_entry *oe); @@ -189,6 +190,9 @@ static inline void ovl_copyattr(struct inode *from, struct inode *to) to->i_uid = from->i_uid; to->i_gid = from->i_gid; to->i_mode = from->i_mode; + to->i_atime = from->i_atime; + to->i_mtime = from->i_mtime; + to->i_ctime = from->i_ctime; } /* dir.c */ diff --git a/fs/overlayfs/super.c b/fs/overlayfs/super.c index 80598912a5d9..058103c60f54 100644 --- a/fs/overlayfs/super.c +++ b/fs/overlayfs/super.c @@ -608,7 +608,7 @@ out: struct file *ovl_path_open(struct path *path, int flags) { - return dentry_open(path, flags, current_cred()); + return dentry_open(path, flags | O_NOATIME, current_cred()); } static void ovl_put_super(struct super_block *sb) @@ -1075,6 +1075,10 @@ static int ovl_fill_super(struct super_block *sb, void *data, int silent) pr_err("overlayfs: failed to clone upperpath\n"); goto out_put_lowerpath; } + /* Don't inherit atime flags */ + ufs->upper_mnt->mnt_flags &= ~(MNT_NOATIME | MNT_NODIRATIME | MNT_RELATIME); + + sb->s_time_gran = ufs->upper_mnt->mnt_sb->s_time_gran; ufs->workdir = ovl_workdir_create(ufs->upper_mnt, workpath.dentry); err = PTR_ERR(ufs->workdir); @@ -1122,7 +1126,7 @@ static int ovl_fill_super(struct super_block *sb, void *data, int silent) * Make lower_mnt R/O. That way fchmod/fchown on lower file * will fail instead of modifying lower fs. */ - mnt->mnt_flags |= MNT_READONLY; + mnt->mnt_flags |= MNT_READONLY | MNT_NOATIME; ufs->lower_mnt[ufs->numlower] = mnt; ufs->numlower++; From a999d7e161a085e30181d0a88f049bd92112e172 Mon Sep 17 00:00:00 2001 From: Miklos Szeredi Date: Fri, 29 Jul 2016 12:05:23 +0200 Subject: [PATCH 12/23] ovl: permission: return ECHILD instead of ENOENT The error is due to RCU and is temporary. Signed-off-by: Miklos Szeredi --- fs/overlayfs/inode.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/fs/overlayfs/inode.c b/fs/overlayfs/inode.c index 041db9c6621c..0598c169ce41 100644 --- a/fs/overlayfs/inode.c +++ b/fs/overlayfs/inode.c @@ -128,7 +128,7 @@ int ovl_permission(struct inode *inode, int mask) realinode = d_inode_rcu(realdentry); if (!realinode) { WARN_ON(!(mask & MAY_NOT_BLOCK)); - return -ENOENT; + return -ECHILD; } /* From 39b681f8026c170a73972517269efc830db0d7ce Mon Sep 17 00:00:00 2001 From: Miklos Szeredi Date: Fri, 29 Jul 2016 12:05:24 +0200 Subject: [PATCH 13/23] ovl: store real inode pointer in ->i_private To get from overlay inode to real inode we currently use 'struct ovl_entry', which has lifetime connected to overlay dentry. This is okay, since each overlay dentry had a new overlay inode allocated. Following patch will break that assumption, so need to leave out ovl_entry. This patch stores the real inode directly in i_private, with the lowest bit used to indicate whether the inode is upper or lower. Lifetime rules remain, using ovl_inode_real() must only be done while caller holds ref on overlay dentry (and hence on real dentry), or within RCU protected regions. Signed-off-by: Miklos Szeredi --- fs/overlayfs/copy_up.c | 1 + fs/overlayfs/dir.c | 3 ++- fs/overlayfs/inode.c | 11 +++------- fs/overlayfs/overlayfs.h | 18 ++++++++++++---- fs/overlayfs/super.c | 44 ++++++++++++++++++---------------------- 5 files changed, 40 insertions(+), 37 deletions(-) diff --git a/fs/overlayfs/copy_up.c b/fs/overlayfs/copy_up.c index 80aa6f1eb336..54e5d6681786 100644 --- a/fs/overlayfs/copy_up.c +++ b/fs/overlayfs/copy_up.c @@ -292,6 +292,7 @@ static int ovl_copy_up_locked(struct dentry *workdir, struct dentry *upperdir, goto out_cleanup; ovl_dentry_update(dentry, newdentry); + ovl_inode_update(d_inode(dentry), d_inode(newdentry)); newdentry = NULL; /* diff --git a/fs/overlayfs/dir.c b/fs/overlayfs/dir.c index b4eac8173f93..96b1bdcf3674 100644 --- a/fs/overlayfs/dir.c +++ b/fs/overlayfs/dir.c @@ -167,6 +167,7 @@ static void ovl_instantiate(struct dentry *dentry, struct inode *inode, { ovl_dentry_version_inc(dentry->d_parent); ovl_dentry_update(dentry, newdentry); + ovl_inode_update(inode, d_inode(newdentry)); ovl_copyattr(newdentry->d_inode, inode); d_instantiate(dentry, inode); } @@ -416,7 +417,7 @@ static int ovl_create_or_link(struct dentry *dentry, int mode, dev_t rdev, }; err = -ENOMEM; - inode = ovl_new_inode(dentry->d_sb, mode, dentry->d_fsdata); + inode = ovl_new_inode(dentry->d_sb, mode); if (!inode) goto out; diff --git a/fs/overlayfs/inode.c b/fs/overlayfs/inode.c index 0598c169ce41..2bdd3cae0f71 100644 --- a/fs/overlayfs/inode.c +++ b/fs/overlayfs/inode.c @@ -117,15 +117,12 @@ static int ovl_getattr(struct vfsmount *mnt, struct dentry *dentry, int ovl_permission(struct inode *inode, int mask) { - struct ovl_entry *oe = inode->i_private; bool is_upper; - struct dentry *realdentry = ovl_entry_real(oe, &is_upper); - struct inode *realinode; + struct inode *realinode = ovl_inode_real(inode, &is_upper); const struct cred *old_cred; int err; /* Careful in RCU walk mode */ - realinode = d_inode_rcu(realdentry); if (!realinode) { WARN_ON(!(mask & MAY_NOT_BLOCK)); return -ECHILD; @@ -312,7 +309,7 @@ out: struct posix_acl *ovl_get_acl(struct inode *inode, int type) { - struct inode *realinode = ovl_inode_real(inode); + struct inode *realinode = ovl_inode_real(inode, NULL); const struct cred *old_cred; struct posix_acl *acl; @@ -412,8 +409,7 @@ static const struct inode_operations ovl_symlink_inode_operations = { .update_time = ovl_update_time, }; -struct inode *ovl_new_inode(struct super_block *sb, umode_t mode, - struct ovl_entry *oe) +struct inode *ovl_new_inode(struct super_block *sb, umode_t mode) { struct inode *inode; @@ -424,7 +420,6 @@ struct inode *ovl_new_inode(struct super_block *sb, umode_t mode, inode->i_ino = get_next_ino(); inode->i_mode = mode; inode->i_flags |= S_NOCMTIME; - inode->i_private = oe; mode &= S_IFMT; switch (mode) { diff --git a/fs/overlayfs/overlayfs.h b/fs/overlayfs/overlayfs.h index fb73c09a84e7..6410209ea616 100644 --- a/fs/overlayfs/overlayfs.h +++ b/fs/overlayfs/overlayfs.h @@ -27,6 +27,8 @@ enum ovl_path_type { #define OVL_XATTR_PRE_LEN 16 #define OVL_XATTR_OPAQUE OVL_XATTR_PRE_NAME"opaque" +#define OVL_ISUPPER_MASK 1UL + static inline int ovl_do_rmdir(struct inode *dir, struct dentry *dentry) { int err = vfs_rmdir(dir, dentry); @@ -131,6 +133,16 @@ static inline int ovl_do_whiteout(struct inode *dir, struct dentry *dentry) return err; } +static inline struct inode *ovl_inode_real(struct inode *inode, bool *is_upper) +{ + unsigned long x = (unsigned long) READ_ONCE(inode->i_private); + + if (is_upper) + *is_upper = x & OVL_ISUPPER_MASK; + + return (struct inode *) (x & ~OVL_ISUPPER_MASK); +} + enum ovl_path_type ovl_path_type(struct dentry *dentry); u64 ovl_dentry_version_get(struct dentry *dentry); void ovl_dentry_version_inc(struct dentry *dentry); @@ -141,8 +153,6 @@ int ovl_path_next(int idx, struct dentry *dentry, struct path *path); struct dentry *ovl_dentry_upper(struct dentry *dentry); struct dentry *ovl_dentry_lower(struct dentry *dentry); struct dentry *ovl_dentry_real(struct dentry *dentry); -struct dentry *ovl_entry_real(struct ovl_entry *oe, bool *is_upper); -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); @@ -155,6 +165,7 @@ void ovl_dentry_set_opaque(struct dentry *dentry, bool opaque); bool ovl_is_whiteout(struct dentry *dentry); const struct cred *ovl_override_creds(struct super_block *sb); void ovl_dentry_update(struct dentry *dentry, struct dentry *upperdentry); +void ovl_inode_update(struct inode *inode, struct inode *upperinode); struct dentry *ovl_lookup(struct inode *dir, struct dentry *dentry, unsigned int flags); struct file *ovl_path_open(struct path *path, int flags); @@ -183,8 +194,7 @@ struct posix_acl *ovl_get_acl(struct inode *inode, int type); int ovl_open_maybe_copy_up(struct dentry *dentry, unsigned int file_flags); int ovl_update_time(struct inode *inode, struct timespec *ts, int flags); -struct inode *ovl_new_inode(struct super_block *sb, umode_t mode, - struct ovl_entry *oe); +struct inode *ovl_new_inode(struct super_block *sb, umode_t mode); static inline void ovl_copyattr(struct inode *from, struct inode *to) { to->i_uid = from->i_uid; diff --git a/fs/overlayfs/super.c b/fs/overlayfs/super.c index 058103c60f54..313f773652ff 100644 --- a/fs/overlayfs/super.c +++ b/fs/overlayfs/super.c @@ -145,25 +145,11 @@ struct dentry *ovl_dentry_real(struct dentry *dentry) return realdentry; } -struct dentry *ovl_entry_real(struct ovl_entry *oe, bool *is_upper) +static void ovl_inode_init(struct inode *inode, struct inode *realinode, + bool is_upper) { - struct dentry *realdentry; - - realdentry = ovl_upperdentry_dereference(oe); - if (realdentry) { - *is_upper = true; - } else { - realdentry = __ovl_dentry_lower(oe); - *is_upper = false; - } - return realdentry; -} - -struct inode *ovl_inode_real(struct inode *inode) -{ - bool tmp; - - return d_inode(ovl_entry_real(inode->i_private, &tmp)); + WRITE_ONCE(inode->i_private, (unsigned long) realinode | + (is_upper ? OVL_ISUPPER_MASK : 0)); } struct vfsmount *ovl_entry_mnt_real(struct ovl_entry *oe, struct inode *inode, @@ -235,7 +221,6 @@ void ovl_dentry_update(struct dentry *dentry, struct dentry *upperdentry) WARN_ON(!inode_is_locked(upperdentry->d_parent->d_inode)); WARN_ON(oe->__upperdentry); - BUG_ON(!upperdentry->d_inode); /* * Make sure upperdentry is consistent before making it visible to * ovl_upperdentry_dereference(). @@ -244,6 +229,13 @@ void ovl_dentry_update(struct dentry *dentry, struct dentry *upperdentry) oe->__upperdentry = upperdentry; } +void ovl_inode_update(struct inode *inode, struct inode *upperinode) +{ + WARN_ON(!upperinode); + WRITE_ONCE(inode->i_private, + (unsigned long) upperinode | OVL_ISUPPER_MASK); +} + void ovl_dentry_version_inc(struct dentry *dentry) { struct ovl_entry *oe = dentry->d_fsdata; @@ -574,14 +566,16 @@ struct dentry *ovl_lookup(struct inode *dir, struct dentry *dentry, if (upperdentry || ctr) { struct dentry *realdentry; + struct inode *realinode; realdentry = upperdentry ? upperdentry : stack[0].dentry; + realinode = d_inode(realdentry); err = -ENOMEM; - inode = ovl_new_inode(dentry->d_sb, realdentry->d_inode->i_mode, - oe); + inode = ovl_new_inode(dentry->d_sb, realinode->i_mode); if (!inode) goto out_free_oe; + ovl_inode_init(inode, realinode, !!upperdentry); ovl_copyattr(realdentry->d_inode, inode); } @@ -969,6 +963,7 @@ static int ovl_fill_super(struct super_block *sb, void *data, int silent) struct path upperpath = { NULL, NULL }; struct path workpath = { NULL, NULL }; struct dentry *root_dentry; + struct inode *realinode; struct ovl_entry *oe; struct ovl_fs *ufs; struct path *stack = NULL; @@ -1150,7 +1145,7 @@ static int ovl_fill_super(struct super_block *sb, void *data, int silent) if (!oe) goto out_put_cred; - root_dentry = d_make_root(ovl_new_inode(sb, S_IFDIR, oe)); + root_dentry = d_make_root(ovl_new_inode(sb, S_IFDIR)); if (!root_dentry) goto out_free_oe; @@ -1169,8 +1164,9 @@ static int ovl_fill_super(struct super_block *sb, void *data, int silent) root_dentry->d_fsdata = oe; - ovl_copyattr(ovl_dentry_real(root_dentry)->d_inode, - root_dentry->d_inode); + realinode = d_inode(ovl_dentry_real(root_dentry)); + ovl_inode_init(d_inode(root_dentry), realinode, !!upperpath.dentry); + ovl_copyattr(realinode, d_inode(root_dentry)); sb->s_magic = OVERLAYFS_SUPER_MAGIC; sb->s_op = &ovl_super_operations; From 51f7e52dc943468c6929fa0a82d4afac3c8e9636 Mon Sep 17 00:00:00 2001 From: Miklos Szeredi Date: Fri, 29 Jul 2016 12:05:24 +0200 Subject: [PATCH 14/23] ovl: share inode for hard link Inode attributes are copied up to overlay inode (uid, gid, mode, atime, mtime, ctime) so generic code using these fields works correcty. If a hard link is created in overlayfs separate inodes are allocated for each link. If chmod/chown/etc. is performed on one of the links then the inode belonging to the other ones won't be updated. This patch attempts to fix this by sharing inodes for hard links. Use inode hash (with real inode pointer as a key) to make sure overlay inodes are shared for hard links on upper. Hard links on lower are still split (which is not user observable until the copy-up happens, see Documentation/filesystems/overlayfs.txt under "Non-standard behavior"). The inode is only inserted in the hash if it is non-directoy and upper. Signed-off-by: Miklos Szeredi --- fs/overlayfs/dir.c | 84 +++++++++++++++++++++++----------------- fs/overlayfs/inode.c | 51 ++++++++++++++++++------ fs/overlayfs/overlayfs.h | 1 + fs/overlayfs/super.c | 12 +++++- 4 files changed, 100 insertions(+), 48 deletions(-) diff --git a/fs/overlayfs/dir.c b/fs/overlayfs/dir.c index 96b1bdcf3674..d456e9fb012a 100644 --- a/fs/overlayfs/dir.c +++ b/fs/overlayfs/dir.c @@ -163,12 +163,17 @@ static int ovl_dir_getattr(struct vfsmount *mnt, struct dentry *dentry, /* Common operations required to be done after creation of file on upper */ static void ovl_instantiate(struct dentry *dentry, struct inode *inode, - struct dentry *newdentry) + struct dentry *newdentry, bool hardlink) { ovl_dentry_version_inc(dentry->d_parent); ovl_dentry_update(dentry, newdentry); - ovl_inode_update(inode, d_inode(newdentry)); - ovl_copyattr(newdentry->d_inode, inode); + if (!hardlink) { + ovl_inode_update(inode, d_inode(newdentry)); + ovl_copyattr(newdentry->d_inode, inode); + } else { + WARN_ON(ovl_inode_real(inode, NULL) != d_inode(newdentry)); + inc_nlink(inode); + } d_instantiate(dentry, inode); } @@ -191,7 +196,7 @@ static int ovl_create_upper(struct dentry *dentry, struct inode *inode, if (err) goto out_dput; - ovl_instantiate(dentry, inode, newdentry); + ovl_instantiate(dentry, inode, newdentry, !!hardlink); newdentry = NULL; out_dput: dput(newdentry); @@ -361,7 +366,8 @@ static int ovl_create_over_whiteout(struct dentry *dentry, struct inode *inode, /* * mode could have been mutilated due to umask (e.g. sgid directory) */ - if (!S_ISLNK(stat->mode) && newdentry->d_inode->i_mode != stat->mode) { + if (!hardlink && + !S_ISLNK(stat->mode) && newdentry->d_inode->i_mode != stat->mode) { struct iattr attr = { .ia_valid = ATTR_MODE, .ia_mode = stat->mode, @@ -373,7 +379,7 @@ static int ovl_create_over_whiteout(struct dentry *dentry, struct inode *inode, goto out_cleanup; } - if (S_ISDIR(stat->mode)) { + if (!hardlink && S_ISDIR(stat->mode)) { err = ovl_set_opaque(newdentry); if (err) goto out_cleanup; @@ -389,7 +395,7 @@ static int ovl_create_over_whiteout(struct dentry *dentry, struct inode *inode, if (err) goto out_cleanup; } - ovl_instantiate(dentry, inode, newdentry); + ovl_instantiate(dentry, inode, newdentry, !!hardlink); newdentry = NULL; out_dput2: dput(upper); @@ -405,28 +411,17 @@ out_cleanup: goto out_dput2; } -static int ovl_create_or_link(struct dentry *dentry, int mode, dev_t rdev, - const char *link, struct dentry *hardlink) +static int ovl_create_or_link(struct dentry *dentry, struct inode *inode, + struct kstat *stat, const char *link, + struct dentry *hardlink) { int err; - struct inode *inode; const struct cred *old_cred; struct cred *override_cred; - struct kstat stat = { - .rdev = rdev, - }; - - err = -ENOMEM; - inode = ovl_new_inode(dentry->d_sb, mode); - if (!inode) - goto out; err = ovl_copy_up(dentry->d_parent); if (err) - goto out_iput; - - inode_init_owner(inode, dentry->d_parent->d_inode, mode); - stat.mode = inode->i_mode; + return err; old_cred = ovl_override_creds(dentry->d_sb); err = -ENOMEM; @@ -438,10 +433,10 @@ static int ovl_create_or_link(struct dentry *dentry, int mode, dev_t rdev, put_cred(override_cred); if (!ovl_dentry_is_opaque(dentry)) - err = ovl_create_upper(dentry, inode, &stat, link, + err = ovl_create_upper(dentry, inode, stat, link, hardlink); else - err = ovl_create_over_whiteout(dentry, inode, &stat, + err = ovl_create_over_whiteout(dentry, inode, stat, link, hardlink); } revert_creds(old_cred); @@ -451,11 +446,7 @@ static int ovl_create_or_link(struct dentry *dentry, int mode, dev_t rdev, WARN_ON(inode->i_mode != realinode->i_mode); WARN_ON(!uid_eq(inode->i_uid, realinode->i_uid)); WARN_ON(!gid_eq(inode->i_gid, realinode->i_gid)); - inode = NULL; } -out_iput: - iput(inode); -out: return err; } @@ -463,13 +454,30 @@ static int ovl_create_object(struct dentry *dentry, int mode, dev_t rdev, const char *link) { int err; + struct inode *inode; + struct kstat stat = { + .rdev = rdev, + }; err = ovl_want_write(dentry); - if (!err) { - err = ovl_create_or_link(dentry, mode, rdev, link, NULL); - ovl_drop_write(dentry); - } + if (err) + goto out; + err = -ENOMEM; + inode = ovl_new_inode(dentry->d_sb, mode); + if (!inode) + goto out_drop_write; + + inode_init_owner(inode, dentry->d_parent->d_inode, mode); + stat.mode = inode->i_mode; + + err = ovl_create_or_link(dentry, inode, &stat, link, NULL); + if (err) + iput(inode); + +out_drop_write: + ovl_drop_write(dentry); +out: return err; } @@ -504,7 +512,7 @@ static int ovl_link(struct dentry *old, struct inode *newdir, struct dentry *new) { int err; - struct dentry *upper; + struct inode *inode; err = ovl_want_write(old); if (err) @@ -514,8 +522,12 @@ static int ovl_link(struct dentry *old, struct inode *newdir, if (err) goto out_drop_write; - upper = ovl_dentry_upper(old); - err = ovl_create_or_link(new, upper->d_inode->i_mode, 0, NULL, upper); + inode = d_inode(old); + ihold(inode); + + err = ovl_create_or_link(new, inode, NULL, NULL, ovl_dentry_upper(old)); + if (err) + iput(inode); out_drop_write: ovl_drop_write(old); @@ -684,6 +696,8 @@ static int ovl_do_remove(struct dentry *dentry, bool is_dir) else err = ovl_remove_and_whiteout(dentry, is_dir); revert_creds(old_cred); + if (!err && !is_dir) + drop_nlink(dentry->d_inode); out_drop_write: ovl_drop_write(dentry); out: diff --git a/fs/overlayfs/inode.c b/fs/overlayfs/inode.c index 2bdd3cae0f71..6be0d276fd05 100644 --- a/fs/overlayfs/inode.c +++ b/fs/overlayfs/inode.c @@ -409,14 +409,8 @@ static const struct inode_operations ovl_symlink_inode_operations = { .update_time = ovl_update_time, }; -struct inode *ovl_new_inode(struct super_block *sb, umode_t mode) +static void ovl_fill_inode(struct inode *inode, umode_t mode) { - struct inode *inode; - - inode = new_inode(sb); - if (!inode) - return NULL; - inode->i_ino = get_next_ino(); inode->i_mode = mode; inode->i_flags |= S_NOCMTIME; @@ -432,6 +426,10 @@ struct inode *ovl_new_inode(struct super_block *sb, umode_t mode) inode->i_op = &ovl_symlink_inode_operations; break; + default: + WARN(1, "illegal file type: %i\n", mode); + /* Fall through */ + case S_IFREG: case S_IFSOCK: case S_IFBLK: @@ -439,11 +437,42 @@ struct inode *ovl_new_inode(struct super_block *sb, umode_t mode) case S_IFIFO: inode->i_op = &ovl_file_inode_operations; break; + } +} - default: - WARN(1, "illegal file type: %i\n", mode); - iput(inode); - inode = NULL; +struct inode *ovl_new_inode(struct super_block *sb, umode_t mode) +{ + struct inode *inode; + + inode = new_inode(sb); + if (inode) + ovl_fill_inode(inode, mode); + + return inode; +} + +static int ovl_inode_test(struct inode *inode, void *data) +{ + return ovl_inode_real(inode, NULL) == data; +} + +static int ovl_inode_set(struct inode *inode, void *data) +{ + inode->i_private = (void *) (((unsigned long) data) | OVL_ISUPPER_MASK); + return 0; +} + +struct inode *ovl_get_inode(struct super_block *sb, struct inode *realinode) + +{ + struct inode *inode; + + inode = iget5_locked(sb, (unsigned long) realinode, + ovl_inode_test, ovl_inode_set, realinode); + if (inode && inode->i_state & I_NEW) { + ovl_fill_inode(inode, realinode->i_mode); + set_nlink(inode, realinode->i_nlink); + unlock_new_inode(inode); } return inode; diff --git a/fs/overlayfs/overlayfs.h b/fs/overlayfs/overlayfs.h index 6410209ea616..abeef1e6db56 100644 --- a/fs/overlayfs/overlayfs.h +++ b/fs/overlayfs/overlayfs.h @@ -195,6 +195,7 @@ int ovl_open_maybe_copy_up(struct dentry *dentry, unsigned int file_flags); int ovl_update_time(struct inode *inode, struct timespec *ts, int flags); struct inode *ovl_new_inode(struct super_block *sb, umode_t mode); +struct inode *ovl_get_inode(struct super_block *sb, struct inode *realinode); static inline void ovl_copyattr(struct inode *from, struct inode *to) { to->i_uid = from->i_uid; diff --git a/fs/overlayfs/super.c b/fs/overlayfs/super.c index 313f773652ff..44c4510f5adf 100644 --- a/fs/overlayfs/super.c +++ b/fs/overlayfs/super.c @@ -232,8 +232,11 @@ void ovl_dentry_update(struct dentry *dentry, struct dentry *upperdentry) void ovl_inode_update(struct inode *inode, struct inode *upperinode) { WARN_ON(!upperinode); + WARN_ON(!inode_unhashed(inode)); WRITE_ONCE(inode->i_private, (unsigned long) upperinode | OVL_ISUPPER_MASK); + if (!S_ISDIR(upperinode->i_mode)) + __insert_inode_hash(inode, (unsigned long) upperinode); } void ovl_dentry_version_inc(struct dentry *dentry) @@ -572,10 +575,15 @@ struct dentry *ovl_lookup(struct inode *dir, struct dentry *dentry, realinode = d_inode(realdentry); err = -ENOMEM; - inode = ovl_new_inode(dentry->d_sb, realinode->i_mode); + if (upperdentry && !d_is_dir(upperdentry)) { + inode = ovl_get_inode(dentry->d_sb, realinode); + } else { + inode = ovl_new_inode(dentry->d_sb, realinode->i_mode); + if (inode) + ovl_inode_init(inode, realinode, !!upperdentry); + } if (!inode) goto out_free_oe; - ovl_inode_init(inode, realinode, !!upperdentry); ovl_copyattr(realdentry->d_inode, inode); } From d837a49bd57f1ec2f6411efa829fecc34002b110 Mon Sep 17 00:00:00 2001 From: Miklos Szeredi Date: Fri, 29 Jul 2016 12:05:24 +0200 Subject: [PATCH 15/23] ovl: fix POSIX ACL setting Setting POSIX ACL needs special handling: 1) Some permission checks are done by ->setxattr() which now uses mounter's creds ("ovl: do operations on underlying file system in mounter's context"). These permission checks need to be done with current cred as well. 2) Setting ACL can fail for various reasons. We do not need to copy up in these cases. In the mean time switch to using generic_setxattr. [Arnd Bergmann] Fix link error without POSIX ACL. posix_acl_from_xattr() doesn't have a 'static inline' implementation when CONFIG_FS_POSIX_ACL is disabled, and I could not come up with an obvious way to do it. This instead avoids the link error by defining two sets of ACL operations and letting the compiler drop one of the two at compile time depending on CONFIG_FS_POSIX_ACL. This avoids all references to the ACL code, also leading to smaller code. Signed-off-by: Miklos Szeredi --- fs/overlayfs/dir.c | 2 +- fs/overlayfs/inode.c | 12 +++-- fs/overlayfs/overlayfs.h | 6 +-- fs/overlayfs/super.c | 95 ++++++++++++++++++++++++++++++++++++++++ 4 files changed, 104 insertions(+), 11 deletions(-) diff --git a/fs/overlayfs/dir.c b/fs/overlayfs/dir.c index d456e9fb012a..8f95c213c0c6 100644 --- a/fs/overlayfs/dir.c +++ b/fs/overlayfs/dir.c @@ -953,7 +953,7 @@ const struct inode_operations ovl_dir_inode_operations = { .mknod = ovl_mknod, .permission = ovl_permission, .getattr = ovl_dir_getattr, - .setxattr = ovl_setxattr, + .setxattr = generic_setxattr, .getxattr = ovl_getxattr, .listxattr = ovl_listxattr, .removexattr = ovl_removexattr, diff --git a/fs/overlayfs/inode.c b/fs/overlayfs/inode.c index 6be0d276fd05..f7caf16f9bec 100644 --- a/fs/overlayfs/inode.c +++ b/fs/overlayfs/inode.c @@ -190,7 +190,9 @@ static int ovl_readlink(struct dentry *dentry, char __user *buf, int bufsiz) static bool ovl_is_private_xattr(const char *name) { - return strncmp(name, OVL_XATTR_PRE_NAME, OVL_XATTR_PRE_LEN) == 0; +#define OVL_XATTR_PRE_NAME OVL_XATTR_PREFIX "." + return strncmp(name, OVL_XATTR_PRE_NAME, + sizeof(OVL_XATTR_PRE_NAME) - 1) == 0; } int ovl_setxattr(struct dentry *dentry, struct inode *inode, @@ -205,10 +207,6 @@ int ovl_setxattr(struct dentry *dentry, struct inode *inode, if (err) goto out; - err = -EPERM; - if (ovl_is_private_xattr(name)) - goto out_drop_write; - err = ovl_copy_up(dentry); if (err) goto out_drop_write; @@ -389,7 +387,7 @@ static const struct inode_operations ovl_file_inode_operations = { .setattr = ovl_setattr, .permission = ovl_permission, .getattr = ovl_getattr, - .setxattr = ovl_setxattr, + .setxattr = generic_setxattr, .getxattr = ovl_getxattr, .listxattr = ovl_listxattr, .removexattr = ovl_removexattr, @@ -402,7 +400,7 @@ static const struct inode_operations ovl_symlink_inode_operations = { .get_link = ovl_get_link, .readlink = ovl_readlink, .getattr = ovl_getattr, - .setxattr = ovl_setxattr, + .setxattr = generic_setxattr, .getxattr = ovl_getxattr, .listxattr = ovl_listxattr, .removexattr = ovl_removexattr, diff --git a/fs/overlayfs/overlayfs.h b/fs/overlayfs/overlayfs.h index abeef1e6db56..e4f5c9536bfe 100644 --- a/fs/overlayfs/overlayfs.h +++ b/fs/overlayfs/overlayfs.h @@ -23,9 +23,9 @@ enum ovl_path_type { #define OVL_TYPE_MERGE_OR_LOWER(type) \ (OVL_TYPE_MERGE(type) || !OVL_TYPE_UPPER(type)) -#define OVL_XATTR_PRE_NAME "trusted.overlay." -#define OVL_XATTR_PRE_LEN 16 -#define OVL_XATTR_OPAQUE OVL_XATTR_PRE_NAME"opaque" + +#define OVL_XATTR_PREFIX XATTR_TRUSTED_PREFIX "overlay" +#define OVL_XATTR_OPAQUE OVL_XATTR_PREFIX ".opaque" #define OVL_ISUPPER_MASK 1UL diff --git a/fs/overlayfs/super.c b/fs/overlayfs/super.c index 44c4510f5adf..0d40f9a83c32 100644 --- a/fs/overlayfs/super.c +++ b/fs/overlayfs/super.c @@ -20,6 +20,7 @@ #include #include #include +#include #include "overlayfs.h" MODULE_AUTHOR("Miklos Szeredi "); @@ -966,6 +967,96 @@ static unsigned int ovl_split_lowerdirs(char *str) return ctr; } +static int ovl_posix_acl_xattr_set(const struct xattr_handler *handler, + struct dentry *dentry, struct inode *inode, + const char *name, const void *value, + size_t size, int flags) +{ + struct dentry *workdir = ovl_workdir(dentry); + struct inode *realinode = ovl_inode_real(inode, NULL); + struct posix_acl *acl = NULL; + int err; + + /* Check that everything is OK before copy-up */ + if (value) { + acl = posix_acl_from_xattr(&init_user_ns, value, size); + if (IS_ERR(acl)) + return PTR_ERR(acl); + } + err = -EOPNOTSUPP; + if (!IS_POSIXACL(d_inode(workdir))) + goto out_acl_release; + if (!realinode->i_op->set_acl) + goto out_acl_release; + if (handler->flags == ACL_TYPE_DEFAULT && !S_ISDIR(inode->i_mode)) { + err = acl ? -EACCES : 0; + goto out_acl_release; + } + err = -EPERM; + if (!inode_owner_or_capable(inode)) + goto out_acl_release; + + posix_acl_release(acl); + + return ovl_setxattr(dentry, inode, handler->name, value, size, flags); + +out_acl_release: + posix_acl_release(acl); + return err; +} + +static int ovl_other_xattr_set(const struct xattr_handler *handler, + struct dentry *dentry, struct inode *inode, + const char *name, const void *value, + size_t size, int flags) +{ + return ovl_setxattr(dentry, inode, name, value, size, flags); +} + +static int ovl_own_xattr_set(const struct xattr_handler *handler, + struct dentry *dentry, struct inode *inode, + const char *name, const void *value, + size_t size, int flags) +{ + return -EPERM; +} + +static const struct xattr_handler ovl_posix_acl_access_xattr_handler = { + .name = XATTR_NAME_POSIX_ACL_ACCESS, + .flags = ACL_TYPE_ACCESS, + .set = ovl_posix_acl_xattr_set, +}; + +static const struct xattr_handler ovl_posix_acl_default_xattr_handler = { + .name = XATTR_NAME_POSIX_ACL_DEFAULT, + .flags = ACL_TYPE_DEFAULT, + .set = ovl_posix_acl_xattr_set, +}; + +static const struct xattr_handler ovl_own_xattr_handler = { + .prefix = OVL_XATTR_PREFIX, + .set = ovl_own_xattr_set, +}; + +static const struct xattr_handler ovl_other_xattr_handler = { + .prefix = "", /* catch all */ + .set = ovl_other_xattr_set, +}; + +static const struct xattr_handler *ovl_xattr_handlers[] = { + &ovl_posix_acl_access_xattr_handler, + &ovl_posix_acl_default_xattr_handler, + &ovl_own_xattr_handler, + &ovl_other_xattr_handler, + NULL +}; + +static const struct xattr_handler *ovl_xattr_noacl_handlers[] = { + &ovl_own_xattr_handler, + &ovl_other_xattr_handler, + NULL, +}; + static int ovl_fill_super(struct super_block *sb, void *data, int silent) { struct path upperpath = { NULL, NULL }; @@ -1178,6 +1269,10 @@ static int ovl_fill_super(struct super_block *sb, void *data, int silent) sb->s_magic = OVERLAYFS_SUPER_MAGIC; sb->s_op = &ovl_super_operations; + if (IS_ENABLED(CONFIG_FS_POSIX_ACL)) + sb->s_xattr = ovl_xattr_handlers; + else + sb->s_xattr = ovl_xattr_noacl_handlers; sb->s_root = root_dentry; sb->s_fs_info = ufs; sb->s_flags |= MS_POSIXACL; From e29841a0ab3d03e77313abd8fb4c16e80fb26e29 Mon Sep 17 00:00:00 2001 From: Vivek Goyal Date: Wed, 13 Jul 2016 11:00:14 -0400 Subject: [PATCH 16/23] ovl: dilute permission checks on lower only if not special file Right now if file is on lower/, we remove MAY_WRITE/MAY_APPEND bits from mask as lower/ will never be written and file will be copied up. But this is not true for special files. These files are not copied up and are opened in place. So don't dilute the checks for these types of files. Reported-by: Dan Walsh Signed-off-by: Vivek Goyal Signed-off-by: Miklos Szeredi --- fs/overlayfs/inode.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/fs/overlayfs/inode.c b/fs/overlayfs/inode.c index f7caf16f9bec..76cfe9d04e64 100644 --- a/fs/overlayfs/inode.c +++ b/fs/overlayfs/inode.c @@ -137,7 +137,7 @@ int ovl_permission(struct inode *inode, int mask) return err; old_cred = ovl_override_creds(inode->i_sb); - if (!is_upper) + if (!is_upper && !special_file(realinode->i_mode)) mask &= ~(MAY_WRITE | MAY_APPEND); err = inode_permission(realinode, mask); revert_creds(old_cred); From 500cac3ccee65526d5075da3af2674101305bf8c Mon Sep 17 00:00:00 2001 From: Vivek Goyal Date: Wed, 13 Jul 2016 11:00:14 -0400 Subject: [PATCH 17/23] ovl: append MAY_READ when diluting write checks Right now we remove MAY_WRITE/MAY_APPEND bits from mask if realfile is on lower/. This is done as files on lower will never be written and will be copied up. But to copy up a file, mounter should have MAY_READ permission otherwise copy up will fail. So set MAY_READ in mask when MAY_WRITE is reset. Dan Walsh noticed this when he did access(lowerfile, W_OK) and it returned True (context mounts) but when he tried to actually write to file, it failed as mounter did not have permission on lower file. [SzM] don't set MAY_READ if only MAY_APPEND is set without MAY_WRITE; this won't trigger a copy-up. Reported-by: Dan Walsh Signed-off-by: Vivek Goyal Signed-off-by: Miklos Szeredi --- fs/overlayfs/inode.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/fs/overlayfs/inode.c b/fs/overlayfs/inode.c index 76cfe9d04e64..1b885c156028 100644 --- a/fs/overlayfs/inode.c +++ b/fs/overlayfs/inode.c @@ -137,8 +137,11 @@ int ovl_permission(struct inode *inode, int mask) return err; old_cred = ovl_override_creds(inode->i_sb); - if (!is_upper && !special_file(realinode->i_mode)) + if (!is_upper && !special_file(realinode->i_mode) && mask & MAY_WRITE) { mask &= ~(MAY_WRITE | MAY_APPEND); + /* Make sure mounter can read file for copy up later */ + mask |= MAY_READ; + } err = inode_permission(realinode, mask); revert_creds(old_cred); From 5f215013a9a17904b9103bb5f499cecc89df55bd Mon Sep 17 00:00:00 2001 From: Wei Yongjun Date: Wed, 6 Jul 2016 12:27:15 +0000 Subject: [PATCH 18/23] ovl: remove duplicated include from super.c Remove duplicated include. Signed-off-by: Wei Yongjun Signed-off-by: Miklos Szeredi --- fs/overlayfs/super.c | 1 - 1 file changed, 1 deletion(-) diff --git a/fs/overlayfs/super.c b/fs/overlayfs/super.c index 0d40f9a83c32..fa997bdd951d 100644 --- a/fs/overlayfs/super.c +++ b/fs/overlayfs/super.c @@ -16,7 +16,6 @@ #include #include #include -#include #include #include #include From 656189d207f0ed267f41338f7ff86f98e420099f Mon Sep 17 00:00:00 2001 From: Miklos Szeredi Date: Fri, 29 Jul 2016 12:05:24 +0200 Subject: [PATCH 19/23] ovl: fix warning There's a superfluous newline in the warning message in ovl_d_real(). Signed-off-by: Miklos Szeredi --- fs/overlayfs/super.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/fs/overlayfs/super.c b/fs/overlayfs/super.c index fa997bdd951d..7fd216a89d9a 100644 --- a/fs/overlayfs/super.c +++ b/fs/overlayfs/super.c @@ -335,7 +335,7 @@ static struct dentry *ovl_d_real(struct dentry *dentry, /* Handle recursion */ return d_real(real, inode, open_flags); bug: - WARN(1, "ovl_d_real(%pd4, %s:%lu\n): real dentry not found\n", dentry, + WARN(1, "ovl_d_real(%pd4, %s:%lu): real dentry not found\n", dentry, inode ? inode->i_sb->s_id : "NULL", inode ? inode->i_ino : 0); return dentry; } From 76bc8e2843b66f8205026365966b49ec6da39ae7 Mon Sep 17 00:00:00 2001 From: Miklos Szeredi Date: Fri, 29 Jul 2016 12:05:24 +0200 Subject: [PATCH 20/23] ovl: disallow overlayfs as upperdir This does not work and does not make sense. So instead of fixing it (probably not hard) just disallow. Reported-by: Andrei Vagin Signed-off-by: Miklos Szeredi Cc: --- fs/overlayfs/super.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/fs/overlayfs/super.c b/fs/overlayfs/super.c index 7fd216a89d9a..f3577395eca5 100644 --- a/fs/overlayfs/super.c +++ b/fs/overlayfs/super.c @@ -407,7 +407,8 @@ static struct ovl_entry *ovl_alloc_entry(unsigned int numlower) static bool ovl_dentry_remote(struct dentry *dentry) { return dentry->d_flags & - (DCACHE_OP_REVALIDATE | DCACHE_OP_WEAK_REVALIDATE); + (DCACHE_OP_REVALIDATE | DCACHE_OP_WEAK_REVALIDATE | + DCACHE_OP_REAL); } static bool ovl_dentry_weird(struct dentry *dentry) From dbc816d05ddcfb189af8784d04fc84c812db3747 Mon Sep 17 00:00:00 2001 From: Miklos Szeredi Date: Fri, 29 Jul 2016 12:05:24 +0200 Subject: [PATCH 21/23] ovl: clear nlink on rmdir To make delete notification work on fa/inotify. Signed-off-by: Miklos Szeredi --- fs/overlayfs/dir.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/fs/overlayfs/dir.c b/fs/overlayfs/dir.c index 8f95c213c0c6..ddda948109d3 100644 --- a/fs/overlayfs/dir.c +++ b/fs/overlayfs/dir.c @@ -696,8 +696,12 @@ static int ovl_do_remove(struct dentry *dentry, bool is_dir) else err = ovl_remove_and_whiteout(dentry, is_dir); revert_creds(old_cred); - if (!err && !is_dir) - drop_nlink(dentry->d_inode); + if (!err) { + if (is_dir) + clear_nlink(dentry->d_inode); + else + drop_nlink(dentry->d_inode); + } out_drop_write: ovl_drop_write(dentry); out: From 29c42e80ba5b1e59a4f427b44e2bdebd347b9409 Mon Sep 17 00:00:00 2001 From: Al Viro Date: Wed, 20 Jul 2016 22:36:53 -0400 Subject: [PATCH 22/23] qstr: constify instances in overlayfs Signed-off-by: Al Viro Signed-off-by: Miklos Szeredi --- fs/overlayfs/super.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/fs/overlayfs/super.c b/fs/overlayfs/super.c index f3577395eca5..4036132842b5 100644 --- a/fs/overlayfs/super.c +++ b/fs/overlayfs/super.c @@ -421,7 +421,7 @@ static bool ovl_dentry_weird(struct dentry *dentry) static inline struct dentry *ovl_lookup_real(struct super_block *ovl_sb, struct dentry *dir, - struct qstr *name) + const struct qstr *name) { const struct cred *old_cred; struct dentry *dentry; From 30c17ebfb2a11468fe825de19afa3934ee98bfd2 Mon Sep 17 00:00:00 2001 From: Miklos Szeredi Date: Fri, 29 Jul 2016 12:05:25 +0200 Subject: [PATCH 23/23] ovl: simplify empty checking The empty checking logic is duplicated in ovl_check_empty_and_clear() and ovl_remove_and_whiteout(), except the condition for clearing whiteouts is different: ovl_check_empty_and_clear() checked for being upper ovl_remove_and_whiteout() checked for merge OR lower Move the intersection of those checks (upper AND merge) into ovl_check_empty_and_clear() and simplify ovl_remove_and_whiteout(). Signed-off-by: Miklos Szeredi --- fs/overlayfs/dir.c | 50 +++++++++++++++++++--------------------------- 1 file changed, 21 insertions(+), 29 deletions(-) diff --git a/fs/overlayfs/dir.c b/fs/overlayfs/dir.c index ddda948109d3..12bcd07b9e32 100644 --- a/fs/overlayfs/dir.c +++ b/fs/overlayfs/dir.c @@ -307,23 +307,29 @@ static struct dentry *ovl_check_empty_and_clear(struct dentry *dentry) { int err; struct dentry *ret = NULL; + enum ovl_path_type type = ovl_path_type(dentry); LIST_HEAD(list); err = ovl_check_empty_dir(dentry, &list); - if (err) + if (err) { ret = ERR_PTR(err); - else { - /* - * If no upperdentry then skip clearing whiteouts. - * - * Can race with copy-up, since we don't hold the upperdir - * mutex. Doesn't matter, since copy-up can't create a - * non-empty directory from an empty one. - */ - if (ovl_dentry_upper(dentry)) - ret = ovl_clear_empty(dentry, &list); + goto out_free; } + /* + * When removing an empty opaque directory, then it makes no sense to + * replace it with an exact replica of itself. + * + * If no upperdentry then skip clearing whiteouts. + * + * Can race with copy-up, since we don't hold the upperdir mutex. + * Doesn't matter, since copy-up can't create a non-empty directory + * from an empty one. + */ + if (OVL_TYPE_UPPER(type) && OVL_TYPE_MERGE(type)) + ret = ovl_clear_empty(dentry, &list); + +out_free: ovl_cache_free(&list); return ret; @@ -551,24 +557,10 @@ static int ovl_remove_and_whiteout(struct dentry *dentry, bool is_dir) return -EROFS; if (is_dir) { - if (OVL_TYPE_MERGE_OR_LOWER(ovl_path_type(dentry))) { - opaquedir = ovl_check_empty_and_clear(dentry); - err = PTR_ERR(opaquedir); - if (IS_ERR(opaquedir)) - goto out; - } else { - LIST_HEAD(list); - - /* - * When removing an empty opaque directory, then it - * makes no sense to replace it with an exact replica of - * itself. But emptiness still needs to be checked. - */ - err = ovl_check_empty_dir(dentry, &list); - ovl_cache_free(&list); - if (err) - goto out; - } + opaquedir = ovl_check_empty_and_clear(dentry); + err = PTR_ERR(opaquedir); + if (IS_ERR(opaquedir)) + goto out; } err = ovl_lock_rename_workdir(workdir, upperdir);