ovl: use wrappers to all vfs_*xattr() calls

[ Upstream commit c914c0e27eb0843b7cf3bec71d6f34d53a3a671e ]

Use helpers ovl_*xattr() to access user/trusted.overlay.* xattrs
and use helpers ovl_do_*xattr() to access generic xattrs. This is a
preparatory patch for using idmapped base layers with overlay.

Note that a few of those places called vfs_*xattr() calls directly to
reduce the amount of debug output. But as Miklos pointed out since
overlayfs has been stable for quite some time the debug output isn't all
that relevant anymore and the additional debug in all locations was
actually quite helpful when developing this patch series.

Cc: <linux-unionfs@vger.kernel.org>
Tested-by: Giuseppe Scrivano <gscrivan@redhat.com>
Reviewed-by: Christian Brauner (Microsoft) <brauner@kernel.org>
Signed-off-by: Amir Goldstein <amir73il@gmail.com>
Signed-off-by: Christian Brauner (Microsoft) <brauner@kernel.org>
Signed-off-by: Miklos Szeredi <mszeredi@redhat.com>
Stable-dep-of: c84e125fff26 ("ovl: fix UAF in ovl_dentry_update_reval by moving dput() in ovl_link_up")
Signed-off-by: Sasha Levin <sashal@kernel.org>

diff --git a/fs/overlayfs/copy_up.c b/fs/overlayfs/copy_up.c
index 0ed70eff9cb9e825f178245d4bb8e138b07a4b9d..80e7ae8152fdd8204bfee7b2c9c26a92307e0890 100644 (file)
--- a/fs/overlayfs/copy_up.c
+++ b/fs/overlayfs/copy_up.c
@@ -117,7 +117,7 @@ retry:
                        goto retry;
                }
 
-               error = vfs_setxattr(&init_user_ns, new, name, value, size, 0);
+               error = ovl_do_setxattr(OVL_FS(sb), new, name, value, size, 0);
                if (error) {
                        if (error != -EOPNOTSUPP || ovl_must_copy_xattr(name))
                                break;
@@ -433,7 +433,7 @@ static int ovl_set_upper_fh(struct ovl_fs *ofs, struct dentry *upper,
        if (IS_ERR(fh))
                return PTR_ERR(fh);
 
-       err = ovl_do_setxattr(ofs, index, OVL_XATTR_UPPER, fh->buf, fh->fb.len);
+       err = ovl_setxattr(ofs, index, OVL_XATTR_UPPER, fh->buf, fh->fb.len);
 
        kfree(fh);
        return err;
@@ -868,12 +868,13 @@ static bool ovl_need_meta_copy_up(struct dentry *dentry, umode_t mode,
        return true;
 }
 
-static ssize_t ovl_getxattr(struct dentry *dentry, char *name, char **value)
+static ssize_t ovl_getxattr_value(struct ovl_fs *ofs, struct dentry *dentry,
+                                 char *name, char **value)
 {
        ssize_t res;
        char *buf;
 
-       res = vfs_getxattr(&init_user_ns, dentry, name, NULL, 0);
+       res = ovl_do_getxattr(ofs, dentry, name, NULL, 0);
        if (res == -ENODATA || res == -EOPNOTSUPP)
                res = 0;
 
@@ -882,7 +883,7 @@ static ssize_t ovl_getxattr(struct dentry *dentry, char *name, char **value)
                if (!buf)
                        return -ENOMEM;
 
-               res = vfs_getxattr(&init_user_ns, dentry, name, buf, res);
+               res = ovl_do_getxattr(ofs, dentry, name, buf, res);
                if (res < 0)
                        kfree(buf);
                else
@@ -909,8 +910,8 @@ static int ovl_copy_up_meta_inode_data(struct ovl_copy_up_ctx *c)
                return -EIO;
 
        if (c->stat.size) {
-               err = cap_size = ovl_getxattr(upperpath.dentry, XATTR_NAME_CAPS,
-                                             &capability);
+               err = cap_size = ovl_getxattr_value(ofs, upperpath.dentry,
+                                                   XATTR_NAME_CAPS, &capability);
                if (cap_size < 0)
                        goto out;
        }
@@ -924,14 +925,14 @@ static int ovl_copy_up_meta_inode_data(struct ovl_copy_up_ctx *c)
         * don't want that to happen for normal copy-up operation.
         */
        if (capability) {
-               err = vfs_setxattr(&init_user_ns, upperpath.dentry,
-                                  XATTR_NAME_CAPS, capability, cap_size, 0);
+               err = ovl_do_setxattr(ofs, upperpath.dentry, XATTR_NAME_CAPS,
+                                     capability, cap_size, 0);
                if (err)
                        goto out_free;
        }
 
 
-       err = ovl_do_removexattr(ofs, upperpath.dentry, OVL_XATTR_METACOPY);
+       err = ovl_removexattr(ofs, upperpath.dentry, OVL_XATTR_METACOPY);
        if (err)
                goto out_free;
 

diff --git a/fs/overlayfs/dir.c b/fs/overlayfs/dir.c
index 519193ce7d575febbd5011046626a47b705f07b1..5e9005a0afaade3b88480ed02cfc1215b24214bf 100644 (file)
--- a/fs/overlayfs/dir.c
+++ b/fs/overlayfs/dir.c
@@ -431,8 +431,8 @@ out:
        return ERR_PTR(err);
 }
 
-static int ovl_set_upper_acl(struct dentry *upperdentry, const char *name,
-                            const struct posix_acl *acl)
+static int ovl_set_upper_acl(struct ovl_fs *ofs, struct dentry *upperdentry,
+                            const char *name, const struct posix_acl *acl)
 {
        void *buffer;
        size_t size;
@@ -450,7 +450,7 @@ static int ovl_set_upper_acl(struct dentry *upperdentry, const char *name,
        if (err < 0)
                goto out_free;
 
-       err = vfs_setxattr(&init_user_ns, upperdentry, name, buffer, size, XATTR_CREATE);
+       err = ovl_do_setxattr(ofs, upperdentry, name, buffer, size, XATTR_CREATE);
 out_free:
        kfree(buffer);
        return err;
@@ -459,6 +459,7 @@ out_free:
 static int ovl_create_over_whiteout(struct dentry *dentry, struct inode *inode,
                                    struct ovl_cattr *cattr)
 {
+       struct ovl_fs *ofs = OVL_FS(dentry->d_sb);
        struct dentry *workdir = ovl_workdir(dentry);
        struct inode *wdir = workdir->d_inode;
        struct dentry *upperdir = ovl_dentry_upper(dentry->d_parent);
@@ -515,13 +516,13 @@ static int ovl_create_over_whiteout(struct dentry *dentry, struct inode *inode,
                        goto out_cleanup;
        }
        if (!hardlink) {
-               err = ovl_set_upper_acl(newdentry, XATTR_NAME_POSIX_ACL_ACCESS,
-                                       acl);
+               err = ovl_set_upper_acl(ofs, newdentry,
+                                       XATTR_NAME_POSIX_ACL_ACCESS, acl);
                if (err)
                        goto out_cleanup;
 
-               err = ovl_set_upper_acl(newdentry, XATTR_NAME_POSIX_ACL_DEFAULT,
-                                       default_acl);
+               err = ovl_set_upper_acl(ofs, newdentry,
+                                       XATTR_NAME_POSIX_ACL_DEFAULT, default_acl);
                if (err)
                        goto out_cleanup;
        }

diff --git a/fs/overlayfs/inode.c b/fs/overlayfs/inode.c
index 7961d6888c52050ed3edbe2984c1faf962708ad3..aa8513aac4728b230402f44e220f77cc3b68951d 100644 (file)
--- a/fs/overlayfs/inode.c
+++ b/fs/overlayfs/inode.c
@@ -342,6 +342,7 @@ int ovl_xattr_set(struct dentry *dentry, struct inode *inode, const char *name,
                  const void *value, size_t size, int flags)
 {
        int err;
+       struct ovl_fs *ofs = OVL_FS(dentry->d_sb);
        struct dentry *upperdentry = ovl_i_dentry_upper(inode);
        struct dentry *realdentry = upperdentry ?: ovl_dentry_lower(dentry);
        const struct cred *old_cred;
@@ -367,12 +368,12 @@ int ovl_xattr_set(struct dentry *dentry, struct inode *inode, const char *name,
        }
 
        old_cred = ovl_override_creds(dentry->d_sb);
-       if (value)
-               err = vfs_setxattr(&init_user_ns, realdentry, name, value, size,
-                                  flags);
-       else {
+       if (value) {
+               err = ovl_do_setxattr(ofs, realdentry, name, value, size,
+                                     flags);
+       } else {
                WARN_ON(flags != XATTR_REPLACE);
-               err = vfs_removexattr(&init_user_ns, realdentry, name);
+               err = ovl_do_removexattr(ofs, realdentry, name);
        }
        revert_creds(old_cred);
 
@@ -887,8 +888,8 @@ static int ovl_set_nlink_common(struct dentry *dentry,
        if (WARN_ON(len >= sizeof(buf)))
                return -EIO;
 
-       return ovl_do_setxattr(OVL_FS(inode->i_sb), ovl_dentry_upper(dentry),
-                              OVL_XATTR_NLINK, buf, len);
+       return ovl_setxattr(OVL_FS(inode->i_sb), ovl_dentry_upper(dentry),
+                           OVL_XATTR_NLINK, buf, len);
 }
 
 int ovl_set_nlink_upper(struct dentry *dentry)
@@ -913,7 +914,7 @@ unsigned int ovl_get_nlink(struct ovl_fs *ofs, struct dentry *lowerdentry,
        if (!lowerdentry || !upperdentry || d_inode(lowerdentry)->i_nlink == 1)
                return fallback;
 
-       err = ovl_do_getxattr(ofs, upperdentry, OVL_XATTR_NLINK,
+       err = ovl_getxattr(ofs, upperdentry, OVL_XATTR_NLINK,
                              &buf, sizeof(buf) - 1);
        if (err < 0)
                goto fail;

diff --git a/fs/overlayfs/namei.c b/fs/overlayfs/namei.c
index 9c055d11a95de9f950b0cae089f4366585faf2eb..00d74311aa0d3983fe6b8c5c3489c56abe153f63 100644 (file)
--- a/fs/overlayfs/namei.c
+++ b/fs/overlayfs/namei.c
@@ -111,7 +111,7 @@ static struct ovl_fh *ovl_get_fh(struct ovl_fs *ofs, struct dentry *dentry,
        int res, err;
        struct ovl_fh *fh = NULL;
 
-       res = ovl_do_getxattr(ofs, dentry, ox, NULL, 0);
+       res = ovl_getxattr(ofs, dentry, ox, NULL, 0);
        if (res < 0) {
                if (res == -ENODATA || res == -EOPNOTSUPP)
                        return NULL;
@@ -125,7 +125,7 @@ static struct ovl_fh *ovl_get_fh(struct ovl_fs *ofs, struct dentry *dentry,
        if (!fh)
                return ERR_PTR(-ENOMEM);
 
-       res = ovl_do_getxattr(ofs, dentry, ox, fh->buf, res);
+       res = ovl_getxattr(ofs, dentry, ox, fh->buf, res);
        if (res < 0)
                goto fail;
 
@@ -464,7 +464,7 @@ int ovl_verify_set_fh(struct ovl_fs *ofs, struct dentry *dentry,
 
        err = ovl_verify_fh(ofs, dentry, ox, fh);
        if (set && err == -ENODATA)
-               err = ovl_do_setxattr(ofs, dentry, ox, fh->buf, fh->fb.len);
+               err = ovl_setxattr(ofs, dentry, ox, fh->buf, fh->fb.len);
        if (err)
                goto fail;
 

diff --git a/fs/overlayfs/overlayfs.h b/fs/overlayfs/overlayfs.h
index a96b67586f817d9b1943ea1b16699c4221098d58..3f4655b9c71ca12bf71ef987be10e08e330f2c43 100644 (file)
--- a/fs/overlayfs/overlayfs.h
+++ b/fs/overlayfs/overlayfs.h
@@ -183,10 +183,9 @@ static inline int ovl_do_symlink(struct inode *dir, struct dentry *dentry,
 }
 
 static inline ssize_t ovl_do_getxattr(struct ovl_fs *ofs, struct dentry *dentry,
-                                     enum ovl_xattr ox, void *value,
+                                     const char *name, void *value,
                                      size_t size)
 {
-       const char *name = ovl_xattr(ofs, ox);
        int err = vfs_getxattr(&init_user_ns, dentry, name, value, size);
        int len = (value && err > 0) ? err : 0;
 
@@ -195,26 +194,45 @@ static inline ssize_t ovl_do_getxattr(struct ovl_fs *ofs, struct dentry *dentry,
        return err;
 }
 
+static inline ssize_t ovl_getxattr(struct ovl_fs *ofs, struct dentry *dentry,
+                                  enum ovl_xattr ox, void *value,
+                                  size_t size)
+{
+       return ovl_do_getxattr(ofs, dentry, ovl_xattr(ofs, ox), value, size);
+}
+
 static inline int ovl_do_setxattr(struct ovl_fs *ofs, struct dentry *dentry,
-                                 enum ovl_xattr ox, const void *value,
-                                 size_t size)
+                                 const char *name, const void *value,
+                                 size_t size, int flags)
 {
-       const char *name = ovl_xattr(ofs, ox);
-       int err = vfs_setxattr(&init_user_ns, dentry, name, value, size, 0);
-       pr_debug("setxattr(%pd2, \"%s\", \"%*pE\", %zu, 0) = %i\n",
-                dentry, name, min((int)size, 48), value, size, err);
+       int err = vfs_setxattr(&init_user_ns, dentry, name, value, size, flags);
+
+       pr_debug("setxattr(%pd2, \"%s\", \"%*pE\", %zu, %d) = %i\n",
+                dentry, name, min((int)size, 48), value, size, flags, err);
        return err;
 }
 
+static inline int ovl_setxattr(struct ovl_fs *ofs, struct dentry *dentry,
+                              enum ovl_xattr ox, const void *value,
+                              size_t size)
+{
+       return ovl_do_setxattr(ofs, dentry, ovl_xattr(ofs, ox), value, size, 0);
+}
+
 static inline int ovl_do_removexattr(struct ovl_fs *ofs, struct dentry *dentry,
-                                    enum ovl_xattr ox)
+                                    const char *name)
 {
-       const char *name = ovl_xattr(ofs, ox);
        int err = vfs_removexattr(&init_user_ns, dentry, name);
        pr_debug("removexattr(%pd2, \"%s\") = %i\n", dentry, name, err);
        return err;
 }
 
+static inline int ovl_removexattr(struct ovl_fs *ofs, struct dentry *dentry,
+                                 enum ovl_xattr ox)
+{
+       return ovl_do_removexattr(ofs, dentry, ovl_xattr(ofs, ox));
+}
+
 static inline int ovl_do_rename(struct inode *olddir, struct dentry *olddentry,
                                struct inode *newdir, struct dentry *newdentry,
                                unsigned int flags)

diff --git a/fs/overlayfs/readdir.c b/fs/overlayfs/readdir.c
index 150fdf3bc68d4c812fe45a886c724e58c56f84aa..c7b542331065c4325ee0c3a3ebb50bdaf9068bc9 100644 (file)
--- a/fs/overlayfs/readdir.c
+++ b/fs/overlayfs/readdir.c
@@ -623,8 +623,8 @@ static struct ovl_dir_cache *ovl_cache_get_impure(struct path *path)
                 * Removing the "impure" xattr is best effort.
                 */
                if (!ovl_want_write(dentry)) {
-                       ovl_do_removexattr(ofs, ovl_dentry_upper(dentry),
-                                          OVL_XATTR_IMPURE);
+                       ovl_removexattr(ofs, ovl_dentry_upper(dentry),
+                                       OVL_XATTR_IMPURE);
                        ovl_drop_write(dentry);
                }
                ovl_clear_flag(OVL_IMPURE, d_inode(dentry));

diff --git a/fs/overlayfs/super.c b/fs/overlayfs/super.c
index 5310271cf2e38c6e8dd1fc407d71ffdf9d601b1a..1dad3dabe0099a0d85e8f8e1e46094c7616a5af3 100644 (file)
--- a/fs/overlayfs/super.c
+++ b/fs/overlayfs/super.c
@@ -815,13 +815,13 @@ retry:
                 * allowed as upper are limited to "normal" ones, where checking
                 * for the above two errors is sufficient.
                 */
-               err = vfs_removexattr(&init_user_ns, work,
-                                     XATTR_NAME_POSIX_ACL_DEFAULT);
+               err = ovl_do_removexattr(ofs, work,
+                                        XATTR_NAME_POSIX_ACL_DEFAULT);
                if (err && err != -ENODATA && err != -EOPNOTSUPP)
                        goto out_dput;
 
-               err = vfs_removexattr(&init_user_ns, work,
-                                     XATTR_NAME_POSIX_ACL_ACCESS);
+               err = ovl_do_removexattr(ofs, work,
+                                        XATTR_NAME_POSIX_ACL_ACCESS);
                if (err && err != -ENODATA && err != -EOPNOTSUPP)
                        goto out_dput;
 
@@ -1417,7 +1417,7 @@ static int ovl_make_workdir(struct super_block *sb, struct ovl_fs *ofs,
        /*
         * Check if upper/work fs supports (trusted|user).overlay.* xattr
         */
-       err = ovl_do_setxattr(ofs, ofs->workdir, OVL_XATTR_OPAQUE, "0", 1);
+       err = ovl_setxattr(ofs, ofs->workdir, OVL_XATTR_OPAQUE, "0", 1);
        if (err) {
                pr_warn("failed to set xattr on upper\n");
                ofs->noxattr = true;
@@ -1438,7 +1438,7 @@ static int ovl_make_workdir(struct super_block *sb, struct ovl_fs *ofs,
                        pr_info("try mounting with 'userxattr' option\n");
                err = 0;
        } else {
-               ovl_do_removexattr(ofs, ofs->workdir, OVL_XATTR_OPAQUE);
+               ovl_removexattr(ofs, ofs->workdir, OVL_XATTR_OPAQUE);
        }
 
        /*

diff --git a/fs/overlayfs/util.c b/fs/overlayfs/util.c
index 747b47048b3aa851371e2b02ee1f9523169ae37d..eea9ec8f8c57a21c2e8c44b9e747527b4f87e675 100644 (file)
--- a/fs/overlayfs/util.c
+++ b/fs/overlayfs/util.c
@@ -586,7 +586,7 @@ bool ovl_check_origin_xattr(struct ovl_fs *ofs, struct dentry *dentry)
 {
        int res;
 
-       res = ovl_do_getxattr(ofs, dentry, OVL_XATTR_ORIGIN, NULL, 0);
+       res = ovl_getxattr(ofs, dentry, OVL_XATTR_ORIGIN, NULL, 0);
 
        /* Zero size value means "copied up but origin unknown" */
        if (res >= 0)
@@ -604,7 +604,7 @@ bool ovl_check_dir_xattr(struct super_block *sb, struct dentry *dentry,
        if (!d_is_dir(dentry))
                return false;
 
-       res = ovl_do_getxattr(OVL_FS(sb), dentry, ox, &val, 1);
+       res = ovl_getxattr(OVL_FS(sb), dentry, ox, &val, 1);
        if (res == 1 && val == 'y')
                return true;
 
@@ -644,7 +644,7 @@ int ovl_check_setxattr(struct ovl_fs *ofs, struct dentry *upperdentry,
        if (ofs->noxattr)
                return xerr;
 
-       err = ovl_do_setxattr(ofs, upperdentry, ox, value, size);
+       err = ovl_setxattr(ofs, upperdentry, ox, value, size);
 
        if (err == -EOPNOTSUPP) {
                pr_warn("cannot set %s xattr on upper\n", ovl_xattr(ofs, ox));
@@ -684,7 +684,7 @@ void ovl_check_protattr(struct inode *inode, struct dentry *upper)
        char buf[OVL_PROTATTR_MAX+1];
        int res, n;
 
-       res = ovl_do_getxattr(ofs, upper, OVL_XATTR_PROTATTR, buf,
+       res = ovl_getxattr(ofs, upper, OVL_XATTR_PROTATTR, buf,
                              OVL_PROTATTR_MAX);
        if (res < 0)
                return;
@@ -740,7 +740,7 @@ int ovl_set_protattr(struct inode *inode, struct dentry *upper,
                err = ovl_check_setxattr(ofs, upper, OVL_XATTR_PROTATTR,
                                         buf, len, -EPERM);
        } else if (inode->i_flags & OVL_PROT_I_FLAGS_MASK) {
-               err = ovl_do_removexattr(ofs, upper, OVL_XATTR_PROTATTR);
+               err = ovl_removexattr(ofs, upper, OVL_XATTR_PROTATTR);
                if (err == -EOPNOTSUPP || err == -ENODATA)
                        err = 0;
        }
@@ -983,7 +983,7 @@ int ovl_check_metacopy_xattr(struct ovl_fs *ofs, struct dentry *dentry)
        if (!S_ISREG(d_inode(dentry)->i_mode))
                return 0;
 
-       res = ovl_do_getxattr(ofs, dentry, OVL_XATTR_METACOPY, NULL, 0);
+       res = ovl_getxattr(ofs, dentry, OVL_XATTR_METACOPY, NULL, 0);
        if (res < 0) {
                if (res == -ENODATA || res == -EOPNOTSUPP)
                        return 0;
@@ -1025,7 +1025,7 @@ char *ovl_get_redirect_xattr(struct ovl_fs *ofs, struct dentry *dentry,
        int res;
        char *s, *next, *buf = NULL;
 
-       res = ovl_do_getxattr(ofs, dentry, OVL_XATTR_REDIRECT, NULL, 0);
+       res = ovl_getxattr(ofs, dentry, OVL_XATTR_REDIRECT, NULL, 0);
        if (res == -ENODATA || res == -EOPNOTSUPP)
                return NULL;
        if (res < 0)
@@ -1037,7 +1037,7 @@ char *ovl_get_redirect_xattr(struct ovl_fs *ofs, struct dentry *dentry,
        if (!buf)
                return ERR_PTR(-ENOMEM);
 
-       res = ovl_do_getxattr(ofs, dentry, OVL_XATTR_REDIRECT, buf, res);
+       res = ovl_getxattr(ofs, dentry, OVL_XATTR_REDIRECT, buf, res);
        if (res < 0)
                goto fail;
        if (res == 0)