ted 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 Signed-off-by: Sasha Levin --- LLM Generated explanations, may be completely bogus: YES - Fix makes `xattr_key()` terminate on the terminating NUL (`fs/orangefs/xattr.c:54-61`); the old `while (key)` loop never ended, so any user who ran `setfattr`/`getfattr` hit an infinite scan, leading to faults, hangs, and reported OrangeFS corruption. That is a serious, user-triggerable bug worth fixing in stable. - Added NULL guard in the same helper (`fs/orangefs/xattr.c:57-58`) keeps the cache code from hashing bogus pointers; this is defensive, tightly scoped, and carries no observable side effects for valid callers. - Entries now go straight into the bucket chosen by `xattr_key()` via `hlist_add_head()` (`fs/orangefs/xattr.c:180-181` and `fs/orangefs/xattr.c:234-235`). Previously `hash_add()` rehashed the already-reduced key, so `find_cached_xattr()`’s bucket walk (`fs/orangefs/xattr.c:71-82`) never located cached/negative entries, leaking a `kmalloc()` on every lookup until OOM (seen in xfstest generic/069). The new storage method matches the existing lookup/removal logic and the cleanup walk in `hash_for_each_safe()` (`fs/orangefs/super.c:115-131`), so it simply restores the intended caching behaviour. - Change set stays within `fs/orangefs/xattr.c`, doesn’t alter interfaces, and directly resolves the regression; without it OrangeFS remains trivially DoS-able and leaky. With it, functionality is restored and risk is low, making this a strong stable backport candidate. Suggested next step: rerun xfstest generic/069 on the target stable branch to confirm the leak is gone. 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 74ef75586f384..eee3c5ed1bbbb 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)]); } } -- 2.51.0[PATCH AUTOSEL 6.17-5.4] orangefs: fix xattr related buffer overflow...Sasha Levin undefinedpatches@lists.linux.dev, stable@vger.kernel.org undefined undefined undefined undefined undefined