ve_devmnt_check() kstrdup()s the option string and then walks it with
strsep(&tmp_options, ","). strsep() advances the pointer it is given past
each returned token, so on the mismatch (-EPERM) path
kfree(tmp_options);
frees a pointer that points into the *middle* of the kstrdup() allocation
(or is NULL if the rejected token had no following separator). SLUB's
non-debug kfree() does no object-boundary check: it derives the cache from
the page and links the interior address onto the freelist as if it were a
valid object, corrupting the kmalloc-16 slab. A subsequent kmalloc() of
that size then hands out an interior pointer; the corruption typically
surfaces far away, e.g. as
usercopy: Kernel memory overwrite attempt detected to SLUB object
'kmalloc-16' (offset 11, size 15)! kernel BUG at mm/usercopy.c:102
in an unrelated kernfs write. KASAN pinpoints it directly:
BUG: KASAN: invalid-free in ve_devmnt_check+0x135
kfree <- ve_devmnt_check <- ve_devmnt_process
Allocated by task N: kstrdup <- ve_devmnt_check
This is reachable by any container root via mount(2)/fsconfig() with mount
data of the form "allowedopt,disallowedopt" for a device that has a
ve.mount_opts entry, so an unprivileged CT can corrupt host kernel slab.
Walk a throwaway cursor and always kfree() the original allocation. This
also fixes a leak on the success path, where strsep() leaves tmp_options
NULL and the old code did kfree(NULL), leaking the buffer on every allowed
container mount.
Fixes: 263467c864c5 ("ve/fs/devmnt: process mount options")
Feature: fs/mnt: hidden and allowed mount options in CT
https://virtuozzo.atlassian.net/browse/VSTOR-137234
Signed-off-by: Konstantin Khorenko <[email protected]>
---
fs/namespace.c | 13 +++++++++++--
1 file changed, 11 insertions(+), 2 deletions(-)
diff --git a/fs/namespace.c b/fs/namespace.c
index 566f11a222fc..c04b9f05f79f 100644
--- a/fs/namespace.c
+++ b/fs/namespace.c
@@ -3140,7 +3140,7 @@ static char *strstr_separated(char *haystack, char
*needle, char sep)
static int ve_devmnt_check(char *options, char *allowed)
{
char *p;
- char *tmp_options;
+ char *tmp_options, *tmp;
if (!options || !*options)
return 0;
@@ -3153,7 +3153,16 @@ static int ve_devmnt_check(char *options, char *allowed)
if (!tmp_options)
return -ENOMEM;
- while ((p = strsep(&tmp_options, ",")) != NULL) {
+ /*
+ * strsep() advances the pointer it is given past each token, so walk a
+ * throwaway cursor and always free the original kstrdup() allocation.
+ * Freeing the advanced pointer would kfree() an address in the middle
+ * of the object (an interior/invalid free that corrupts the slab), and
+ * on the success path strsep() leaves the cursor NULL so the original
+ * buffer would leak.
+ */
+ tmp = tmp_options;
+ while ((p = strsep(&tmp, ",")) != NULL) {
if (!*p)
continue;
--
2.47.1
_______________________________________________
Devel mailing list
[email protected]
https://lists.openvz.org/mailman/listinfo/devel
