Linux kernel mirror (for testing) git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel os linux

argv_split(): teach it to handle mutable strings

argv_split() allocates argv[count_argc(str)] array and assumes that it
will find the same number of arguments later. This is obviously wrong if
this string can be changed, say, by sysctl.

With this patch argv_split() kstrndup's the whole string and does not
split it, we simply replace the spaces with zeroes and keep the allocated
memory in argv[-1] for argv_free(arg).

We do not use argv[0] because:

- str can be all-spaces or empty. In fact this case is fine,
we could kfree() it before return, but:

- str can have a space at the start, and we can not rely on
kstrndup(skip_spaces(str)) because it can equally race if
this string is mutable.

Also, simplify count_argc() and kill the no longer used skip_arg().

Signed-off-by: Oleg Nesterov <oleg@redhat.com>
Cc: Andi Kleen <andi@firstfloor.org>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>

authored by

Oleg Nesterov and committed by
Linus Torvalds
095d141b 30493cc9

+40 -47
+40 -47
lib/argv_split.c
··· 8 8 #include <linux/slab.h> 9 9 #include <linux/export.h> 10 10 11 - static const char *skip_arg(const char *cp) 12 - { 13 - while (*cp && !isspace(*cp)) 14 - cp++; 15 - 16 - return cp; 17 - } 18 - 19 11 static int count_argc(const char *str) 20 12 { 21 13 int count = 0; 14 + bool was_space; 22 15 23 - while (*str) { 24 - str = skip_spaces(str); 25 - if (*str) { 16 + for (was_space = true; *str; str++) { 17 + if (isspace(*str)) { 18 + was_space = true; 19 + } else if (was_space) { 20 + was_space = false; 26 21 count++; 27 - str = skip_arg(str); 28 22 } 29 23 } 30 24 ··· 33 39 */ 34 40 void argv_free(char **argv) 35 41 { 36 - char **p; 37 - for (p = argv; *p; p++) 38 - kfree(*p); 39 - 42 + argv--; 43 + kfree(argv[0]); 40 44 kfree(argv); 41 45 } 42 46 EXPORT_SYMBOL(argv_free); ··· 51 59 * considered to be a single argument separator. The returned array 52 60 * is always NULL-terminated. Returns NULL on memory allocation 53 61 * failure. 62 + * 63 + * The source string at `str' may be undergoing concurrent alteration via 64 + * userspace sysctl activity (at least). The argv_split() implementation 65 + * attempts to handle this gracefully by taking a local copy to work on. 54 66 */ 55 67 char **argv_split(gfp_t gfp, const char *str, int *argcp) 56 68 { 57 - int argc = count_argc(str); 58 - char **argv = kzalloc(sizeof(*argv) * (argc+1), gfp); 59 - char **argvp; 69 + char *argv_str; 70 + bool was_space; 71 + char **argv, **argv_ret; 72 + int argc; 60 73 61 - if (argv == NULL) 62 - goto out; 74 + argv_str = kstrndup(str, KMALLOC_MAX_SIZE - 1, gfp); 75 + if (!argv_str) 76 + return NULL; 77 + 78 + argc = count_argc(argv_str); 79 + argv = kmalloc(sizeof(*argv) * (argc + 2), gfp); 80 + if (!argv) { 81 + kfree(argv_str); 82 + return NULL; 83 + } 84 + 85 + *argv = argv_str; 86 + argv_ret = ++argv; 87 + for (was_space = true; *argv_str; argv_str++) { 88 + if (isspace(*argv_str)) { 89 + was_space = true; 90 + *argv_str = 0; 91 + } else if (was_space) { 92 + was_space = false; 93 + *argv++ = argv_str; 94 + } 95 + } 96 + *argv = NULL; 63 97 64 98 if (argcp) 65 99 *argcp = argc; 66 - 67 - argvp = argv; 68 - 69 - while (*str) { 70 - str = skip_spaces(str); 71 - 72 - if (*str) { 73 - const char *p = str; 74 - char *t; 75 - 76 - str = skip_arg(str); 77 - 78 - t = kstrndup(p, str-p, gfp); 79 - if (t == NULL) 80 - goto fail; 81 - *argvp++ = t; 82 - } 83 - } 84 - *argvp = NULL; 85 - 86 - out: 87 - return argv; 88 - 89 - fail: 90 - argv_free(argv); 91 - return NULL; 100 + return argv_ret; 92 101 } 93 102 EXPORT_SYMBOL(argv_split);