Commit a06a4dc3a08201ff6a8a958f935b3cbf7744115f

Authored by Neil Horman
Committed by Linus Torvalds
1 parent 065add3941

kmod: add init function to usermodehelper

About 6 months ago, I made a set of changes to how the core-dump-to-a-pipe
feature in the kernel works.  We had reports of several races, including
some reports of apps bypassing our recursion check so that a process that
was forked as part of a core_pattern setup could infinitely crash and
refork until the system crashed.

We fixed those by improving our recursion checks.  The new check basically
refuses to fork a process if its core limit is zero, which works well.

Unfortunately, I've been getting grief from maintainer of user space
programs that are inserted as the forked process of core_pattern.  They
contend that in order for their programs (such as abrt and apport) to
work, all the running processes in a system must have their core limits
set to a non-zero value, to which I say 'yes'.  I did this by design, and
think thats the right way to do things.

But I've been asked to ease this burden on user space enough times that I
thought I would take a look at it.  The first suggestion was to make the
recursion check fail on a non-zero 'special' number, like one.  That way
the core collector process could set its core size ulimit to 1, and enable
the kernel's recursion detection.  This isn't a bad idea on the surface,
but I don't like it since its opt-in, in that if a program like abrt or
apport has a bug and fails to set such a core limit, we're left with a
recursively crashing system again.

So I've come up with this.  What I've done is modify the
call_usermodehelper api such that an extra parameter is added, a function
pointer which will be called by the user helper task, after it forks, but
before it exec's the required process.  This will give the caller the
opportunity to get a call back in the processes context, allowing it to do
whatever it needs to to the process in the kernel prior to exec-ing the
user space code.  In the case of do_coredump, this callback is ues to set
the core ulimit of the helper process to 1.  This elimnates the opt-in
problem that I had above, as it allows the ulimit for core sizes to be set
to the value of 1, which is what the recursion check looks for in
do_coredump.

This patch:

Create new function call_usermodehelper_fns() and allow it to assign both
an init and cleanup function, as we'll as arbitrary data.

The init function is called from the context of the forked process and
allows for customization of the helper process prior to calling exec.  Its
return code gates the continuation of the process, or causes its exit.
Also add an arbitrary data pointer to the subprocess_info struct allowing
for data to be passed from the caller to the new process, and the
subsequent cleanup process

Also, use this patch to cleanup the cleanup function.  It currently takes
an argp and envp pointer for freeing, which is ugly.  Lets instead just
make the subprocess_info structure public, and pass that to the cleanup
and init routines

Signed-off-by: Neil Horman <nhorman@tuxdriver.com>
Reviewed-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>

Showing 3 changed files with 73 additions and 35 deletions Side-by-side Diff

include/linux/kmod.h
... ... @@ -23,6 +23,7 @@
23 23 #include <linux/stddef.h>
24 24 #include <linux/errno.h>
25 25 #include <linux/compiler.h>
  26 +#include <linux/workqueue.h>
26 27  
27 28 #define KMOD_PATH_LEN 256
28 29  
29 30  
... ... @@ -45,8 +46,28 @@
45 46  
46 47 struct key;
47 48 struct file;
48   -struct subprocess_info;
49 49  
  50 +enum umh_wait {
  51 + UMH_NO_WAIT = -1, /* don't wait at all */
  52 + UMH_WAIT_EXEC = 0, /* wait for the exec, but not the process */
  53 + UMH_WAIT_PROC = 1, /* wait for the process to complete */
  54 +};
  55 +
  56 +struct subprocess_info {
  57 + struct work_struct work;
  58 + struct completion *complete;
  59 + struct cred *cred;
  60 + char *path;
  61 + char **argv;
  62 + char **envp;
  63 + enum umh_wait wait;
  64 + int retval;
  65 + struct file *stdin;
  66 + int (*init)(struct subprocess_info *info);
  67 + void (*cleanup)(struct subprocess_info *info);
  68 + void *data;
  69 +};
  70 +
50 71 /* Allocate a subprocess_info structure */
51 72 struct subprocess_info *call_usermodehelper_setup(char *path, char **argv,
52 73 char **envp, gfp_t gfp_mask);
53 74  
... ... @@ -56,15 +77,11 @@
56 77 struct key *session_keyring);
57 78 int call_usermodehelper_stdinpipe(struct subprocess_info *sub_info,
58 79 struct file **filp);
59   -void call_usermodehelper_setcleanup(struct subprocess_info *info,
60   - void (*cleanup)(char **argv, char **envp));
  80 +void call_usermodehelper_setfns(struct subprocess_info *info,
  81 + int (*init)(struct subprocess_info *info),
  82 + void (*cleanup)(struct subprocess_info *info),
  83 + void *data);
61 84  
62   -enum umh_wait {
63   - UMH_NO_WAIT = -1, /* don't wait at all */
64   - UMH_WAIT_EXEC = 0, /* wait for the exec, but not the process */
65   - UMH_WAIT_PROC = 1, /* wait for the process to complete */
66   -};
67   -
68 85 /* Actually execute the sub-process */
69 86 int call_usermodehelper_exec(struct subprocess_info *info, enum umh_wait wait);
70 87  
71 88  
72 89  
73 90  
... ... @@ -73,15 +90,29 @@
73 90 void call_usermodehelper_freeinfo(struct subprocess_info *info);
74 91  
75 92 static inline int
76   -call_usermodehelper(char *path, char **argv, char **envp, enum umh_wait wait)
  93 +call_usermodehelper_fns(char *path, char **argv, char **envp,
  94 + enum umh_wait wait,
  95 + int (*init)(struct subprocess_info *info),
  96 + void (*cleanup)(struct subprocess_info *), void *data)
77 97 {
78 98 struct subprocess_info *info;
79 99 gfp_t gfp_mask = (wait == UMH_NO_WAIT) ? GFP_ATOMIC : GFP_KERNEL;
80 100  
81 101 info = call_usermodehelper_setup(path, argv, envp, gfp_mask);
  102 +
82 103 if (info == NULL)
83 104 return -ENOMEM;
  105 +
  106 + call_usermodehelper_setfns(info, init, cleanup, data);
  107 +
84 108 return call_usermodehelper_exec(info, wait);
  109 +}
  110 +
  111 +static inline int
  112 +call_usermodehelper(char *path, char **argv, char **envp, enum umh_wait wait)
  113 +{
  114 + return call_usermodehelper_fns(path, argv, envp, wait,
  115 + NULL, NULL, NULL);
85 116 }
86 117  
87 118 static inline int
... ... @@ -116,27 +116,16 @@
116 116  
117 117 trace_module_request(module_name, wait, _RET_IP_);
118 118  
119   - ret = call_usermodehelper(modprobe_path, argv, envp,
120   - wait ? UMH_WAIT_PROC : UMH_WAIT_EXEC);
  119 + ret = call_usermodehelper_fns(modprobe_path, argv, envp,
  120 + wait ? UMH_WAIT_PROC : UMH_WAIT_EXEC,
  121 + NULL, NULL, NULL);
  122 +
121 123 atomic_dec(&kmod_concurrent);
122 124 return ret;
123 125 }
124 126 EXPORT_SYMBOL(__request_module);
125 127 #endif /* CONFIG_MODULES */
126 128  
127   -struct subprocess_info {
128   - struct work_struct work;
129   - struct completion *complete;
130   - struct cred *cred;
131   - char *path;
132   - char **argv;
133   - char **envp;
134   - enum umh_wait wait;
135   - int retval;
136   - struct file *stdin;
137   - void (*cleanup)(char **argv, char **envp);
138   -};
139   -
140 129 /*
141 130 * This is the task which runs the usermode application
142 131 */
143 132  
... ... @@ -184,9 +173,16 @@
184 173 */
185 174 set_user_nice(current, 0);
186 175  
  176 + if (sub_info->init) {
  177 + retval = sub_info->init(sub_info);
  178 + if (retval)
  179 + goto fail;
  180 + }
  181 +
187 182 retval = kernel_execve(sub_info->path, sub_info->argv, sub_info->envp);
188 183  
189 184 /* Exec failed? */
  185 +fail:
190 186 sub_info->retval = retval;
191 187 do_exit(0);
192 188 }
... ... @@ -194,7 +190,7 @@
194 190 void call_usermodehelper_freeinfo(struct subprocess_info *info)
195 191 {
196 192 if (info->cleanup)
197   - (*info->cleanup)(info->argv, info->envp);
  193 + (*info->cleanup)(info);
198 194 if (info->cred)
199 195 put_cred(info->cred);
200 196 kfree(info);
201 197  
202 198  
203 199  
204 200  
205 201  
... ... @@ -406,21 +402,31 @@
406 402 EXPORT_SYMBOL(call_usermodehelper_setkeys);
407 403  
408 404 /**
409   - * call_usermodehelper_setcleanup - set a cleanup function
  405 + * call_usermodehelper_setfns - set a cleanup/init function
410 406 * @info: a subprocess_info returned by call_usermodehelper_setup
411 407 * @cleanup: a cleanup function
  408 + * @init: an init function
  409 + * @data: arbitrary context sensitive data
412 410 *
413   - * The cleanup function is just befor ethe subprocess_info is about to
  411 + * The init function is used to customize the helper process prior to
  412 + * exec. A non-zero return code causes the process to error out, exit,
  413 + * and return the failure to the calling process
  414 + *
  415 + * The cleanup function is just before ethe subprocess_info is about to
414 416 * be freed. This can be used for freeing the argv and envp. The
415 417 * Function must be runnable in either a process context or the
416 418 * context in which call_usermodehelper_exec is called.
417 419 */
418   -void call_usermodehelper_setcleanup(struct subprocess_info *info,
419   - void (*cleanup)(char **argv, char **envp))
  420 +void call_usermodehelper_setfns(struct subprocess_info *info,
  421 + int (*init)(struct subprocess_info *info),
  422 + void (*cleanup)(struct subprocess_info *info),
  423 + void *data)
420 424 {
421 425 info->cleanup = cleanup;
  426 + info->init = init;
  427 + info->data = data;
422 428 }
423   -EXPORT_SYMBOL(call_usermodehelper_setcleanup);
  429 +EXPORT_SYMBOL(call_usermodehelper_setfns);
424 430  
425 431 /**
426 432 * call_usermodehelper_stdinpipe - set up a pipe to be used for stdin
... ... @@ -515,7 +521,8 @@
515 521 struct subprocess_info *sub_info;
516 522 int ret;
517 523  
518   - sub_info = call_usermodehelper_setup(path, argv, envp, GFP_KERNEL);
  524 + sub_info = call_usermodehelper_setup(path, argv, envp,
  525 + GFP_KERNEL);
519 526 if (sub_info == NULL)
520 527 return -ENOMEM;
521 528  
... ... @@ -1632,9 +1632,9 @@
1632 1632  
1633 1633 char poweroff_cmd[POWEROFF_CMD_PATH_LEN] = "/sbin/poweroff";
1634 1634  
1635   -static void argv_cleanup(char **argv, char **envp)
  1635 +static void argv_cleanup(struct subprocess_info *info)
1636 1636 {
1637   - argv_free(argv);
  1637 + argv_free(info->argv);
1638 1638 }
1639 1639  
1640 1640 /**
... ... @@ -1668,7 +1668,7 @@
1668 1668 goto out;
1669 1669 }
1670 1670  
1671   - call_usermodehelper_setcleanup(info, argv_cleanup);
  1671 + call_usermodehelper_setfns(info, NULL, argv_cleanup, NULL);
1672 1672  
1673 1673 ret = call_usermodehelper_exec(info, UMH_NO_WAIT);
1674 1674