From: Miklos Szeredi Date: Fri, 5 Jun 2026 13:53:18 +0000 (+0200) Subject: simple_xattr: change interface to pass struct simple_xattrs ** X-Git-Url: http://git.ipfire.org/gitweb/?a=commitdiff_plain;h=076e5cef28e27febfc09b5f72544d2b857c75201;p=thirdparty%2Fkernel%2Flinux.git simple_xattr: change interface to pass struct simple_xattrs ** Change the simple_xattr API to accept pointer-to-pointer (struct simple_xattrs **) instead of pointer. This allows the functions to handle lazy allocation internally without requiring callers to use simple_xattrs_lazy_alloc(). The simple_xattr_set(), simple_xattr_set_limited() and simple_xattr_add() functions now handle allocation when xattrs is NULL. simple_xattrs_free() now also frees the xattrs structure itself and sets the pointer to NULL. This simplifies callers and removes the need for most callers to explicitly manage xattrs allocation and lifetime. In shmem_initxattrs(), the total required space for all initial xattrs (ispace) is pre-calculated and deducted from sbinfo->free_ispace. Since this patch modifies the function to add new xattrs directly to the inode's &info->xattrs list rather than using a local temporary variable, a failure means that the partially populated info->xattrs list remains attached to the inode. When the VFS caller handles the -ENOMEM error, it drops the newly created inode via iput(), shmem_free_inode() adds freed to sbinfo->free_ispace a second time, permanently inflating the tmpfs free space quota. Fix by substracting already added xattrs from ispace. Signed-off-by: Miklos Szeredi Link: https://patch.msgid.link/20260605135322.2632068-4-mszeredi@redhat.com Signed-off-by: Christian Brauner (Amutable) --- diff --git a/fs/kernfs/dir.c b/fs/kernfs/dir.c index 4f9ade82b08ab..368dc4a217d93 100644 --- a/fs/kernfs/dir.c +++ b/fs/kernfs/dir.c @@ -605,11 +605,8 @@ void kernfs_put(struct kernfs_node *kn) if (kernfs_type(kn) == KERNFS_LINK) kernfs_put(kn->symlink.target_kn); - if (kn->iattr && kn->iattr->xattrs) { - simple_xattrs_free(kn->iattr->xattrs, NULL); - kfree(kn->iattr->xattrs); - kn->iattr->xattrs = NULL; - } + if (kn->iattr) + simple_xattrs_free(&kn->iattr->xattrs, NULL); spin_lock(&root->kernfs_idr_lock); idr_remove(&root->ino_idr, (u32)kernfs_ino(kn)); @@ -709,10 +706,7 @@ static struct kernfs_node *__kernfs_new_node(struct kernfs_root *root, err_out4: if (kn->iattr) { - if (kn->iattr->xattrs) { - simple_xattrs_free(kn->iattr->xattrs, NULL); - kfree(kn->iattr->xattrs); - } + simple_xattrs_free(&kn->iattr->xattrs, NULL); kmem_cache_free(kernfs_iattrs_cache, kn->iattr); } err_out3: diff --git a/fs/kernfs/inode.c b/fs/kernfs/inode.c index e676737d9531a..f2298de6bc6f8 100644 --- a/fs/kernfs/inode.c +++ b/fs/kernfs/inode.c @@ -144,8 +144,7 @@ ssize_t kernfs_iop_listxattr(struct dentry *dentry, char *buf, size_t size) if (!attrs) return -ENOMEM; - return simple_xattr_list(d_inode(dentry), READ_ONCE(attrs->xattrs), - buf, size); + return simple_xattr_list(d_inode(dentry), &attrs->xattrs, buf, size); } static inline void set_default_inode_attr(struct inode *inode, umode_t mode) @@ -297,23 +296,17 @@ int kernfs_xattr_get(struct kernfs_node *kn, const char *name, void *value, size_t size) { struct kernfs_iattrs *attrs = kernfs_iattrs_noalloc(kn); - struct simple_xattrs *xattrs; if (!attrs) return -ENODATA; - xattrs = READ_ONCE(attrs->xattrs); - if (!xattrs) - return -ENODATA; - - return simple_xattr_get(xattrs, name, value, size); + return simple_xattr_get(&attrs->xattrs, name, value, size); } 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 simple_xattrs *xattrs; struct kernfs_iattrs *attrs; attrs = kernfs_iattrs(kn); @@ -329,11 +322,7 @@ int kernfs_xattr_set(struct kernfs_node *kn, const char *name, */ CLASS(kernfs_node_lock, lock)(kn); - xattrs = simple_xattrs_lazy_alloc(&attrs->xattrs, value, flags); - if (IS_ERR_OR_NULL(xattrs)) - return PTR_ERR(xattrs); - - old_xattr = simple_xattr_set(xattrs, name, value, size, flags); + old_xattr = simple_xattr_set(&attrs->xattrs, name, value, size, flags); if (IS_ERR(old_xattr)) return PTR_ERR(old_xattr); @@ -371,7 +360,6 @@ static int kernfs_vfs_user_xattr_set(const struct xattr_handler *handler, { const char *full_name = xattr_full_name(handler, suffix); struct kernfs_node *kn = inode->i_private; - struct simple_xattrs *xattrs; struct kernfs_iattrs *attrs; if (!(kernfs_root(kn)->flags & KERNFS_ROOT_SUPPORT_USER_XATTR)) @@ -384,11 +372,7 @@ static int kernfs_vfs_user_xattr_set(const struct xattr_handler *handler, /* See comment in kernfs_xattr_set() about locking. */ CLASS(kernfs_node_lock, lock)(kn); - xattrs = simple_xattrs_lazy_alloc(&attrs->xattrs, value, flags); - if (IS_ERR_OR_NULL(xattrs)) - return PTR_ERR(xattrs); - - return simple_xattr_set_limited(xattrs, &attrs->xattr_limits, + return simple_xattr_set_limited(&attrs->xattrs, &attrs->xattr_limits, full_name, value, size, flags); } diff --git a/fs/pidfs.c b/fs/pidfs.c index 1cce4f34a0512..eb5105bddecaf 100644 --- a/fs/pidfs.c +++ b/fs/pidfs.c @@ -196,12 +196,7 @@ static void pidfs_free_attr_work(struct work_struct *work) head = llist_del_all(&pidfs_free_list); llist_for_each_entry_safe(attr, next, head, pidfs_llist) { - struct simple_xattrs *xattrs = attr->xattrs; - - if (xattrs) { - simple_xattrs_free(xattrs, NULL); - kfree(xattrs); - } + simple_xattrs_free(&attr->xattrs, NULL); kfree(attr); } } @@ -815,14 +810,8 @@ static ssize_t pidfs_listxattr(struct dentry *dentry, char *buf, size_t size) { struct inode *inode = d_inode(dentry); struct pid *pid = inode->i_private; - struct pidfs_attr *attr = pid->attr; - struct simple_xattrs *xattrs; - - xattrs = READ_ONCE(attr->xattrs); - if (!xattrs) - return 0; - return simple_xattr_list(inode, xattrs, buf, size); + return simple_xattr_list(inode, &pid->attr->xattrs, buf, size); } static const struct inode_operations pidfs_inode_operations = { @@ -1057,16 +1046,9 @@ static int pidfs_xattr_get(const struct xattr_handler *handler, const char *suffix, void *value, size_t size) { struct pid *pid = inode->i_private; - struct pidfs_attr *attr = pid->attr; - const char *name; - struct simple_xattrs *xattrs; - - xattrs = READ_ONCE(attr->xattrs); - if (!xattrs) - return -ENODATA; + const char *name = xattr_full_name(handler, suffix); - name = xattr_full_name(handler, suffix); - return simple_xattr_get(xattrs, name, value, size); + return simple_xattr_get(&pid->attr->xattrs, name, value, size); } static int pidfs_xattr_set(const struct xattr_handler *handler, @@ -1075,20 +1057,13 @@ static int pidfs_xattr_set(const struct xattr_handler *handler, const void *value, size_t size, int flags) { struct pid *pid = inode->i_private; - struct pidfs_attr *attr = pid->attr; - const char *name; - struct simple_xattrs *xattrs; + const char *name = xattr_full_name(handler, suffix); struct simple_xattr *old_xattr; /* Ensure we're the only one to set @attr->xattrs. */ WARN_ON_ONCE(!inode_is_locked(inode)); - xattrs = simple_xattrs_lazy_alloc(&attr->xattrs, value, flags); - if (IS_ERR_OR_NULL(xattrs)) - return PTR_ERR(xattrs); - - name = xattr_full_name(handler, suffix); - old_xattr = simple_xattr_set(xattrs, name, value, size, flags); + old_xattr = simple_xattr_set(&pid->attr->xattrs, name, value, size, flags); if (IS_ERR(old_xattr)) return PTR_ERR(old_xattr); diff --git a/fs/xattr.c b/fs/xattr.c index 09ecbaaa16608..9ef7ad8a8f321 100644 --- a/fs/xattr.c +++ b/fs/xattr.c @@ -1311,12 +1311,17 @@ static const struct rhashtable_params simple_xattr_params = { * Return: On success the length of the xattr value is returned. On error a * negative error code is returned. */ -int simple_xattr_get(struct simple_xattrs *xattrs, const char *name, +int simple_xattr_get(struct simple_xattrs **xattrsp, const char *name, void *buffer, size_t size) { + struct simple_xattrs *xattrs; struct simple_xattr *xattr; int ret = -ENODATA; + xattrs = READ_ONCE(*xattrsp); + if (!xattrs) + return -ENODATA; + guard(rcu)(); xattr = rhashtable_lookup(&xattrs->ht, name, simple_xattr_params); if (xattr) { @@ -1331,6 +1336,9 @@ int simple_xattr_get(struct simple_xattrs *xattrs, const char *name, return ret; } +static struct simple_xattrs *simple_xattrs_lazy_alloc(struct simple_xattrs **xattrsp, + const void *value, int flags); + /** * simple_xattr_set - set an xattr object * @xattrs: the header of the xattr object @@ -1362,13 +1370,18 @@ int simple_xattr_get(struct simple_xattrs *xattrs, const char *name, * 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. */ -struct simple_xattr *simple_xattr_set(struct simple_xattrs *xattrs, +struct simple_xattr *simple_xattr_set(struct simple_xattrs **xattrsp, const char *name, const void *value, size_t size, int flags) { + struct simple_xattrs *xattrs; struct simple_xattr *old_xattr = NULL; int err; + xattrs = simple_xattrs_lazy_alloc(xattrsp, value, flags); + if (IS_ERR_OR_NULL(xattrs)) + return ERR_CAST(xattrs); + CLASS(simple_xattr, new_xattr)(value, size); if (IS_ERR(new_xattr)) return new_xattr; @@ -1467,7 +1480,7 @@ static inline int simple_xattr_limits_inc(struct simple_xattr_limits *limits, * Return: On success zero is returned. On failure a negative error code is * returned. */ -int simple_xattr_set_limited(struct simple_xattrs *xattrs, +int simple_xattr_set_limited(struct simple_xattrs **xattrs, struct simple_xattr_limits *limits, const char *name, const void *value, size_t size, int flags) @@ -1527,10 +1540,11 @@ static bool xattr_is_maclabel(const char *name) * Return: On success the required size or the size of the copied xattrs is * returned. On error a negative error code is returned. */ -ssize_t simple_xattr_list(struct inode *inode, struct simple_xattrs *xattrs, +ssize_t simple_xattr_list(struct inode *inode, struct simple_xattrs **xattrsp, char *buffer, size_t size) { bool trusted = ns_capable_noaudit(&init_user_ns, CAP_SYS_ADMIN); + struct simple_xattrs *xattrs; struct rhashtable_iter iter; struct simple_xattr *xattr; ssize_t remaining_size = size; @@ -1552,6 +1566,7 @@ ssize_t simple_xattr_list(struct inode *inode, struct simple_xattrs *xattrs, remaining_size -= err; err = 0; + xattrs = READ_ONCE(*xattrsp); if (!xattrs) return size - remaining_size; @@ -1597,9 +1612,15 @@ ssize_t simple_xattr_list(struct inode *inode, struct simple_xattrs *xattrs, * Return: On success zero is returned. On failure a negative error code is * returned. */ -int simple_xattr_add(struct simple_xattrs *xattrs, +int simple_xattr_add(struct simple_xattrs **xattrsp, struct simple_xattr *new_xattr) { + struct simple_xattrs *xattrs; + + xattrs = simple_xattrs_lazy_alloc(xattrsp, new_xattr->value, 0); + if (IS_ERR(xattrs)) + return PTR_ERR(xattrs); + return rhashtable_insert_fast(&xattrs->ht, &new_xattr->hash_node, simple_xattr_params); } @@ -1629,7 +1650,7 @@ int simple_xattrs_init(struct simple_xattrs *xattrs) * Return: On success a new simple_xattrs is returned. On failure an * ERR_PTR is returned. */ -struct simple_xattrs *simple_xattrs_alloc(void) +static struct simple_xattrs *simple_xattrs_alloc(void) { struct simple_xattrs *xattrs __free(kfree) = NULL; int ret; @@ -1661,8 +1682,8 @@ struct simple_xattrs *simple_xattrs_alloc(void) * check with IS_ERR_OR_NULL() and propagate with PTR_ERR() which * correctly returns 0 for the NULL no-op case. */ -struct simple_xattrs *simple_xattrs_lazy_alloc(struct simple_xattrs **xattrsp, - const void *value, int flags) +static struct simple_xattrs *simple_xattrs_lazy_alloc(struct simple_xattrs **xattrsp, + const void *value, int flags) { struct simple_xattrs *xattrs; @@ -1697,12 +1718,19 @@ static void simple_xattr_ht_free(void *ptr, void *arg) * Destroy all xattrs in @xattr. When this is called no one can hold a * reference to any of the xattrs anymore. */ -void simple_xattrs_free(struct simple_xattrs *xattrs, size_t *freed_space) +void simple_xattrs_free(struct simple_xattrs **xattrsp, size_t *freed_space) { + struct simple_xattrs *xattrs = *xattrsp; + might_sleep(); + if (!xattrs) + return; + if (freed_space) *freed_space = 0; rhashtable_free_and_destroy(&xattrs->ht, simple_xattr_ht_free, freed_space); + kfree(xattrs); + *xattrsp = NULL; } diff --git a/include/linux/xattr.h b/include/linux/xattr.h index 8b6601367eae8..ded446c1ef81f 100644 --- a/include/linux/xattr.h +++ b/include/linux/xattr.h @@ -133,26 +133,23 @@ static inline void simple_xattr_limits_init(struct simple_xattr_limits *limits) } int simple_xattrs_init(struct simple_xattrs *xattrs); -struct simple_xattrs *simple_xattrs_alloc(void); -struct simple_xattrs *simple_xattrs_lazy_alloc(struct simple_xattrs **xattrsp, - const void *value, int flags); -void simple_xattrs_free(struct simple_xattrs *xattrs, size_t *freed_space); +void simple_xattrs_free(struct simple_xattrs **xattrs, size_t *freed_space); size_t simple_xattr_space(const char *name, size_t size); struct simple_xattr *simple_xattr_alloc(const void *value, size_t size); void simple_xattr_free(struct simple_xattr *xattr); void simple_xattr_free_rcu(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); -struct simple_xattr *simple_xattr_set(struct simple_xattrs *xattrs, +struct simple_xattr *simple_xattr_set(struct simple_xattrs **xattrs, const char *name, const void *value, size_t size, int flags); -int simple_xattr_set_limited(struct simple_xattrs *xattrs, +int simple_xattr_set_limited(struct simple_xattrs **xattrs, struct simple_xattr_limits *limits, const char *name, const void *value, 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); -int simple_xattr_add(struct simple_xattrs *xattrs, +int simple_xattr_add(struct simple_xattrs **xattrs, struct simple_xattr *new_xattr); int xattr_list_one(char **buffer, ssize_t *remaining_size, const char *name); @@ -162,10 +159,4 @@ DEFINE_CLASS(simple_xattr, simple_xattr_alloc(value, size), const void *value, size_t size) -DEFINE_CLASS(simple_xattrs, - struct simple_xattrs *, - if (!IS_ERR_OR_NULL(_T)) { simple_xattrs_free(_T, NULL); kfree(_T); }, - simple_xattrs_alloc(), - void) - #endif /* _LINUX_XATTR_H */ diff --git a/mm/shmem.c b/mm/shmem.c index c7897570fc9f9..cf4ee9f411911 100644 --- a/mm/shmem.c +++ b/mm/shmem.c @@ -1425,10 +1425,8 @@ static void shmem_evict_inode(struct inode *inode) } } - if (info->xattrs) { - simple_xattrs_free(info->xattrs, sbinfo->max_inodes ? &freed : NULL); - kfree(info->xattrs); - } + simple_xattrs_free(&info->xattrs, sbinfo->max_inodes ? &freed : NULL); + shmem_free_inode(inode->i_sb, freed); WARN_ON(inode->i_blocks); clear_inode(inode); @@ -4233,10 +4231,6 @@ static int shmem_initxattrs(struct inode *inode, const struct xattr *xattr; size_t ispace = 0; - CLASS(simple_xattrs, xattrs)(); - if (IS_ERR(xattrs)) - return PTR_ERR(xattrs); - if (sbinfo->max_inodes) { for (xattr = xattr_array; xattr->name != NULL; xattr++) { ispace += simple_xattr_space(xattr->name, @@ -4264,8 +4258,11 @@ static int shmem_initxattrs(struct inode *inode, if (!new_xattr->name) break; - if (simple_xattr_add(xattrs, new_xattr)) + if (simple_xattr_add(&info->xattrs, new_xattr)) break; + + if (sbinfo->max_inodes) + ispace -= simple_xattr_space(new_xattr->name, new_xattr->size); retain_and_null_ptr(new_xattr); } @@ -4277,8 +4274,8 @@ static int shmem_initxattrs(struct inode *inode, } return -ENOMEM; } + WARN_ON(ispace); - smp_store_release(&info->xattrs, no_free_ptr(xattrs)); return 0; } @@ -4287,14 +4284,9 @@ static int shmem_xattr_handler_get(const struct xattr_handler *handler, const char *name, void *buffer, size_t size) { struct shmem_inode_info *info = SHMEM_I(inode); - struct simple_xattrs *xattrs; - - xattrs = READ_ONCE(info->xattrs); - if (!xattrs) - return -ENODATA; name = xattr_full_name(handler, name); - return simple_xattr_get(xattrs, name, buffer, size); + return simple_xattr_get(&info->xattrs, name, buffer, size); } static int shmem_xattr_handler_set(const struct xattr_handler *handler, @@ -4305,16 +4297,11 @@ static int shmem_xattr_handler_set(const struct xattr_handler *handler, { struct shmem_inode_info *info = SHMEM_I(inode); struct shmem_sb_info *sbinfo = SHMEM_SB(inode->i_sb); - struct simple_xattrs *xattrs; struct simple_xattr *old_xattr; size_t ispace = 0; name = xattr_full_name(handler, name); - xattrs = simple_xattrs_lazy_alloc(&info->xattrs, value, flags); - if (IS_ERR_OR_NULL(xattrs)) - return PTR_ERR(xattrs); - if (value && sbinfo->max_inodes) { ispace = simple_xattr_space(name, size); raw_spin_lock(&sbinfo->stat_lock); @@ -4327,7 +4314,7 @@ static int shmem_xattr_handler_set(const struct xattr_handler *handler, return -ENOSPC; } - old_xattr = simple_xattr_set(xattrs, name, value, size, flags); + old_xattr = simple_xattr_set(&info->xattrs, name, value, size, flags); if (!IS_ERR(old_xattr)) { ispace = 0; if (old_xattr && sbinfo->max_inodes) @@ -4375,8 +4362,7 @@ static ssize_t shmem_listxattr(struct dentry *dentry, char *buffer, size_t size) { struct shmem_inode_info *info = SHMEM_I(d_inode(dentry)); - return simple_xattr_list(d_inode(dentry), READ_ONCE(info->xattrs), - buffer, size); + return simple_xattr_list(d_inode(dentry), &info->xattrs, buffer, size); } #endif /* CONFIG_TMPFS_XATTR */ diff --git a/net/socket.c b/net/socket.c index 22a412fdec079..d3597c8583450 100644 --- a/net/socket.c +++ b/net/socket.c @@ -347,12 +347,8 @@ static struct inode *sock_alloc_inode(struct super_block *sb) static void sock_evict_inode(struct inode *inode) { struct sockfs_inode *si = SOCKFS_I(inode); - struct simple_xattrs *xattrs = si->xattrs; - if (xattrs) { - simple_xattrs_free(xattrs, NULL); - kfree(xattrs); - } + simple_xattrs_free(&si->xattrs, NULL); clear_inode(inode); } @@ -443,13 +439,9 @@ static int sockfs_user_xattr_get(const struct xattr_handler *handler, const char *suffix, void *value, size_t size) { const char *name = xattr_full_name(handler, suffix); - struct simple_xattrs *xattrs; - - xattrs = READ_ONCE(SOCKFS_I(inode)->xattrs); - if (!xattrs) - return -ENODATA; + struct sockfs_inode *si = SOCKFS_I(inode); - return simple_xattr_get(xattrs, name, value, size); + return simple_xattr_get(&si->xattrs, name, value, size); } static int sockfs_user_xattr_set(const struct xattr_handler *handler, @@ -460,13 +452,8 @@ static int sockfs_user_xattr_set(const struct xattr_handler *handler, { const char *name = xattr_full_name(handler, suffix); struct sockfs_inode *si = SOCKFS_I(inode); - struct simple_xattrs *xattrs; - - xattrs = simple_xattrs_lazy_alloc(&si->xattrs, value, flags); - if (IS_ERR_OR_NULL(xattrs)) - return PTR_ERR(xattrs); - return simple_xattr_set_limited(xattrs, &si->xattr_limits, + return simple_xattr_set_limited(&si->xattrs, &si->xattr_limits, name, value, size, flags); } @@ -635,8 +622,7 @@ static ssize_t sockfs_listxattr(struct dentry *dentry, char *buffer, struct sockfs_inode *si = SOCKFS_I(d_inode(dentry)); ssize_t len, used; - len = simple_xattr_list(d_inode(dentry), READ_ONCE(si->xattrs), - buffer, size); + len = simple_xattr_list(d_inode(dentry), &si->xattrs, buffer, size); if (len < 0) return len;