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>
This commit is contained in:
Hugh Dickins 2023-08-08 21:30:59 -07:00 committed by Christian Brauner
parent 0200679fc7
commit 5de75970c9
4 changed files with 61 additions and 53 deletions

View File

@ -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, int kernfs_xattr_set(struct kernfs_node *kn, const char *name,
const void *value, size_t size, int flags) const void *value, size_t size, int flags)
{ {
struct simple_xattr *old_xattr;
struct kernfs_iattrs *attrs = kernfs_iattrs(kn); struct kernfs_iattrs *attrs = kernfs_iattrs(kn);
if (!attrs) if (!attrs)
return -ENOMEM; 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, 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 *sz = &kn->iattr->user_xattr_size;
atomic_t *nr = &kn->iattr->nr_user_xattrs; atomic_t *nr = &kn->iattr->nr_user_xattrs;
ssize_t removed_size; struct simple_xattr *old_xattr;
int ret; int ret;
if (atomic_inc_return(nr) > KERNFS_MAX_USER_XATTRS) { 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; goto dec_size_out;
} }
ret = simple_xattr_set(xattrs, full_name, value, size, flags, old_xattr = simple_xattr_set(xattrs, full_name, value, size, flags);
&removed_size); if (!old_xattr)
if (!ret && removed_size >= 0)
size = removed_size;
else if (!ret)
return 0; 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: dec_size_out:
atomic_sub(size, sz); atomic_sub(size, sz);
dec_count_out: 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 *sz = &kn->iattr->user_xattr_size;
atomic_t *nr = &kn->iattr->nr_user_xattrs; atomic_t *nr = &kn->iattr->nr_user_xattrs;
ssize_t removed_size; struct simple_xattr *old_xattr;
int ret;
ret = simple_xattr_set(xattrs, full_name, value, size, flags, old_xattr = simple_xattr_set(xattrs, full_name, value, size, flags);
&removed_size); if (!old_xattr)
return 0;
if (removed_size >= 0) { if (IS_ERR(old_xattr))
atomic_sub(removed_size, sz); return PTR_ERR(old_xattr);
atomic_dec(nr);
}
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, static int kernfs_vfs_user_xattr_set(const struct xattr_handler *handler,

View File

@ -1040,12 +1040,12 @@ const char *xattr_full_name(const struct xattr_handler *handler,
EXPORT_SYMBOL(xattr_full_name); EXPORT_SYMBOL(xattr_full_name);
/** /**
* free_simple_xattr - free an xattr object * simple_xattr_free - free an xattr object
* @xattr: the xattr object * @xattr: the xattr object
* *
* Free the xattr object. Can handle @xattr being NULL. * 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) if (xattr)
kfree(xattr->name); 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 * @value: the value to store along the xattr
* @size: the size of @value * @size: the size of @value
* @flags: the flags determining how to set the xattr * @flags: the flags determining how to set the xattr
* @removed_size: the size of the removed xattr
* *
* Set a new xattr object. * Set a new xattr object.
* If @value is passed a new xattr object will be allocated. If XATTR_REPLACE * 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 * nothing if XATTR_CREATE is specified in @flags or @flags is zero. For
* XATTR_REPLACE we fail as mentioned above. * 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, struct simple_xattr *simple_xattr_set(struct simple_xattrs *xattrs,
const void *value, size_t size, int flags, const char *name, const void *value,
ssize_t *removed_size) 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; struct rb_node *parent = NULL, **rbp;
int err = 0, ret; int err = 0, ret;
if (removed_size)
*removed_size = -1;
/* value == NULL means remove */ /* value == NULL means remove */
if (value) { if (value) {
new_xattr = simple_xattr_alloc(value, size); new_xattr = simple_xattr_alloc(value, size);
if (!new_xattr) if (!new_xattr)
return -ENOMEM; return ERR_PTR(-ENOMEM);
new_xattr->name = kstrdup(name, GFP_KERNEL); new_xattr->name = kstrdup(name, GFP_KERNEL);
if (!new_xattr->name) { if (!new_xattr->name) {
free_simple_xattr(new_xattr); simple_xattr_free(new_xattr);
return -ENOMEM; return ERR_PTR(-ENOMEM);
} }
} }
@ -1217,12 +1214,12 @@ int simple_xattr_set(struct simple_xattrs *xattrs, const char *name,
else if (ret > 0) else if (ret > 0)
rbp = &(*rbp)->rb_right; rbp = &(*rbp)->rb_right;
else else
xattr = rb_entry(*rbp, struct simple_xattr, rb_node); old_xattr = rb_entry(*rbp, struct simple_xattr, rb_node);
if (xattr) if (old_xattr)
break; break;
} }
if (xattr) { if (old_xattr) {
/* Fail if XATTR_CREATE is requested and the xattr exists. */ /* Fail if XATTR_CREATE is requested and the xattr exists. */
if (flags & XATTR_CREATE) { if (flags & XATTR_CREATE) {
err = -EEXIST; err = -EEXIST;
@ -1230,12 +1227,10 @@ int simple_xattr_set(struct simple_xattrs *xattrs, const char *name,
} }
if (new_xattr) if (new_xattr)
rb_replace_node(&xattr->rb_node, &new_xattr->rb_node, rb_replace_node(&old_xattr->rb_node,
&xattrs->rb_root); &new_xattr->rb_node, &xattrs->rb_root);
else else
rb_erase(&xattr->rb_node, &xattrs->rb_root); rb_erase(&old_xattr->rb_node, &xattrs->rb_root);
if (!err && removed_size)
*removed_size = xattr->size;
} else { } else {
/* Fail if XATTR_REPLACE is requested but no xattr is found. */ /* Fail if XATTR_REPLACE is requested but no xattr is found. */
if (flags & XATTR_REPLACE) { if (flags & XATTR_REPLACE) {
@ -1260,12 +1255,10 @@ int simple_xattr_set(struct simple_xattrs *xattrs, const char *name,
out_unlock: out_unlock:
write_unlock(&xattrs->lock); write_unlock(&xattrs->lock);
if (err) if (!err)
free_simple_xattr(new_xattr); return old_xattr;
else simple_xattr_free(new_xattr);
free_simple_xattr(xattr); return ERR_PTR(err);
return err;
} }
static bool xattr_is_trusted(const char *name) 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); rbp_next = rb_next(rbp);
xattr = rb_entry(rbp, struct simple_xattr, rb_node); xattr = rb_entry(rbp, struct simple_xattr, rb_node);
rb_erase(&xattr->rb_node, &xattrs->rb_root); rb_erase(&xattr->rb_node, &xattrs->rb_root);
free_simple_xattr(xattr); simple_xattr_free(xattr);
rbp = rbp_next; rbp = rbp_next;
} }
} }

View File

@ -116,11 +116,12 @@ struct simple_xattr {
void simple_xattrs_init(struct simple_xattrs *xattrs); void simple_xattrs_init(struct simple_xattrs *xattrs);
void simple_xattrs_free(struct simple_xattrs *xattrs); void simple_xattrs_free(struct simple_xattrs *xattrs);
struct simple_xattr *simple_xattr_alloc(const void *value, size_t size); 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, int simple_xattr_get(struct simple_xattrs *xattrs, const char *name,
void *buffer, size_t size); void *buffer, size_t size);
int simple_xattr_set(struct simple_xattrs *xattrs, const char *name, struct simple_xattr *simple_xattr_set(struct simple_xattrs *xattrs,
const void *value, size_t size, int flags, const char *name, const void *value,
ssize_t *removed_size); size_t size, int flags);
ssize_t simple_xattr_list(struct inode *inode, struct simple_xattrs *xattrs, ssize_t simple_xattr_list(struct inode *inode, struct simple_xattrs *xattrs,
char *buffer, size_t size); char *buffer, size_t size);
void simple_xattr_add(struct simple_xattrs *xattrs, void simple_xattr_add(struct simple_xattrs *xattrs,

View File

@ -3598,15 +3598,17 @@ static int shmem_xattr_handler_set(const struct xattr_handler *handler,
size_t size, int flags) size_t size, int flags)
{ {
struct shmem_inode_info *info = SHMEM_I(inode); struct shmem_inode_info *info = SHMEM_I(inode);
int err; struct simple_xattr *old_xattr;
name = xattr_full_name(handler, name); name = xattr_full_name(handler, name);
err = simple_xattr_set(&info->xattrs, name, value, size, flags, NULL); old_xattr = simple_xattr_set(&info->xattrs, name, value, size, flags);
if (!err) { if (!IS_ERR(old_xattr)) {
simple_xattr_free(old_xattr);
old_xattr = NULL;
inode->i_ctime = current_time(inode); inode->i_ctime = current_time(inode);
inode_inc_iversion(inode); inode_inc_iversion(inode);
} }
return err; return PTR_ERR(old_xattr);
} }
static const struct xattr_handler shmem_security_xattr_handler = { static const struct xattr_handler shmem_security_xattr_handler = {