From 112b0131b9896fa62a1022e93db8e0ff1cdd79f9 Mon Sep 17 00:00:00 2001 From: Andrew Walker Date: Mon, 25 May 2026 18:02:08 -0500 Subject: [PATCH] zpl_xattr: stop heap-allocating prefixed xattr names The six __zpl_xattr_{user,trusted,security}_{get,set} entry points built their prefixed name via kmem_asprintf("%s%s", prefix, name) and freed it with kmem_strfree on the way out. The Linux xattr API caps the full prefix+name length at XATTR_NAME_MAX (255), the same bound fs/xattr.c's syscall handlers rely on with their stack-resident struct xattr_name, and so do the same in our xattr handlers. Reviewed-by: Brian Behlendorf Reviewed-by: Rob Norris Reviewed-by: Ameer Hamza Signed-off-by: Andrew Walker Closes #18570 --- module/os/linux/zfs/zpl_xattr.c | 92 ++++++++++++++++++++------------- 1 file changed, 56 insertions(+), 36 deletions(-) diff --git a/module/os/linux/zfs/zpl_xattr.c b/module/os/linux/zfs/zpl_xattr.c index d93282db815..68050c870de 100644 --- a/module/os/linux/zfs/zpl_xattr.c +++ b/module/os/linux/zfs/zpl_xattr.c @@ -701,6 +701,24 @@ zpl_xattr_set(struct inode *ip, const char *name, const void *value, * ZFS allows extended user attributes to be disabled administratively * by setting the 'xattr=off' property on the dataset. */ + +/* + * Concatenate prefix + name into a NUL-terminated stack buffer. + * Linux fs/xattr.c (import_xattr_name) caps the full xattr name at + * XATTR_NAME_MAX before any handler runs, so XATTR_NAME_MAX + 1 + * bytes always fit. + */ +static inline void +zpl_xattr_join_name(char *buf, size_t buflen, const char *prefix, + size_t prefix_len, const char *name, size_t name_len) +{ + ASSERT3U(prefix_len + name_len + 1, <=, buflen); + + memcpy(buf, prefix, prefix_len); + memcpy(buf + prefix_len, name, name_len); + buf[prefix_len + name_len] = '\0'; +} + static int __zpl_xattr_user_list(struct inode *ip, char *list, size_t list_size, const char *name, size_t name_len) @@ -726,9 +744,13 @@ __zpl_xattr_user_get(struct inode *ip, const char *name, * try again without the namespace prefix for compatibility with * other platforms. */ - char *xattr_name = kmem_asprintf("%s%s", XATTR_USER_PREFIX, name); + char xattr_name[XATTR_NAME_MAX + 1]; + + zpl_xattr_join_name(xattr_name, sizeof (xattr_name), + XATTR_USER_PREFIX, XATTR_USER_PREFIX_LEN, + name, strlen(name)); + error = zpl_xattr_get(ip, xattr_name, value, size); - kmem_strfree(xattr_name); if (error == -ENODATA) error = zpl_xattr_get(ip, name, value, size); @@ -758,8 +780,13 @@ __zpl_xattr_user_set(zidmap_t *user_ns, * XATTR_CREATE: fail if xattr already exists * XATTR_REPLACE: fail if xattr does not exist */ - char *prefixed_name = kmem_asprintf("%s%s", XATTR_USER_PREFIX, name); + char prefixed_name[XATTR_NAME_MAX + 1]; const char *clear_name, *set_name; + + zpl_xattr_join_name(prefixed_name, sizeof (prefixed_name), + XATTR_USER_PREFIX, XATTR_USER_PREFIX_LEN, + name, strlen(name)); + if (zfs_xattr_compat) { clear_name = prefixed_name; set_name = name; @@ -776,7 +803,7 @@ __zpl_xattr_user_set(zidmap_t *user_ns, * because it already exists. Stop here. */ if (error == -EEXIST) - goto out; + return (error); /* * If XATTR_REPLACE was specified and we succeeded to clear * an xattr, we don't need to replace anything when setting @@ -788,10 +815,7 @@ __zpl_xattr_user_set(zidmap_t *user_ns, /* * Set the new value with the configured name format. */ - error = zpl_xattr_set(ip, set_name, value, size, flags); -out: - kmem_strfree(prefixed_name); - return (error); + return (zpl_xattr_set(ip, set_name, value, size, flags)); } ZPL_XATTR_SET_WRAPPER(zpl_xattr_user_set); @@ -824,17 +848,16 @@ static int __zpl_xattr_trusted_get(struct inode *ip, const char *name, void *value, size_t size) { - char *xattr_name; - int error; + char xattr_name[XATTR_NAME_MAX + 1]; if (!capable(CAP_SYS_ADMIN)) return (-EACCES); - /* xattr_resolve_name will do this for us if this is defined */ - xattr_name = kmem_asprintf("%s%s", XATTR_TRUSTED_PREFIX, name); - error = zpl_xattr_get(ip, xattr_name, value, size); - kmem_strfree(xattr_name); - return (error); + zpl_xattr_join_name(xattr_name, sizeof (xattr_name), + XATTR_TRUSTED_PREFIX, XATTR_TRUSTED_PREFIX_LEN, + name, strlen(name)); + + return (zpl_xattr_get(ip, xattr_name, value, size)); } ZPL_XATTR_GET_WRAPPER(zpl_xattr_trusted_get); @@ -844,17 +867,16 @@ __zpl_xattr_trusted_set(zidmap_t *user_ns, const void *value, size_t size, int flags) { (void) user_ns; - char *xattr_name; - int error; + char xattr_name[XATTR_NAME_MAX + 1]; if (!capable(CAP_SYS_ADMIN)) return (-EACCES); - /* xattr_resolve_name will do this for us if this is defined */ - xattr_name = kmem_asprintf("%s%s", XATTR_TRUSTED_PREFIX, name); - error = zpl_xattr_set(ip, xattr_name, value, size, flags); - kmem_strfree(xattr_name); - return (error); + zpl_xattr_join_name(xattr_name, sizeof (xattr_name), + XATTR_TRUSTED_PREFIX, XATTR_TRUSTED_PREFIX_LEN, + name, strlen(name)); + + return (zpl_xattr_set(ip, xattr_name, value, size, flags)); } ZPL_XATTR_SET_WRAPPER(zpl_xattr_trusted_set); @@ -889,14 +911,13 @@ static int __zpl_xattr_security_get(struct inode *ip, const char *name, void *value, size_t size) { - char *xattr_name; - int error; - /* xattr_resolve_name will do this for us if this is defined */ - xattr_name = kmem_asprintf("%s%s", XATTR_SECURITY_PREFIX, name); - error = zpl_xattr_get(ip, xattr_name, value, size); - kmem_strfree(xattr_name); + char xattr_name[XATTR_NAME_MAX + 1]; - return (error); + zpl_xattr_join_name(xattr_name, sizeof (xattr_name), + XATTR_SECURITY_PREFIX, XATTR_SECURITY_PREFIX_LEN, + name, strlen(name)); + + return (zpl_xattr_get(ip, xattr_name, value, size)); } ZPL_XATTR_GET_WRAPPER(zpl_xattr_security_get); @@ -906,14 +927,13 @@ __zpl_xattr_security_set(zidmap_t *user_ns, const void *value, size_t size, int flags) { (void) user_ns; - char *xattr_name; - int error; - /* xattr_resolve_name will do this for us if this is defined */ - xattr_name = kmem_asprintf("%s%s", XATTR_SECURITY_PREFIX, name); - error = zpl_xattr_set(ip, xattr_name, value, size, flags); - kmem_strfree(xattr_name); + char xattr_name[XATTR_NAME_MAX + 1]; - return (error); + zpl_xattr_join_name(xattr_name, sizeof (xattr_name), + XATTR_SECURITY_PREFIX, XATTR_SECURITY_PREFIX_LEN, + name, strlen(name)); + + return (zpl_xattr_set(ip, xattr_name, value, size, flags)); } ZPL_XATTR_SET_WRAPPER(zpl_xattr_security_set);