mirror of
https://github.com/raspberrypi/linux.git
synced 2025-12-06 10:00:17 +00:00
orangefs: fix xattr related buffer overflow...
[ Upstream commit 025e880759 ]
Willy Tarreau <w@1wt.eu> forwarded me a message from
Disclosure <disclosure@aisle.com> 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 <hubcap@omnibond.com>
Reported-by: Stanislav Fort of Aisle Research <stanislav.fort@aisle.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
This commit is contained in:
committed by
Greg Kroah-Hartman
parent
6af18a2c0c
commit
c2ca015ac1
@@ -54,7 +54,9 @@ static inline int convert_to_internal_xattr_flags(int setxattr_flags)
|
|||||||
static unsigned int xattr_key(const char *key)
|
static unsigned int xattr_key(const char *key)
|
||||||
{
|
{
|
||||||
unsigned int i = 0;
|
unsigned int i = 0;
|
||||||
while (key)
|
if (!key)
|
||||||
|
return 0;
|
||||||
|
while (*key)
|
||||||
i += *key++;
|
i += *key++;
|
||||||
return i % 16;
|
return i % 16;
|
||||||
}
|
}
|
||||||
@@ -175,8 +177,8 @@ ssize_t orangefs_inode_getxattr(struct inode *inode, const char *name,
|
|||||||
cx->length = -1;
|
cx->length = -1;
|
||||||
cx->timeout = jiffies +
|
cx->timeout = jiffies +
|
||||||
orangefs_getattr_timeout_msecs*HZ/1000;
|
orangefs_getattr_timeout_msecs*HZ/1000;
|
||||||
hash_add(orangefs_inode->xattr_cache, &cx->node,
|
hlist_add_head( &cx->node,
|
||||||
xattr_key(cx->key));
|
&orangefs_inode->xattr_cache[xattr_key(cx->key)]);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
goto out_release_op;
|
goto out_release_op;
|
||||||
@@ -229,8 +231,8 @@ ssize_t orangefs_inode_getxattr(struct inode *inode, const char *name,
|
|||||||
memcpy(cx->val, buffer, length);
|
memcpy(cx->val, buffer, length);
|
||||||
cx->length = length;
|
cx->length = length;
|
||||||
cx->timeout = jiffies + HZ;
|
cx->timeout = jiffies + HZ;
|
||||||
hash_add(orangefs_inode->xattr_cache, &cx->node,
|
hlist_add_head(&cx->node,
|
||||||
xattr_key(cx->key));
|
&orangefs_inode->xattr_cache[xattr_key(cx->key)]);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user