On 05/10, Lucas De Marchi wrote:
>
> Oh, right. Forgot about that. And this patch set should have been sent
> as RFC, since I'm interested in feedback about the idea. What do you
> think?
Well, personally I think it would be better to use kasprintf(), see the
patch I sent (it is actually wrong, needs kfree(args) before return).
Or. How about the patch below? It should be split into 2 changes:
1. Introduce __argv_split(). It can have more callers, for
example do_coredump() and ftrace_function_filter_re()
can use it to avoid kstrndup() + kfree().
2. Change call_modprobe() to use kasprintf() + __argv_split().
uncompiled/untested.
Oleg.
--- x/lib/argv_split.c
+++ x/lib/argv_split.c
@@ -39,31 +39,15 @@ void argv_free(char **argv)
}
EXPORT_SYMBOL(argv_free);
-/**
- * argv_split - split a string at whitespace, returning an argv
- * @gfp: the GFP mask used to allocate memory
- * @str: the string to be split
- * @argcp: returned argument count
- *
- * Returns an array of pointers to strings which are split out from
- * @str. This is performed by strictly splitting on white-space; no
- * quote processing is performed. Multiple whitespace characters are
- * considered to be a single argument separator. The returned array
- * is always NULL-terminated. Returns NULL on memory allocation
- * failure.
- *
- * The source string at `str' may be undergoing concurrent alteration via
- * userspace sysctl activity (at least). The argv_split() implementation
- * attempts to handle this gracefully by taking a local copy to work on.
+/*
+ * @argv_str should be kmalloc'ed by the caller, freed by this func.
*/
-char **argv_split(gfp_t gfp, const char *str, int *argcp)
+char **__argv_split(gfp_t gfp, const char *argv_str, int *argcp)
{
- char *argv_str;
bool was_space;
char **argv, **argv_ret;
int argc;
- argv_str = kstrndup(str, KMALLOC_MAX_SIZE - 1, gfp);
if (!argv_str)
return NULL;
@@ -91,4 +75,28 @@ char **argv_split(gfp_t gfp, const char
*argcp = argc;
return argv_ret;
}
+
+/**
+ * argv_split - split a string at whitespace, returning an argv
+ * @gfp: the GFP mask used to allocate memory
+ * @str: the string to be split
+ * @argcp: returned argument count
+ *
+ * Returns an array of pointers to strings which are split out from
+ * @str. This is performed by strictly splitting on white-space; no
+ * quote processing is performed. Multiple whitespace characters are
+ * considered to be a single argument separator. The returned array
+ * is always NULL-terminated. Returns NULL on memory allocation
+ * failure.
+ *
+ * The source string at `str' may be undergoing concurrent alteration via
+ * userspace sysctl activity (at least). The argv_split() implementation
+ * attempts to handle this gracefully by taking a local copy to work on.
+ */
+char **argv_split(gfp_t gfp, const char *str, int *argcp)
+{
+ char *dup = kstrndup(str, KMALLOC_MAX_SIZE - 1, gfp);
+ return __argv_split(gfp, dup, argcp);
+}
+
EXPORT_SYMBOL(argv_split);
--- x/kernel/kmod.c
+++ x/kernel/kmod.c
@@ -64,20 +64,16 @@ static DECLARE_RWSEM(umhelper_sem);
#ifdef CONFIG_MODULES
-/*
- modprobe_path is set via /proc/sys.
-*/
-char modprobe_path[KMOD_PATH_LEN] = "/sbin/modprobe";
+/* modprobe_path is set via /proc/sys/kernel/modprobe */
+char modprobe_path[KMOD_PATH_LEN] = "/sbin/modprobe -q --";
static void free_modprobe_argv(struct subprocess_info *info)
{
- kfree(info->argv[3]); /* check call_modprobe() */
- kfree(info->argv);
+ argv_free(info->argv);
}
static int call_modprobe(char *module_name, int wait)
{
- struct subprocess_info *info;
static char *envp[] = {
"HOME=/",
"TERM=linux",
@@ -85,31 +81,24 @@ static int call_modprobe(char *module_na
NULL
};
- char **argv = kmalloc(sizeof(char *[5]), GFP_KERNEL);
+ struct subprocess_info *info;
+ char **argv;
+ char *args;
+
+ args = kasprintf(GFP_KERNEL, "%s %s", modprobe_path, module_name);
+ argv = __argv_split(GFP_KERNEL, args, NULL);
if (!argv)
goto out;
- module_name = kstrdup(module_name, GFP_KERNEL);
- if (!module_name)
- goto free_argv;
-
- argv[0] = modprobe_path;
- argv[1] = "-q";
- argv[2] = "--";
- argv[3] = module_name; /* check free_modprobe_argv() */
- argv[4] = NULL;
-
info = call_usermodehelper_setup(modprobe_path, argv, envp, GFP_KERNEL,
NULL, free_modprobe_argv, NULL);
if (!info)
- goto free_module_name;
+ goto free_argv;
return call_usermodehelper_exec(info, wait | UMH_KILLABLE);
-free_module_name:
- kfree(module_name);
free_argv:
- kfree(argv);
+ argv_free(argv);
out:
return -ENOMEM;
}
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
