Merge tag 'ovl-update-5.1' of git://git.kernel.org/pub/scm/linux/kernel/git/mszeredi/vfs

Pull overlayfs updates from Miklos Szeredi:
 "Fix copy up of security related xattrs"

* tag 'ovl-update-5.1' of git://git.kernel.org/pub/scm/linux/kernel/git/mszeredi/vfs:
  ovl: Do not lose security.capability xattr over metadata file copy-up
  ovl: During copy up, first copy up data and then xattrs

3 files changed, 81 insertions, 35 deletions

diff --git a/fs/overlayfs/copy_up.c b/fs/overlayfs/copy_up.c
index 9e62dcf06fc4..68b3303e4b46 100644
--- a/fs/overlayfs/copy_up.c
+++ b/fs/overlayfs/copy_up.c
@@ -443,6 +443,24 @@ static int ovl_copy_up_inode(struct ovl_copy_up_ctx *c, struct dentry *temp)
 {
         int err;
 
+        /*
+         * Copy up data first and then xattrs. Writing data after
+         * xattrs will remove security.capability xattr automatically.
+         */
+        if (S_ISREG(c->stat.mode) && !c->metacopy) {
+                struct path upperpath, datapath;
+
+                ovl_path_upper(c->dentry, &upperpath);
+                if (WARN_ON(upperpath.dentry != NULL))
+                        return -EIO;
+                upperpath.dentry = temp;
+
+                ovl_path_lowerdata(c->dentry, &datapath);
+                err = ovl_copy_up_data(&datapath, &upperpath, c->stat.size);
+                if (err)
+                        return err;
+        }
+
         err = ovl_copy_xattr(c->lowerpath.dentry, temp);
         if (err)
                 return err;
@@ -460,19 +478,6 @@ static int ovl_copy_up_inode(struct ovl_copy_up_ctx *c, struct dentry *temp)
                         return err;
         }
 
-        if (S_ISREG(c->stat.mode) && !c->metacopy) {
-                struct path upperpath, datapath;
-
-                ovl_path_upper(c->dentry, &upperpath);
-                BUG_ON(upperpath.dentry != NULL);
-                upperpath.dentry = temp;
-
-                ovl_path_lowerdata(c->dentry, &datapath);
-                err = ovl_copy_up_data(&datapath, &upperpath, c->stat.size);
-                if (err)
-                        return err;
-        }
-
         if (c->metacopy) {
                 err = ovl_check_setxattr(c->dentry, temp, OVL_XATTR_METACOPY,
                                          NULL, 0, -EOPNOTSUPP);
@@ -737,6 +742,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))
@@ -746,15 +753,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 5e45cb3630a0..9c6018287d57 100644
--- 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 7c01327b1852..4035e640f402 100644
--- a/fs/overlayfs/util.c
+++ b/fs/overlayfs/util.c
@@ -863,28 +863,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;
 
@@ -900,15 +921,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);
 }