Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752561Ab3ETI4m (ORCPT ); Mon, 20 May 2013 04:56:42 -0400 Received: from relay.parallels.com ([195.214.232.42]:41662 "EHLO relay.parallels.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750798Ab3ETI4l (ORCPT ); Mon, 20 May 2013 04:56:41 -0400 Message-ID: <5199E533.8030608@parallels.com> Date: Mon, 20 May 2013 12:56:19 +0400 From: Stanislav Kinsbursky User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:17.0) Gecko/20130329 Thunderbird/17.0.5 MIME-Version: 1.0 To: Jeff Layton CC: , , , , , , , , Subject: Re: [RFC PATCH] kmod: add ability to swap root in usermode helper References: <20130520070017.7957.9224.stgit@localhost.localdomain> <20130520044254.42b1cd88@tlielax.poochiereds.net> In-Reply-To: <20130520044254.42b1cd88@tlielax.poochiereds.net> Content-Type: text/plain; charset="UTF-8"; format=flowed Content-Transfer-Encoding: 8bit X-Originating-IP: [10.30.18.163] Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 7839 Lines: 211 20.05.2013 12:42, Jeff Layton пишет: > On Mon, 20 May 2013 11:00:37 +0400 > Stanislav Kinsbursky wrote: > >> Usermode helper executes all binaries in global "init" root context. This >> doesn't allow to call to call the binary from other root (for example in a >> container). >> Currently, containerized NFS server requires an ability to execute a binary in >> a other context, than "init" root (UMH is used for client recovery tracking). >> This patch adds root swap to ____call_usermodehelper(), if non-NULL root was >> passed as a part of subprocess_info data, and a call_usermodehelper_root() >> helper, which accept root as a parameter. Root path reference must be hold by >> the caller, since it will be on UMH thread exit. >> > > I assume you mean that it will be put on thread exit. > Yes, sure. Sorry. >> Signed-off-by: Stanislav Kinsbursky >> --- >> include/linux/kmod.h | 9 +++++++++ >> kernel/kmod.c | 41 ++++++++++++++++++++++++++++++++++------- >> 2 files changed, 43 insertions(+), 7 deletions(-) >> >> diff --git a/include/linux/kmod.h b/include/linux/kmod.h >> index 0555cc6..0b041c7 100644 >> --- a/include/linux/kmod.h >> +++ b/include/linux/kmod.h >> @@ -64,12 +64,21 @@ struct subprocess_info { >> int (*init)(struct subprocess_info *info, struct cred *new); >> void (*cleanup)(struct subprocess_info *info); >> void *data; >> + struct path *root; >> }; >> >> extern int >> +call_usermodehelper_root(char *path, char **argv, char **envp, int wait, >> + struct path *root); >> +extern int >> call_usermodehelper(char *path, char **argv, char **envp, int wait); >> >> extern struct subprocess_info * >> +call_usermodehelper_setup_root(char *path, char **argv, char **envp, gfp_t gfp_mask, >> + int (*init)(struct subprocess_info *info, struct cred *new), >> + void (*cleanup)(struct subprocess_info *), void *data, >> + struct path *root); >> +extern struct subprocess_info * >> call_usermodehelper_setup(char *path, char **argv, char **envp, gfp_t gfp_mask, >> int (*init)(struct subprocess_info *info, struct cred *new), >> void (*cleanup)(struct subprocess_info *), void *data); >> diff --git a/kernel/kmod.c b/kernel/kmod.c >> index 1296e72..1b41f2c 100644 >> --- a/kernel/kmod.c >> +++ b/kernel/kmod.c >> @@ -39,6 +39,7 @@ >> #include >> #include >> #include >> +#include >> #include >> >> #include >> @@ -215,6 +216,9 @@ static int ____call_usermodehelper(void *data) >> */ >> set_user_nice(current, 0); >> >> + if (sub_info->root) >> + set_fs_root(current->fs, sub_info->root); >> + >> retval = -ENOMEM; >> new = prepare_kernel_cred(current); >> if (!new) >> @@ -505,7 +509,7 @@ static void helper_unlock(void) >> } >> >> /** >> - * call_usermodehelper_setup - prepare to call a usermode helper >> + * call_usermodehelper_setup_root - prepare to call a usermode helper >> * @path: path to usermode executable >> * @argv: arg vector for process >> * @envp: environment for process >> @@ -513,6 +517,7 @@ static void helper_unlock(void) >> * @cleanup: a cleanup function >> * @init: an init function >> * @data: arbitrary context sensitive data >> + * @root: fs root to swap to before binary execution >> * >> * Returns either %NULL on allocation failure, or a subprocess_info >> * structure. This should be passed to call_usermodehelper_exec to >> @@ -527,11 +532,11 @@ static void helper_unlock(void) >> * Function must be runnable in either a process context or the >> * context in which call_usermodehelper_exec is called. >> */ > > It would be best to spell out the need for the caller to hold a > reference in the above kerneldoc comment, since that's what people will > look at when they write new code that calls this. > Sound reasonable, thanks. >> -struct subprocess_info *call_usermodehelper_setup(char *path, char **argv, >> +struct subprocess_info *call_usermodehelper_setup_root(char *path, char **argv, >> char **envp, gfp_t gfp_mask, >> int (*init)(struct subprocess_info *info, struct cred *new), >> void (*cleanup)(struct subprocess_info *info), >> - void *data) >> + void *data, struct path *root) >> { >> struct subprocess_info *sub_info; >> sub_info = kzalloc(sizeof(struct subprocess_info), gfp_mask); >> @@ -546,9 +551,22 @@ struct subprocess_info *call_usermodehelper_setup(char *path, char **argv, >> sub_info->cleanup = cleanup; >> sub_info->init = init; >> sub_info->data = data; >> + >> + sub_info->root = root; >> out: >> return sub_info; >> } >> +EXPORT_SYMBOL(call_usermodehelper_setup_root); >> + > > nit: do we really need a new exported helper function here? There > aren't that many callers of call_usermodehelper_setup, so you could > just add the argument and fix up the existing callers. > Or maybe even define call_usermodehelper_setup as a macro? >> +struct subprocess_info *call_usermodehelper_setup(char *path, char **argv, >> + char **envp, gfp_t gfp_mask, >> + int (*init)(struct subprocess_info *info, struct cred *new), >> + void (*cleanup)(struct subprocess_info *info), >> + void *data) >> +{ >> + return call_usermodehelper_setup_root(path, argv, envp, gfp_mask, init, >> + cleanup, data, NULL); >> +} >> EXPORT_SYMBOL(call_usermodehelper_setup); >> >> /** >> @@ -617,7 +635,7 @@ unlock: >> EXPORT_SYMBOL(call_usermodehelper_exec); >> >> /** >> - * call_usermodehelper() - prepare and start a usermode application >> + * call_usermodehelper_root() - prepare and start a usermode application >> * @path: path to usermode executable >> * @argv: arg vector for process >> * @envp: environment for process >> @@ -625,24 +643,33 @@ EXPORT_SYMBOL(call_usermodehelper_exec); >> * when UMH_NO_WAIT don't wait at all, but you get no useful error back >> * when the program couldn't be exec'ed. This makes it safe to call >> * from interrupt context. >> + * @root: fs root to swap to before binary execution >> * >> * This function is the equivalent to use call_usermodehelper_setup() and >> * call_usermodehelper_exec(). >> */ > > I'd also spell out the need for the caller to hold an extra reference > to @root here. > Right, thanks. >> -int call_usermodehelper(char *path, char **argv, char **envp, int wait) >> +int call_usermodehelper_root(char *path, char **argv, char **envp, int wait, >> + struct path *root) >> { >> struct subprocess_info *info; >> gfp_t gfp_mask = (wait == UMH_NO_WAIT) ? GFP_ATOMIC : GFP_KERNEL; >> >> - info = call_usermodehelper_setup(path, argv, envp, gfp_mask, >> - NULL, NULL, NULL); >> + info = call_usermodehelper_setup_root(path, argv, envp, gfp_mask, >> + NULL, NULL, NULL, root); >> if (info == NULL) >> return -ENOMEM; >> >> return call_usermodehelper_exec(info, wait); >> } >> +EXPORT_SYMBOL(call_usermodehelper_root); >> + >> +int call_usermodehelper(char *path, char **argv, char **envp, int wait) >> +{ >> + return call_usermodehelper_root(path, argv, envp, wait, NULL); >> +} >> EXPORT_SYMBOL(call_usermodehelper); >> >> + >> static int proc_cap_handler(struct ctl_table *table, int write, >> void __user *buffer, size_t *lenp, loff_t *ppos) >> { >> > > Looks reasonable otherwise, you can add my Acked-by when you fix up the > comments. Nice work... > -- Best regards, Stanislav Kinsbursky -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/