From: Oleg Nesterov <oleg@redhat.com>
To: Andrew Morton <akpm@linux-foundation.org>
Cc: Linus Torvalds <torvalds@linux-foundation.org>,
Andi Kleen <andi@firstfloor.org>,
Lucas De Marchi <lucas.de.marchi@gmail.com>,
Benjamin Herrenschmidt <benh@kernel.crashing.org>,
Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
Paul Mackerras <paulus@samba.org>,
david@gibson.dropbear.id.au, Kees Cook <keescook@chromium.org>,
Serge Hallyn <serge.hallyn@canonical.com>,
"Rafael J. Wysocki" <rjw@sisk.pl>,
Feng Hong <hongfeng@marvell.com>,
Lucas De Marchi <lucas.demarchi@profusion.mobi>
Subject: [PATCH 1/2] teach argv_split() to handle the mutable strings
Date: Sat, 16 Mar 2013 21:23:53 +0100 [thread overview]
Message-ID: <20130316202353.GB18613@redhat.com> (raw)
In-Reply-To: <20130316202327.GA18613@redhat.com>
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_arg(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>
---
lib/argv_split.c | 82 +++++++++++++++++++++++-------------------------------
1 files changed, 35 insertions(+), 47 deletions(-)
diff --git a/lib/argv_split.c b/lib/argv_split.c
index 1e9a6cb..dfa534d 100644
--- a/lib/argv_split.c
+++ b/lib/argv_split.c
@@ -8,23 +8,17 @@
#include <linux/slab.h>
#include <linux/export.h>
-static const char *skip_arg(const char *cp)
-{
- while (*cp && !isspace(*cp))
- cp++;
-
- return cp;
-}
-
static int count_argc(const char *str)
{
int count = 0;
+ bool was_space;
- while (*str) {
- str = skip_spaces(str);
- if (*str) {
+ for (was_space = true; *str; str++) {
+ if (isspace(*str)) {
+ was_space = true;
+ } else if (was_space) {
+ was_space = false;
count++;
- str = skip_arg(str);
}
}
@@ -39,10 +33,7 @@ static int count_argc(const char *str)
*/
void argv_free(char **argv)
{
- char **p;
- for (p = argv; *p; p++)
- kfree(*p);
-
+ kfree(argv[-1]);
kfree(argv);
}
EXPORT_SYMBOL(argv_free);
@@ -62,40 +53,37 @@ EXPORT_SYMBOL(argv_free);
*/
char **argv_split(gfp_t gfp, const char *str, int *argcp)
{
- int argc = count_argc(str);
- char **argv = kzalloc(sizeof(*argv) * (argc+1), gfp);
- char **argvp;
-
- if (argv == NULL)
- goto out;
-
- if (argcp)
- *argcp = argc;
-
- argvp = argv;
-
- while (*str) {
- str = skip_spaces(str);
-
- if (*str) {
- const char *p = str;
- char *t;
-
- str = skip_arg(str);
+ char *argv_str;
+ bool was_space;
+ char **argv, **argv_ret;
+ int argc;
+
+ argv_str = kstrndup(str, KMALLOC_MAX_SIZE, gfp);
+ if (!argv_str)
+ return NULL;
+
+ argc = count_argc(argv_str);
+ argv = kmalloc(sizeof(*argv) * (argc + 2), gfp);
+ if (!argv) {
+ kfree(argv_str);
+ return NULL;
+ }
- t = kstrndup(p, str-p, gfp);
- if (t == NULL)
- goto fail;
- *argvp++ = t;
+ *argv = argv_str;
+ argv_ret = ++argv;
+ for (was_space = true; *argv_str; argv_str++) {
+ if (isspace(*argv_str)) {
+ was_space = true;
+ *argv_str = 0;
+ } else if (was_space) {
+ was_space = false;
+ *argv++ = argv_str;
}
}
- *argvp = NULL;
-
- out:
- return argv;
+ *argv = NULL;
- fail:
- argv_free(argv);
- return NULL;
+ if (argcp)
+ *argcp = argc;
+ return argv_ret;
}
EXPORT_SYMBOL(argv_split);
--
1.5.5.1
next prev parent reply other threads:[~2013-03-16 20:26 UTC|newest]
Thread overview: 29+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-03-12 3:25 Regression with orderly_poweroff() Benjamin Herrenschmidt
2013-03-12 14:46 ` Linus Torvalds
2013-03-12 17:46 ` Oleg Nesterov
2013-03-12 17:54 ` Lucas De Marchi
2013-03-12 18:22 ` Oleg Nesterov
2013-03-12 18:42 ` Linus Torvalds
2013-03-12 19:11 ` Oleg Nesterov
2013-03-12 19:20 ` Linus Torvalds
2013-03-12 20:35 ` Oleg Nesterov
2013-03-13 17:46 ` [PATCH 0/1] poweroff: change orderly_poweroff() to use schedule_work() Oleg Nesterov
2013-03-13 17:47 ` [PATCH 1/1] " Oleg Nesterov
2013-03-14 22:28 ` Andrew Morton
2013-03-15 16:39 ` Oleg Nesterov
2013-03-16 20:23 ` [PATCH 0/2] finx argv_split() vs sysctl race Oleg Nesterov
2013-03-16 20:23 ` Oleg Nesterov [this message]
2013-03-18 16:03 ` [PATCH v2 1/2] teach argv_split() to handle the mutable strings Oleg Nesterov
2013-03-18 21:53 ` [PATCH " Andrew Morton
2013-03-19 19:54 ` [PATCH -mm] argv_split-teach-it-to-handle-mutable-strings-fix-2 Oleg Nesterov
2013-03-16 20:24 ` [PATCH 2/2] set_task_comm: kill the pointless memset() + wmb() Oleg Nesterov
2013-03-16 20:32 ` [PATCH 0/2] finx argv_split() vs sysctl race Andi Kleen
2013-03-16 20:45 ` Oleg Nesterov
2013-03-16 20:56 ` Andi Kleen
2013-03-16 21:23 ` Oleg Nesterov
2013-03-16 21:54 ` Andi Kleen
2013-03-17 14:15 ` Oleg Nesterov
2013-03-18 16:03 ` Oleg Nesterov
2013-03-13 23:35 ` [PATCH 0/1] poweroff: change orderly_poweroff() to use schedule_work() Lucas De Marchi
2013-03-12 20:13 ` Regression with orderly_poweroff() Andi Kleen
2013-03-12 19:28 ` Benjamin Herrenschmidt
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20130316202353.GB18613@redhat.com \
--to=oleg@redhat.com \
--cc=akpm@linux-foundation.org \
--cc=andi@firstfloor.org \
--cc=benh@kernel.crashing.org \
--cc=david@gibson.dropbear.id.au \
--cc=hongfeng@marvell.com \
--cc=keescook@chromium.org \
--cc=linux-kernel@vger.kernel.org \
--cc=lucas.de.marchi@gmail.com \
--cc=lucas.demarchi@profusion.mobi \
--cc=paulus@samba.org \
--cc=rjw@sisk.pl \
--cc=serge.hallyn@canonical.com \
--cc=torvalds@linux-foundation.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.