xattr: simple_xattr_set() return old_xattr to be freed

tmpfs wants to support limited user extended attributes, but kernfs
(or cgroupfs, the only kernfs with KERNFS_ROOT_SUPPORT_USER_XATTR)
already supports user extended attributes through simple xattrs: but
limited by a policy (128KiB per inode) too liberal to be used on tmpfs.

To allow a different limiting policy for tmpfs, without affecting the
policy for kernfs, change simple_xattr_set() to return the replaced or
removed xattr (if any), leaving the caller to update their accounting
then free the xattr (by simple_xattr_free(), renamed from the static
free_simple_xattr()).

Signed-off-by: Hugh Dickins <hughd@google.com>
Reviewed-by: Jan Kara <jack@suse.cz>
Reviewed-by: Christian Brauner <brauner@kernel.org>
Reviewed-by: Carlos Maiolino <cmaiolino@redhat.com>
Message-Id: <158c6585-2aa7-d4aa-90ff-f7c3f8fe407c@google.com>
Signed-off-by: Christian Brauner <brauner@kernel.org>

diff --git a/fs/kernfs/inode.c b/fs/kernfs/inode.c
index b22b74d1a115099e45043bdf50c20ba7a51fd7d5..fec5d5f78f0750cddc66d3429f0d89bb814d3e4e 100644 (file)
--- a/fs/kernfs/inode.c
+++ b/fs/kernfs/inode.c
@@ -306,11 +306,17 @@ int kernfs_xattr_get(struct kernfs_node *kn, const char *name,
 int kernfs_xattr_set(struct kernfs_node *kn, const char *name,
                     const void *value, size_t size, int flags)
 {
+       struct simple_xattr *old_xattr;
        struct kernfs_iattrs *attrs = kernfs_iattrs(kn);
        if (!attrs)
                return -ENOMEM;
 
-       return simple_xattr_set(&attrs->xattrs, name, value, size, flags, NULL);
+       old_xattr = simple_xattr_set(&attrs->xattrs, name, value, size, flags);
+       if (IS_ERR(old_xattr))
+               return PTR_ERR(old_xattr);
+
+       simple_xattr_free(old_xattr);
+       return 0;
 }
 
 static int kernfs_vfs_xattr_get(const struct xattr_handler *handler,
@@ -342,7 +348,7 @@ static int kernfs_vfs_user_xattr_add(struct kernfs_node *kn,
 {
        atomic_t *sz = &kn->iattr->user_xattr_size;
        atomic_t *nr = &kn->iattr->nr_user_xattrs;
-       ssize_t removed_size;
+       struct simple_xattr *old_xattr;
        int ret;
 
        if (atomic_inc_return(nr) > KERNFS_MAX_USER_XATTRS) {
@@ -355,13 +361,18 @@ static int kernfs_vfs_user_xattr_add(struct kernfs_node *kn,
                goto dec_size_out;
        }
 
-       ret = simple_xattr_set(xattrs, full_name, value, size, flags,
-                              &removed_size);
-
-       if (!ret && removed_size >= 0)
-               size = removed_size;
-       else if (!ret)
+       old_xattr = simple_xattr_set(xattrs, full_name, value, size, flags);
+       if (!old_xattr)
                return 0;
+
+       if (IS_ERR(old_xattr)) {
+               ret = PTR_ERR(old_xattr);
+               goto dec_size_out;
+       }
+
+       ret = 0;
+       size = old_xattr->size;
+       simple_xattr_free(old_xattr);
 dec_size_out:
        atomic_sub(size, sz);
 dec_count_out:
@@ -376,18 +387,19 @@ static int kernfs_vfs_user_xattr_rm(struct kernfs_node *kn,
 {
        atomic_t *sz = &kn->iattr->user_xattr_size;
        atomic_t *nr = &kn->iattr->nr_user_xattrs;
-       ssize_t removed_size;
-       int ret;
+       struct simple_xattr *old_xattr;
 
-       ret = simple_xattr_set(xattrs, full_name, value, size, flags,
-                              &removed_size);
+       old_xattr = simple_xattr_set(xattrs, full_name, value, size, flags);
+       if (!old_xattr)
+               return 0;
 
-       if (removed_size >= 0) {
-               atomic_sub(removed_size, sz);
-               atomic_dec(nr);
-       }
+       if (IS_ERR(old_xattr))
+               return PTR_ERR(old_xattr);
 
-       return ret;
+       atomic_sub(old_xattr->size, sz);
+       atomic_dec(nr);
+       simple_xattr_free(old_xattr);
+       return 0;
 }
 
 static int kernfs_vfs_user_xattr_set(const struct xattr_handler *handler,

diff --git a/fs/xattr.c b/fs/xattr.c
index e7bbb7f57557d7bbcbd2e0394539a0af2d274d3e..ba37a8f5cfd1db005795077feb0b14dcfab37564 100644 (file)
--- a/fs/xattr.c
+++ b/fs/xattr.c
@@ -1040,12 +1040,12 @@ const char *xattr_full_name(const struct xattr_handler *handler,
 EXPORT_SYMBOL(xattr_full_name);
 
 /**
- * free_simple_xattr - free an xattr object
+ * simple_xattr_free - free an xattr object
  * @xattr: the xattr object
  *
  * Free the xattr object. Can handle @xattr being NULL.
  */
-static inline void free_simple_xattr(struct simple_xattr *xattr)
+void simple_xattr_free(struct simple_xattr *xattr)
 {
        if (xattr)
                kfree(xattr->name);
@@ -1164,7 +1164,6 @@ int simple_xattr_get(struct simple_xattrs *xattrs, const char *name,
  * @value: the value to store along the xattr
  * @size: the size of @value
  * @flags: the flags determining how to set the xattr
- * @removed_size: the size of the removed xattr
  *
  * Set a new xattr object.
  * If @value is passed a new xattr object will be allocated. If XATTR_REPLACE
@@ -1181,29 +1180,27 @@ int simple_xattr_get(struct simple_xattrs *xattrs, const char *name,
  * nothing if XATTR_CREATE is specified in @flags or @flags is zero. For
  * XATTR_REPLACE we fail as mentioned above.
  *
- * Return: On success zero and on error a negative error code is returned.
+ * Return: On success, the removed or replaced xattr is returned, to be freed
+ * by the caller; or NULL if none. On failure a negative error code is returned.
  */
-int simple_xattr_set(struct simple_xattrs *xattrs, const char *name,
-                    const void *value, size_t size, int flags,
-                    ssize_t *removed_size)
+struct simple_xattr *simple_xattr_set(struct simple_xattrs *xattrs,
+                                     const char *name, const void *value,
+                                     size_t size, int flags)
 {
-       struct simple_xattr *xattr = NULL, *new_xattr = NULL;
+       struct simple_xattr *old_xattr = NULL, *new_xattr = NULL;
        struct rb_node *parent = NULL, **rbp;
        int err = 0, ret;
 
-       if (removed_size)
-               *removed_size = -1;
-
        /* value == NULL means remove */
        if (value) {
                new_xattr = simple_xattr_alloc(value, size);
                if (!new_xattr)
-                       return -ENOMEM;
+                       return ERR_PTR(-ENOMEM);
 
                new_xattr->name = kstrdup(name, GFP_KERNEL);
                if (!new_xattr->name) {
-                       free_simple_xattr(new_xattr);
-                       return -ENOMEM;
+                       simple_xattr_free(new_xattr);
+                       return ERR_PTR(-ENOMEM);
                }
        }
 
@@ -1217,12 +1214,12 @@ int simple_xattr_set(struct simple_xattrs *xattrs, const char *name,
                else if (ret > 0)
                        rbp = &(*rbp)->rb_right;
                else
-                       xattr = rb_entry(*rbp, struct simple_xattr, rb_node);
-               if (xattr)
+                       old_xattr = rb_entry(*rbp, struct simple_xattr, rb_node);
+               if (old_xattr)
                        break;
        }
 
-       if (xattr) {
+       if (old_xattr) {
                /* Fail if XATTR_CREATE is requested and the xattr exists. */
                if (flags & XATTR_CREATE) {
                        err = -EEXIST;
@@ -1230,12 +1227,10 @@ int simple_xattr_set(struct simple_xattrs *xattrs, const char *name,
                }
 
                if (new_xattr)
-                       rb_replace_node(&xattr->rb_node, &new_xattr->rb_node,
-                                       &xattrs->rb_root);
+                       rb_replace_node(&old_xattr->rb_node,
+                                       &new_xattr->rb_node, &xattrs->rb_root);
                else
-                       rb_erase(&xattr->rb_node, &xattrs->rb_root);
-               if (!err && removed_size)
-                       *removed_size = xattr->size;
+                       rb_erase(&old_xattr->rb_node, &xattrs->rb_root);
        } else {
                /* Fail if XATTR_REPLACE is requested but no xattr is found. */
                if (flags & XATTR_REPLACE) {
@@ -1260,12 +1255,10 @@ int simple_xattr_set(struct simple_xattrs *xattrs, const char *name,
 
 out_unlock:
        write_unlock(&xattrs->lock);
-       if (err)
-               free_simple_xattr(new_xattr);
-       else
-               free_simple_xattr(xattr);
-       return err;
-
+       if (!err)
+               return old_xattr;
+       simple_xattr_free(new_xattr);
+       return ERR_PTR(err);
 }
 
 static bool xattr_is_trusted(const char *name)
@@ -1386,7 +1379,7 @@ void simple_xattrs_free(struct simple_xattrs *xattrs)
                rbp_next = rb_next(rbp);
                xattr = rb_entry(rbp, struct simple_xattr, rb_node);
                rb_erase(&xattr->rb_node, &xattrs->rb_root);
-               free_simple_xattr(xattr);
+               simple_xattr_free(xattr);
                rbp = rbp_next;
        }
 }

diff --git a/include/linux/xattr.h b/include/linux/xattr.h
index d591ef59aa98efed1f6cda7d54a21f94ed828bc4..e37fe667ae04462a3981ab513bc100d380ca834d 100644 (file)
--- a/include/linux/xattr.h
+++ b/include/linux/xattr.h
@@ -116,11 +116,12 @@ struct simple_xattr {
 void simple_xattrs_init(struct simple_xattrs *xattrs);
 void simple_xattrs_free(struct simple_xattrs *xattrs);
 struct simple_xattr *simple_xattr_alloc(const void *value, size_t size);
+void simple_xattr_free(struct simple_xattr *xattr);
 int simple_xattr_get(struct simple_xattrs *xattrs, const char *name,
                     void *buffer, size_t size);
-int simple_xattr_set(struct simple_xattrs *xattrs, const char *name,
-                    const void *value, size_t size, int flags,
-                    ssize_t *removed_size);
+struct simple_xattr *simple_xattr_set(struct simple_xattrs *xattrs,
+                                     const char *name, const void *value,
+                                     size_t size, int flags);
 ssize_t simple_xattr_list(struct inode *inode, struct simple_xattrs *xattrs,
                          char *buffer, size_t size);
 void simple_xattr_add(struct simple_xattrs *xattrs,

diff --git a/mm/shmem.c b/mm/shmem.c
index 678a7be46b8978cc17f51858e7d51fed3c906d29..b22d0bf1f5e3a7187ac4f26b1b64b7d1715b888d 100644 (file)
--- a/mm/shmem.c
+++ b/mm/shmem.c
@@ -3598,15 +3598,17 @@ static int shmem_xattr_handler_set(const struct xattr_handler *handler,
                                   size_t size, int flags)
 {
        struct shmem_inode_info *info = SHMEM_I(inode);
-       int err;
+       struct simple_xattr *old_xattr;
 
        name = xattr_full_name(handler, name);
-       err = simple_xattr_set(&info->xattrs, name, value, size, flags, NULL);
-       if (!err) {
+       old_xattr = simple_xattr_set(&info->xattrs, name, value, size, flags);
+       if (!IS_ERR(old_xattr)) {
+               simple_xattr_free(old_xattr);
+               old_xattr = NULL;
                inode->i_ctime = current_time(inode);
                inode_inc_iversion(inode);
        }
-       return err;
+       return PTR_ERR(old_xattr);
 }
 
 static const struct xattr_handler shmem_security_xattr_handler = {