ovl: Do not lose security.capability xattr over metadata file copy-up

commit 993a0b2aec52754f0897b1dab4c453be8217cae5 upstream.

If a file has been copied up metadata only, and later data is copied up,
upper loses any security.capability xattr it has (underlying filesystem
clears it as upon file write).

From a user's point of view, this is just a file copy-up and that should
not result in losing security.capability xattr.  Hence, before data copy
up, save security.capability xattr (if any) and restore it on upper after
data copy up is complete.

Signed-off-by: Vivek Goyal <vgoyal@redhat.com>
Reviewed-by: Amir Goldstein <amir73il@gmail.com>
Fixes: 0c2888749363 ("ovl: A new xattr OVL_XATTR_METACOPY for file on upper")
Cc: <stable@vger.kernel.org> # v4.19+
Signed-off-by: Miklos Szeredi <mszeredi@redhat.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

diff --git a/fs/overlayfs/copy_up.c b/fs/overlayfs/copy_up.c
index 59080f7ba02f960ea4af72d395adeecb7fdb1f4a..75eeee08d8481a91b6598b67cb3c7a9db32e0581 100644 (file)
--- a/fs/overlayfs/copy_up.c
+++ b/fs/overlayfs/copy_up.c
@@ -711,6 +711,8 @@ static int ovl_copy_up_meta_inode_data(struct ovl_copy_up_ctx *c)
 {
        struct path upperpath, datapath;
        int err;
+       char *capability = NULL;
+       ssize_t uninitialized_var(cap_size);
 
        ovl_path_upper(c->dentry, &upperpath);
        if (WARN_ON(upperpath.dentry == NULL))
@@ -720,15 +722,37 @@ static int ovl_copy_up_meta_inode_data(struct ovl_copy_up_ctx *c)
        if (WARN_ON(datapath.dentry == NULL))
                return -EIO;
 
+       if (c->stat.size) {
+               err = cap_size = ovl_getxattr(upperpath.dentry, XATTR_NAME_CAPS,
+                                             &capability, 0);
+               if (err < 0 && err != -ENODATA)
+                       goto out;
+       }
+
        err = ovl_copy_up_data(&datapath, &upperpath, c->stat.size);
        if (err)
-               return err;
+               goto out_free;
+
+       /*
+        * Writing to upper file will clear security.capability xattr. We
+        * don't want that to happen for normal copy-up operation.
+        */
+       if (capability) {
+               err = ovl_do_setxattr(upperpath.dentry, XATTR_NAME_CAPS,
+                                     capability, cap_size, 0);
+               if (err)
+                       goto out_free;
+       }
+
 
        err = vfs_removexattr(upperpath.dentry, OVL_XATTR_METACOPY);
        if (err)
-               return err;
+               goto out_free;
 
        ovl_set_upperdata(d_inode(c->dentry));
+out_free:
+       kfree(capability);
+out:
        return err;
 }
 

diff --git a/fs/overlayfs/overlayfs.h b/fs/overlayfs/overlayfs.h
index a3c0d95843121e92a103a6b07628feb853c31399..d9c16ceebfe7daa94bf5d393cd48eee32baec4ec 100644 (file)
--- a/fs/overlayfs/overlayfs.h
+++ b/fs/overlayfs/overlayfs.h
@@ -277,6 +277,8 @@ int ovl_lock_rename_workdir(struct dentry *workdir, struct dentry *upperdir);
 int ovl_check_metacopy_xattr(struct dentry *dentry);
 bool ovl_is_metacopy_dentry(struct dentry *dentry);
 char *ovl_get_redirect_xattr(struct dentry *dentry, int padding);
+ssize_t ovl_getxattr(struct dentry *dentry, char *name, char **value,
+                    size_t padding);
 
 static inline bool ovl_is_impuredir(struct dentry *dentry)
 {

diff --git a/fs/overlayfs/util.c b/fs/overlayfs/util.c
index ace4fe4c39a9307aa6008702f0195a92af74627c..c9a2e3c6d5374f84cb7fa181c327b26d5be0e736 100644 (file)
--- a/fs/overlayfs/util.c
+++ b/fs/overlayfs/util.c
@@ -867,28 +867,49 @@ bool ovl_is_metacopy_dentry(struct dentry *dentry)
        return (oe->numlower > 1);
 }
 
-char *ovl_get_redirect_xattr(struct dentry *dentry, int padding)
+ssize_t ovl_getxattr(struct dentry *dentry, char *name, char **value,
+                    size_t padding)
 {
-       int res;
-       char *s, *next, *buf = NULL;
+       ssize_t res;
+       char *buf = NULL;
 
-       res = vfs_getxattr(dentry, OVL_XATTR_REDIRECT, NULL, 0);
+       res = vfs_getxattr(dentry, name, NULL, 0);
        if (res < 0) {
                if (res == -ENODATA || res == -EOPNOTSUPP)
-                       return NULL;
+                       return -ENODATA;
                goto fail;
        }
 
-       buf = kzalloc(res + padding + 1, GFP_KERNEL);
-       if (!buf)
-               return ERR_PTR(-ENOMEM);
+       if (res != 0) {
+               buf = kzalloc(res + padding, GFP_KERNEL);
+               if (!buf)
+                       return -ENOMEM;
 
-       if (res == 0)
-               goto invalid;
+               res = vfs_getxattr(dentry, name, buf, res);
+               if (res < 0)
+                       goto fail;
+       }
+       *value = buf;
+
+       return res;
+
+fail:
+       pr_warn_ratelimited("overlayfs: failed to get xattr %s: err=%zi)\n",
+                           name, res);
+       kfree(buf);
+       return res;
+}
 
-       res = vfs_getxattr(dentry, OVL_XATTR_REDIRECT, buf, res);
+char *ovl_get_redirect_xattr(struct dentry *dentry, int padding)
+{
+       int res;
+       char *s, *next, *buf = NULL;
+
+       res = ovl_getxattr(dentry, OVL_XATTR_REDIRECT, &buf, padding + 1);
+       if (res == -ENODATA)
+               return NULL;
        if (res < 0)
-               goto fail;
+               return ERR_PTR(res);
        if (res == 0)
                goto invalid;
 
@@ -904,15 +925,9 @@ char *ovl_get_redirect_xattr(struct dentry *dentry, int padding)
        }
 
        return buf;
-
-err_free:
-       kfree(buf);
-       return ERR_PTR(res);
-fail:
-       pr_warn_ratelimited("overlayfs: failed to get redirect (%i)\n", res);
-       goto err_free;
 invalid:
        pr_warn_ratelimited("overlayfs: invalid redirect (%s)\n", buf);
        res = -EINVAL;
-       goto err_free;
+       kfree(buf);
+       return ERR_PTR(res);
 }