Re: [RFC PATCH] kmod: add ability to swap root in usermode helper
From: Stanislav Kinsbursky
Date: Mon May 20 2013 - 04:56:45 EST
20.05.2013 12:42, Jeff Layton ÐÐÑÐÑ:
On Mon, 20 May 2013 11:00:37 +0400
Stanislav Kinsbursky <skinsbursky@xxxxxxxxxxxxx> 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 <skinsbursky@xxxxxxxxxxxxx>
---
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 <linux/rwsem.h>
#include <linux/ptrace.h>
#include <linux/async.h>
+#include <linux/fs_struct.h>
#include <asm/uaccess.h>
#include <trace/events/module.h>
@@ -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@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/