Messages in this thread | | | Date | Sat, 13 Mar 2010 08:18:59 -0500 | From | Neil Horman <> | Subject | Re: [PATCH 1/2] exec: Rediff wrecked bits of usermodehelper overhaul |
| |
On Sat, Mar 13, 2010 at 12:33:48PM +0100, Oleg Nesterov wrote: > Neil, you confused the ordering ;) > > This is "2/2", the second patch. And the next email is "1/2". > Gahh! You're right! Dang it! Sorry. Andrew, want me to repost? I can fix the patch names while I'm at it :) Neil
> > On 03/12, Neil Horman wrote: > > > > Rediff of the usermodehelper changes after all the wreckage. This patch > > encompasses the patches that were formally in -mm as: > > > > kmod-replace-call_usermodehelper_pipe-with-use-of-umh-init-function-and-resolve-limit.patch > > kmod-replace-call_usermodehelper_pipe-with-use-of-umh-init-function-and-resolve-limitcleanup.patch > > > > kmod: replace call_usermodehelper_pipe() with use of umh init function and resolve limit > > > > The first patch in this series introduced an init function to the > > call_usermodehelper api so that processes could be customized by caller. > > This patch takes advantage of that fact, by customizing the helper in > > do_coredump to create the pipe and set its core limit to one (for our > > recusrsion check). This lets us clean up the previous uglyness in the > > usermodehelper internals and factor call_usermodehelper out entirely. > > While I'm at it, we can also modify the helper setup to look for a core > > limit value of 1 rather than zero for our recursion check > > > > Signed-off-by: Neil Horman <nhorman@tuxdriver.com> > > CC: Oleg Nesterov <oleg@redhat.com> > > CC: Andi Kleen <andi@firstfloor.org> > > > > > > fs/exec.c | 63 ++++++++++++++++++++++++++++++++++----- > > include/linux/kmod.h | 7 ---- > > kernel/kmod.c | 82 --------------------------------------------------- > > 3 files changed, 56 insertions(+), 96 deletions(-) > > > > diff --git a/fs/exec.c b/fs/exec.c > > index f351cdb..64a50b4 100644 > > --- a/fs/exec.c > > +++ b/fs/exec.c > > @@ -1784,6 +1784,50 @@ static void wait_for_dump_helpers(struct file *file) > > } > > > > > > +/* > > + * uhm_pipe_setup > > + * helper function to customize the process used > > + * to collect the core in userspace. Specifically > > + * it sets up a pipe and installs it as fd 0 (stdin) > > + * for the process. Returns 0 on success, or > > + * PTR_ERR on failure. > > + * Note that it also sets the core limit to 1. This > > + * is a special value that we use to trap recursive > > + * core dumps > > + */ > > +static int umh_pipe_setup(struct subprocess_info *info) > > +{ > > + struct file *rp, *wp; > > + struct fdtable *fdt; > > + struct coredump_params *cp = (struct coredump_params *)info->data; > > + struct files_struct *cf = current->files; > > + > > + wp = create_write_pipe(0); > > + if (IS_ERR(wp)) > > + return PTR_ERR(wp); > > + > > + rp = create_read_pipe(wp, 0); > > + if (IS_ERR(rp)) { > > + free_write_pipe(wp); > > + return PTR_ERR(rp); > > + } > > + > > + cp->file = wp; > > + > > + sys_close(0); > > + fd_install(0, rp); > > + spin_lock(&cf->file_lock); > > + fdt = files_fdtable(cf); > > + FD_SET(0, fdt->open_fds); > > + FD_CLR(0, fdt->close_on_exec); > > + spin_unlock(&cf->file_lock); > > + > > + /* and disallow core files too */ > > + current->signal->rlim[RLIMIT_CORE] = (struct rlimit){1, 1}; > > + > > + return 0; > > +} > > + > > void do_coredump(long signr, int exit_code, struct pt_regs *regs) > > { > > struct core_state core_state; > > @@ -1871,15 +1915,15 @@ void do_coredump(long signr, int exit_code, struct pt_regs *regs) > > goto fail_unlock; > > > > if (ispipe) { > > - if (cprm.limit == 0) { > > + if (cprm.limit == 1) { > > /* > > * Normally core limits are irrelevant to pipes, since > > * we're not writing to the file system, but we use > > - * cprm.limit of 0 here as a speacial value. Any > > - * non-zero limit gets set to RLIM_INFINITY below, but > > + * cprm.limit of 1 here as a speacial value. Any > > + * non-1 limit gets set to RLIM_INFINITY below, but > > * a limit of 0 skips the dump. This is a consistent > > * way to catch recursive crashes. We can still crash > > - * if the core_pattern binary sets RLIM_CORE = !0 > > + * if the core_pattern binary sets RLIM_CORE = !1 > > * but it runs as root, and can do lots of stupid things > > * Note that we use task_tgid_vnr here to grab the pid > > * of the process group leader. That way we get the > > @@ -1887,7 +1931,7 @@ void do_coredump(long signr, int exit_code, struct pt_regs *regs) > > * core_pattern process dies. > > */ > > printk(KERN_WARNING > > - "Process %d(%s) has RLIMIT_CORE set to 0\n", > > + "Process %d(%s) has RLIMIT_CORE set to 1\n", > > task_tgid_vnr(current), current->comm); > > printk(KERN_WARNING "Aborting core\n"); > > goto fail_unlock; > > @@ -1911,8 +1955,13 @@ void do_coredump(long signr, int exit_code, struct pt_regs *regs) > > cprm.limit = RLIM_INFINITY; > > > > /* SIGPIPE can happen, but it's just never processed */ > > - if (call_usermodehelper_pipe(helper_argv[0], helper_argv, NULL, > > - &cprm.file)) { > > + cprm.file = NULL; > > + if (call_usermodehelper_fns(helper_argv[0], helper_argv, NULL, > > + UMH_WAIT_EXEC, umh_pipe_setup, > > + NULL, &cprm)) { > > + if (cprm.file) > > + filp_close(cprm.file, NULL); > > + > > printk(KERN_INFO "Core dump to %s pipe failed\n", > > corename); > > goto fail_dropcount; > > diff --git a/include/linux/kmod.h b/include/linux/kmod.h > > index f9edf63..5c05877 100644 > > --- a/include/linux/kmod.h > > +++ b/include/linux/kmod.h > > @@ -62,7 +62,6 @@ struct subprocess_info { > > char **envp; > > enum umh_wait wait; > > int retval; > > - struct file *stdin; > > int (*init)(struct subprocess_info *info); > > void (*cleanup)(struct subprocess_info *info); > > void *data; > > @@ -75,8 +74,6 @@ struct subprocess_info *call_usermodehelper_setup(char *path, char **argv, > > /* Set various pieces of state into the subprocess_info structure */ > > void call_usermodehelper_setkeys(struct subprocess_info *info, > > struct key *session_keyring); > > -int call_usermodehelper_stdinpipe(struct subprocess_info *sub_info, > > - struct file **filp); > > void call_usermodehelper_setfns(struct subprocess_info *info, > > int (*init)(struct subprocess_info *info), > > void (*cleanup)(struct subprocess_info *info), > > @@ -132,10 +129,6 @@ call_usermodehelper_keys(char *path, char **argv, char **envp, > > > > extern void usermodehelper_init(void); > > > > -struct file; > > -extern int call_usermodehelper_pipe(char *path, char *argv[], char *envp[], > > - struct file **filp); > > - > > extern int usermodehelper_disable(void); > > extern void usermodehelper_enable(void); > > > > diff --git a/kernel/kmod.c b/kernel/kmod.c > > index 531ef62..d154454 100644 > > --- a/kernel/kmod.c > > +++ b/kernel/kmod.c > > @@ -147,23 +147,6 @@ static int ____call_usermodehelper(void *data) > > commit_creds(sub_info->cred); > > sub_info->cred = NULL; > > > > - /* Install input pipe when needed */ > > - if (sub_info->stdin) { > > - struct files_struct *f = current->files; > > - struct fdtable *fdt; > > - /* no races because files should be private here */ > > - sys_close(0); > > - fd_install(0, sub_info->stdin); > > - spin_lock(&f->file_lock); > > - fdt = files_fdtable(f); > > - FD_SET(0, fdt->open_fds); > > - FD_CLR(0, fdt->close_on_exec); > > - spin_unlock(&f->file_lock); > > - > > - /* and disallow core files too */ > > - current->signal->rlim[RLIMIT_CORE] = (struct rlimit){0, 0}; > > - } > > - > > /* We can run anywhere, unlike our parent keventd(). */ > > set_cpus_allowed_ptr(current, cpu_all_mask); > > > > @@ -429,35 +412,6 @@ void call_usermodehelper_setfns(struct subprocess_info *info, > > EXPORT_SYMBOL(call_usermodehelper_setfns); > > > > /** > > - * call_usermodehelper_stdinpipe - set up a pipe to be used for stdin > > - * @sub_info: a subprocess_info returned by call_usermodehelper_setup > > - * @filp: set to the write-end of a pipe > > - * > > - * This constructs a pipe, and sets the read end to be the stdin of the > > - * subprocess, and returns the write-end in *@filp. > > - */ > > -int call_usermodehelper_stdinpipe(struct subprocess_info *sub_info, > > - struct file **filp) > > -{ > > - struct file *f; > > - > > - f = create_write_pipe(0); > > - if (IS_ERR(f)) > > - return PTR_ERR(f); > > - *filp = f; > > - > > - f = create_read_pipe(f, 0); > > - if (IS_ERR(f)) { > > - free_write_pipe(*filp); > > - return PTR_ERR(f); > > - } > > - sub_info->stdin = f; > > - > > - return 0; > > -} > > -EXPORT_SYMBOL(call_usermodehelper_stdinpipe); > > - > > -/** > > * call_usermodehelper_exec - start a usermode application > > * @sub_info: information about the subprocessa > > * @wait: wait for the application to finish and return status. > > @@ -504,42 +458,6 @@ unlock: > > } > > EXPORT_SYMBOL(call_usermodehelper_exec); > > > > -/** > > - * call_usermodehelper_pipe - call a usermode helper process with a pipe stdin > > - * @path: path to usermode executable > > - * @argv: arg vector for process > > - * @envp: environment for process > > - * @filp: set to the write-end of a pipe > > - * > > - * This is a simple wrapper which executes a usermode-helper function > > - * with a pipe as stdin. It is implemented entirely in terms of > > - * lower-level call_usermodehelper_* functions. > > - */ > > -int call_usermodehelper_pipe(char *path, char **argv, char **envp, > > - struct file **filp) > > -{ > > - struct subprocess_info *sub_info; > > - int ret; > > - > > - sub_info = call_usermodehelper_setup(path, argv, envp, > > - GFP_KERNEL); > > - if (sub_info == NULL) > > - return -ENOMEM; > > - > > - ret = call_usermodehelper_stdinpipe(sub_info, filp); > > - if (ret < 0) { > > - call_usermodehelper_freeinfo(sub_info); > > - return ret; > > - } > > - > > - ret = call_usermodehelper_exec(sub_info, UMH_WAIT_EXEC); > > - if (ret < 0) /* Failed to execute helper, close pipe */ > > - filp_close(*filp, NULL); > > - > > - return ret; > > -} > > -EXPORT_SYMBOL(call_usermodehelper_pipe); > > - > > void __init usermodehelper_init(void) > > { > > khelper_wq = create_singlethread_workqueue("khelper"); > >
| |