From 3dffadfa99f7ba2e9bee69d1e0bb42fd2d2d6022 Mon Sep 17 00:00:00 2001 From: Zhen Ni Date: Mon, 1 Sep 2025 10:58:18 +0800 Subject: [PATCH 1/3] orangefs: Remove unused type in macro fill_default_sys_attrs Remove the unused type parameter from the macro definition and all its callers, making the interface consistent with its actual usage. Fixes: 69a23de2f3de ("orangefs: clean up fill_default_sys_attrs") Signed-off-by: Zhen Ni Signed-off-by: Mike Marshall --- fs/orangefs/namei.c | 10 +++------- fs/orangefs/orangefs-kernel.h | 2 +- 2 files changed, 4 insertions(+), 8 deletions(-) diff --git a/fs/orangefs/namei.c b/fs/orangefs/namei.c index 82395fe2b956..bec5475de094 100644 --- a/fs/orangefs/namei.c +++ b/fs/orangefs/namei.c @@ -38,8 +38,7 @@ static int orangefs_create(struct mnt_idmap *idmap, new_op->upcall.req.create.parent_refn = parent->refn; - fill_default_sys_attrs(new_op->upcall.req.create.attributes, - ORANGEFS_TYPE_METAFILE, mode); + fill_default_sys_attrs(new_op->upcall.req.create.attributes, mode); strscpy(new_op->upcall.req.create.d_name, dentry->d_name.name); @@ -240,9 +239,7 @@ static int orangefs_symlink(struct mnt_idmap *idmap, new_op->upcall.req.sym.parent_refn = parent->refn; - fill_default_sys_attrs(new_op->upcall.req.sym.attributes, - ORANGEFS_TYPE_SYMLINK, - mode); + fill_default_sys_attrs(new_op->upcall.req.sym.attributes, mode); strscpy(new_op->upcall.req.sym.entry_name, dentry->d_name.name); strscpy(new_op->upcall.req.sym.target, symname); @@ -316,8 +313,7 @@ static struct dentry *orangefs_mkdir(struct mnt_idmap *idmap, struct inode *dir, new_op->upcall.req.mkdir.parent_refn = parent->refn; - fill_default_sys_attrs(new_op->upcall.req.mkdir.attributes, - ORANGEFS_TYPE_DIRECTORY, mode); + fill_default_sys_attrs(new_op->upcall.req.mkdir.attributes, mode); strscpy(new_op->upcall.req.mkdir.d_name, dentry->d_name.name); diff --git a/fs/orangefs/orangefs-kernel.h b/fs/orangefs/orangefs-kernel.h index 3e153c2f6b82..29c6da43e396 100644 --- a/fs/orangefs/orangefs-kernel.h +++ b/fs/orangefs/orangefs-kernel.h @@ -462,7 +462,7 @@ int service_operation(struct orangefs_kernel_op_s *op, ((ORANGEFS_SB(inode->i_sb)->flags & ORANGEFS_OPT_INTR) ? \ ORANGEFS_OP_INTERRUPTIBLE : 0) -#define fill_default_sys_attrs(sys_attr, type, mode) \ +#define fill_default_sys_attrs(sys_attr, mode) \ do { \ sys_attr.owner = from_kuid(&init_user_ns, current_fsuid()); \ sys_attr.group = from_kgid(&init_user_ns, current_fsgid()); \ From 025e880759c279ec64d0f754fe65bf45961da864 Mon Sep 17 00:00:00 2001 From: Mike Marshall Date: Mon, 15 Sep 2025 17:40:46 -0400 Subject: [PATCH 2/3] orangefs: fix xattr related buffer overflow... Willy Tarreau forwarded me a message from Disclosure with the following warning: > The helper `xattr_key()` uses the pointer variable in the loop condition > rather than dereferencing it. As `key` is incremented, it remains non-NULL > (until it runs into unmapped memory), so the loop does not terminate on > valid C strings and will walk memory indefinitely, consuming CPU or hanging > the thread. I easily reproduced this with setfattr and getfattr, causing a kernel oops, hung user processes and corrupted orangefs files. Disclosure sent along a diff (not a patch) with a suggested fix, which I based this patch on. After xattr_key started working right, xfstest generic/069 exposed an xattr related memory leak that lead to OOM. xattr_key returns a hashed key. When adding xattrs to the orangefs xattr cache, orangefs used hash_add, a kernel hashing macro. hash_add also hashes the key using hash_log which resulted in additions to the xattr cache going to the wrong hash bucket. generic/069 tortures a single file and orangefs does a getattr for the xattr "security.capability" every time. Orangefs negative caches on xattrs which includes a kmalloc. Since adds to the xattr cache were going to the wrong bucket, every getattr for "security.capability" resulted in another kmalloc, none of which were ever freed. I changed the two uses of hash_add to hlist_add_head instead and the memory leak ceased and generic/069 quit throwing furniture. Signed-off-by: Mike Marshall Reported-by: Stanislav Fort of Aisle Research --- fs/orangefs/xattr.c | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/fs/orangefs/xattr.c b/fs/orangefs/xattr.c index 74ef75586f38..eee3c5ed1bbb 100644 --- a/fs/orangefs/xattr.c +++ b/fs/orangefs/xattr.c @@ -54,7 +54,9 @@ static inline int convert_to_internal_xattr_flags(int setxattr_flags) static unsigned int xattr_key(const char *key) { unsigned int i = 0; - while (key) + if (!key) + return 0; + while (*key) i += *key++; return i % 16; } @@ -175,8 +177,8 @@ ssize_t orangefs_inode_getxattr(struct inode *inode, const char *name, cx->length = -1; cx->timeout = jiffies + orangefs_getattr_timeout_msecs*HZ/1000; - hash_add(orangefs_inode->xattr_cache, &cx->node, - xattr_key(cx->key)); + hlist_add_head( &cx->node, + &orangefs_inode->xattr_cache[xattr_key(cx->key)]); } } goto out_release_op; @@ -229,8 +231,8 @@ ssize_t orangefs_inode_getxattr(struct inode *inode, const char *name, memcpy(cx->val, buffer, length); cx->length = length; cx->timeout = jiffies + HZ; - hash_add(orangefs_inode->xattr_cache, &cx->node, - xattr_key(cx->key)); + hlist_add_head(&cx->node, + &orangefs_inode->xattr_cache[xattr_key(cx->key)]); } } From 11f6bce77e27e82015a0d044e6c1eec8b139831a Mon Sep 17 00:00:00 2001 From: Thorsten Blum Date: Thu, 18 Sep 2025 17:08:10 +0200 Subject: [PATCH 3/3] fs/orangefs: Replace kzalloc + copy_from_user with memdup_user_nul Replace kzalloc() followed by copy_from_user() with memdup_user_nul() to simplify and improve orangefs_debug_write(). Allocate only 'count' bytes instead of the maximum size ORANGEFS_MAX_DEBUG_STRING_LEN, and set 'buf' to NULL to ensure kfree(buf) still works. No functional changes intended. Signed-off-by: Thorsten Blum Signed-off-by: Mike Marshall --- fs/orangefs/orangefs-debugfs.c | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/fs/orangefs/orangefs-debugfs.c b/fs/orangefs/orangefs-debugfs.c index 1c375fb65018..79267b3419f2 100644 --- a/fs/orangefs/orangefs-debugfs.c +++ b/fs/orangefs/orangefs-debugfs.c @@ -440,14 +440,13 @@ static ssize_t orangefs_debug_write(struct file *file, count = ORANGEFS_MAX_DEBUG_STRING_LEN; } - buf = kzalloc(ORANGEFS_MAX_DEBUG_STRING_LEN, GFP_KERNEL); - if (!buf) - goto out; - - if (copy_from_user(buf, ubuf, count - 1)) { + buf = memdup_user_nul(ubuf, count - 1); + if (IS_ERR(buf)) { gossip_debug(GOSSIP_DEBUGFS_DEBUG, - "%s: copy_from_user failed!\n", + "%s: memdup_user_nul failed!\n", __func__); + rc = PTR_ERR(buf); + buf = NULL; goto out; }