Another attempt at contained helper execution
Subsystems (6)
v5 cover Another attempt at contained helper execution
Following Eric's comments and in light of the most recent nfs and keys patches here is yet another attempt at the basis of contained usermode helper execution.
Initially I thought that creating threads to be used when executing a helper wouldn't be feasible because the number of threads would be far too large but the recent nfs and keys patches make me think that's probably not the case.
There's more work to do on this, namely identifying already existing threads for a requested environment, error handling for environments that have gone away due to summary execution and similar. But I'd like to get feedback as to whether I'm on the right track and what I might be missing before spending more time on it.
---
Ian Kent (7):
kmod - add workqueue service thread store
kmod - teach usermodehelper to use service workqueues
nfsd - use service thread if not executing in init namespace nfs - cache_lib use service thread if not executing in init namespace nfs - objlayout use service thread if not executing in init namespace KEYS - use correct memory allocation flag in call_usermodehelper_keys()
KEYS: exec request key within service thread of key creator
fs/nfs/cache_lib.c | 7 + fs/nfs/objlayout/objlayout.c | 14 +++ fs/nfsd/netns.h | 3 + fs/nfsd/nfs4recover.c | 48 ++++++--- fs/nfsd/nfsctl.c | 6 + include/linux/key.h | 3 + include/linux/kmod.h | 8 ++ include/linux/sunrpc/cache.h | 2 kernel/kmod.c | 217 ++++++++++++++++++++++++++++++++++++++++-- net/sunrpc/cache.c | 5 + security/keys/gc.c | 2 security/keys/key.c | 5 + security/keys/request_key.c | 38 ++++++- 13 files changed, 323 insertions(+), 35 deletions(-)
--
Ian
RFC PATCH v5 1/7 kmod - add workqueue service thread store
From: Ian Kent <[email protected]>
Persistent use of process information is needed for contained execution in a namespace other than the root init namespace.
Use a simple random token as a key to create and store thread information in a hashed list for use by the usermode helper thread runner.
--- include/linux/kmod.h | 3 + kernel/kmod.c | 179 ++++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 181 insertions(+), 1 deletion(-)
include/linux/kmod.h +3 -0
@@ -66,6 +66,9 @@ struct subprocess_info {void *data; }; +extern int umh_wq_get_token (int token,const char *service); +extern void umh_wq_put_token (int token); +extern int call_usermodehelper (char *path,char **argv,char **envp,int wait);
kernel/kmod.c +178 -1
203 lines not on this page · raw message
Re: [RFC PATCH 5 1/7] kmod - add workqueue service thread store
Quoted 98 lines · On Tue, 31 Mar 2015 11:14:42 +0800 Ian Kent <[email protected]> wrote:
From: Ian Kent <[email protected]>
Persistent use of process information is needed for contained execution in a namespace other than the root init namespace.
Use a simple random token as a key to create and store thread information in a hashed list for use by the usermode helper thread runner.
Signed-off-by Ian Kent <[email protected]>Cc Benjamin Coddington <[email protected]>Cc Al Viro <[email protected]>Cc J. Bruce Fields <[email protected]>Cc David Howells <[email protected]>Cc Trond Myklebust <[email protected]>Cc Oleg Nesterov <[email protected]>Cc Eric W. Biederman <[email protected]>Cc Jeff Layton <[email protected]>--- include/linux/kmod.h | 3 + kernel/kmod.c | 179 ++++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 181 insertions(+), 1 deletion(-)include/linux/kmod.h +3
-0@@ -66,6 +66,9 @@ struct subprocess_info {void *data; }; +extern int umh_wq_get_token (int token,const char *service); +extern void umh_wq_put_token (int token); +extern int call_usermodehelper (char *path,char **argv,char **envp,int wait);kernel/kmod.c +39
-0@@ -40,13 +40,30 @@#include <linux/ptrace.h> #include <linux/async.h> #include <asm/uaccess.h> +#include <linux/hash.h> +#include <linux/list.h> +#include <linux/random.h> #include <trace/events/module.h> extern int max_threads; +#define KHELPER "khelper"static struct workqueue_struct *khelper_wq; +#define UMH_WQ_HASH_SHIFT 6 +#define UMH_WQ_HASH_SIZE 1 << UMH_WQ_HASH_SHIFT + +struct umh_wq_entry { +int token; +unsigned int count; +struct workqueue_struct *wq; +struct hlist_node umh_wq_hlist; +}; + +static DEFINE_SPINLOCK (umh_wq_hash_lock); +static struct hlist_head umh_wq_hash[UMH_WQ_HASH_SIZE ]; +#define CAP_BSET (void *)1#define CAP_PI (void *)2@@ -475,6 +492,165 @@ static void helper_unlock(void)wake_up (&running_helpers_waitq); } +static void umh_wq_hash_init (void ) +{ +int i; + +for (i =0 ; i <UMH_WQ_HASH_SIZE ; i++) +INIT_HLIST_HEAD (&umh_wq_hash[i]); +} + +static struct umh_wq_entry *umh_wq_find_entry (int token) +{ +struct umh_wq_entry *this, *entry; +struct hlist_head *bucket; +unsigned int hash; + + hash =hash_32 ((unsigned long ) token,UMH_WQ_HASH_SHIFT ); + bucket = &umh_wq_hash[hash]; + + entry =ERR_PTR (-ENOENT ); +if (hlist_empty (bucket)) + goto out; + +hlist_for_each_entry (this, bucket, umh_wq_hlist) {
kernel/kmod.c static void helper_unlock(void) · line 526
Quoted 10 lines
kernel/kmod.c +10
-0+if (this->token == token) { + entry = this; +break ; + } + } +out: +return entry; +} + +static struct workqueue_struct *umh_find_wq (int token,unsigned int nowait)
nit: there's no caller of this in this patch, but one is added in patch #2.
Quoted 70 lines
kernel/kmod.c +70
-0+{ +struct umh_wq_entry *entry; +unsigned long flags; + +if (!token) +return khelper_wq; + +if (nowait) +spin_lock_irqsave (&umh_wq_hash_lock, flags); +else +spin_lock (&umh_wq_hash_lock); + entry =umh_wq_find_entry (token); +if (nowait) +spin_unlock_irqrestore (&umh_wq_hash_lock, flags); +else +spin_unlock (&umh_wq_hash_lock); + +return entry->wq; +} + +/** +* umh_wq_get_token - create service thread and return an identifying token +* @token: token of an existing service thread or 0 to create a new +* service thread. +* @name: service name to be appended to "khelper" for identification. +* +* Returns a token that used with calls to call_usermode_helper_service(). +* If token corresponds to an existing service thread its reference count +* is increased and the token returned. On failure returns a negative errno. +*/ +int umh_wq_get_token (int token,const char *service) +{ +struct workqueue_struct *wq; +char *wq_name; +int wq_name_len; +struct umh_wq_entry *entry; +struct hlist_head *bucket; +unsigned int hash; +unsigned int new_token; + +if (token) { +spin_lock (&umh_wq_hash_lock); + entry =umh_wq_find_entry (token); +if (entry) { + entry->count++; +spin_unlock (&umh_wq_hash_lock); +return token; + } +spin_unlock (&umh_wq_hash_lock); + } + +if (!service) +return -EINVAL ; + + wq_name_len =sizeof (KHELPER ) +strlen (service) +1 ; + wq_name =kmalloc (wq_name_len,GFP_KERNEL ); +if (!wq_name) +return -ENOMEM ; +strcpy (wq_name,"KHELPER-" ); +strcat (wq_name, service); + + entry =kzalloc (sizeof (struct umh_wq_entry ),GFP_KERNEL ); +if (!entry) { +kfree (wq_name); +return -ENOMEM ; + } + + wq =create_singlethread_workqueue (wq_name); +if (IS_ERR (wq)) { +kfree (wq_name);
kernel/kmod.c static void helper_unlock(void) · line 606
Quoted 10 lines
kernel/kmod.c +10
-0+kfree (entry); +return PTR_ERR (wq); + } +kfree (wq_name); + entry->wq = wq; + + do { + new_token = get_random_int(); + if (!new_token) + continue;
Why a random value here? Is there some attack that can be done by guessing the token?
ISTM that that could end up with you reusing a token soon after it has been removed from the hash if you were really unlucky. If so, then that could potentially be dangerous.
If there isn't a potential attack vector that involves guessing this value, then a simple counter might mean less chance that the token would be reused (particularly if the token were 64 bits).
kernel/kmod.c static void helper_unlock(void) · line 613
Quoted 7 lines
kernel/kmod.c +7
-0+spin_lock (&umh_wq_hash_lock); + entry =umh_wq_find_entry (new_token); +if (likely (IS_ERR (entry))) { + hash =hash_32 (new_token,UMH_WQ_HASH_SHIFT ); + bucket = &umh_wq_hash[hash]; +hlist_add_head (&entry->umh_wq_hlist, bucket); + entry->token = (long ) new_token;
Why cast to long here? Shouldn't that be (int)?
Quoted 53 lines
kernel/kmod.c +42
-1+spin_unlock (&umh_wq_hash_lock); +break ; + } +spin_unlock (&umh_wq_hash_lock); + }while (1 ); + +return (int ) new_token; +} +EXPORT_SYMBOL (umh_wq_get_token); + +/** +* umh_ns_put_token - stop a service thread if it's not longer in use +* and remove it from the serive thread list +* @token: token of a service thread. +*/ +void umh_wq_put_token (int token) +{ +struct umh_wq_entry *entry; + +if (!token) +return ; + +spin_lock (&umh_wq_hash_lock); + entry =umh_wq_find_entry (token); +if (unlikely (IS_ERR (entry))) +spin_unlock (&umh_wq_hash_lock); +else { +if (--entry->count) +spin_unlock (&umh_wq_hash_lock); +else { +hlist_del (&entry->umh_wq_hlist); +spin_unlock (&umh_wq_hash_lock); +destroy_workqueue (entry->wq); +kfree (entry); + } + } +return ; +} +EXPORT_SYMBOL (umh_wq_put_token); +/** * call_usermodehelper_setup - prepare to call a usermode helper * @path: path to usermode executable @@ -689,6 +865,7 @@ struct ctl_table usermodehelper_table[] = {void __init usermodehelper_init (void ) { - khelper_wq =create_singlethread_workqueue ("khelper" ); +umh_wq_hash_init (); + khelper_wq =create_singlethread_workqueue (KHELPER );BUG_ON (!khelper_wq); }
Signature · 1 line
Jeff Layton <[email protected]>
Re: [RFC PATCH 5 1/7] kmod - add workqueue service thread store
Quoted 105 lines · Jeff Layton
Quoted 102 lines · On Tue, 31 Mar 2015 11:14:42 +0800 Ian Kent <[email protected]> wrote:
From: Ian Kent <[email protected]>
Persistent use of process information is needed for contained execution in a namespace other than the root init namespace.
Use a simple random token as a key to create and store thread information in a hashed list for use by the usermode helper thread runner.
Signed-off-by Ian Kent <[email protected]>Cc Benjamin Coddington <[email protected]>Cc Al Viro <[email protected]>Cc J. Bruce Fields <[email protected]>Cc David Howells <[email protected]>Cc Trond Myklebust <[email protected]>Cc Oleg Nesterov <[email protected]>Cc Eric W. Biederman <[email protected]>Cc Jeff Layton <[email protected]>--- include/linux/kmod.h | 3 + kernel/kmod.c | 179 ++++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 181 insertions(+), 1 deletion(-)include/linux/kmod.h +3
-0@@ -66,6 +66,9 @@ struct subprocess_info {void *data; }; +extern int umh_wq_get_token (int token,const char *service); +extern void umh_wq_put_token (int token); +extern int call_usermodehelper (char *path,char **argv,char **envp,int wait);kernel/kmod.c +43
-0@@ -40,13 +40,30 @@#include <linux/ptrace.h> #include <linux/async.h> #include <asm/uaccess.h> +#include <linux/hash.h> +#include <linux/list.h> +#include <linux/random.h> #include <trace/events/module.h> extern int max_threads; +#define KHELPER "khelper"static struct workqueue_struct *khelper_wq; +#define UMH_WQ_HASH_SHIFT 6 +#define UMH_WQ_HASH_SIZE 1 << UMH_WQ_HASH_SHIFT + +struct umh_wq_entry { +int token; +unsigned int count; +struct workqueue_struct *wq; +struct hlist_node umh_wq_hlist; +}; + +static DEFINE_SPINLOCK (umh_wq_hash_lock); +static struct hlist_head umh_wq_hash[UMH_WQ_HASH_SIZE ]; +#define CAP_BSET (void *)1#define CAP_PI (void *)2@@ -475,6 +492,165 @@ static void helper_unlock(void)wake_up (&running_helpers_waitq); } +static void umh_wq_hash_init (void ) +{ +int i; + +for (i =0 ; i <UMH_WQ_HASH_SIZE ; i++) +INIT_HLIST_HEAD (&umh_wq_hash[i]); +} + +static struct umh_wq_entry *umh_wq_find_entry (int token) +{ +struct umh_wq_entry *this, *entry; +struct hlist_head *bucket; +unsigned int hash; + + hash =hash_32 ((unsigned long ) token,UMH_WQ_HASH_SHIFT ); + bucket = &umh_wq_hash[hash]; + + entry =ERR_PTR (-ENOENT ); +if (hlist_empty (bucket)) + goto out; + +hlist_for_each_entry (this, bucket, umh_wq_hlist) { +if (this->token == token) { + entry = this; +break ; + }
Quoted 8 lines
kernel/kmod.c static void helper_unlock(void) · line 526
Quoted 6 lines
kernel/kmod.c +6
-0+ } +out: +return entry; +} + +static struct workqueue_struct *umh_find_wq (int token,unsigned int nowait)nit: there's no caller of this in this patch, but one is added in patch #2.
I could change that.
The patch is meant to setup the infrastructure for the subsequent patch. I can re-organize them if this ends up being worth continuing with.
Quoted 76 lines
Quoted 76 lines
kernel/kmod.c +75
-0+{ +struct umh_wq_entry *entry; +unsigned long flags; + +if (!token) +return khelper_wq; + +if (nowait) +spin_lock_irqsave (&umh_wq_hash_lock, flags); +else +spin_lock (&umh_wq_hash_lock); + entry =umh_wq_find_entry (token); +if (nowait) +spin_unlock_irqrestore (&umh_wq_hash_lock, flags); +else +spin_unlock (&umh_wq_hash_lock); + +return entry->wq; +} + +/** +* umh_wq_get_token - create service thread and return an identifying token +* @token: token of an existing service thread or 0 to create a new +* service thread. +* @name: service name to be appended to "khelper" for identification. +* +* Returns a token that used with calls to call_usermode_helper_service(). +* If token corresponds to an existing service thread its reference count +* is increased and the token returned. On failure returns a negative errno. +*/ +int umh_wq_get_token (int token,const char *service) +{ +struct workqueue_struct *wq; +char *wq_name; +int wq_name_len; +struct umh_wq_entry *entry; +struct hlist_head *bucket; +unsigned int hash; +unsigned int new_token; + +if (token) { +spin_lock (&umh_wq_hash_lock); + entry =umh_wq_find_entry (token); +if (entry) { + entry->count++; +spin_unlock (&umh_wq_hash_lock); +return token; + } +spin_unlock (&umh_wq_hash_lock); + } + +if (!service) +return -EINVAL ; + + wq_name_len =sizeof (KHELPER ) +strlen (service) +1 ; + wq_name =kmalloc (wq_name_len,GFP_KERNEL ); +if (!wq_name) +return -ENOMEM ; +strcpy (wq_name,"KHELPER-" ); +strcat (wq_name, service); + + entry =kzalloc (sizeof (struct umh_wq_entry ),GFP_KERNEL ); +if (!entry) { +kfree (wq_name); +return -ENOMEM ; + } + + wq =create_singlethread_workqueue (wq_name); +if (IS_ERR (wq)) { +kfree (wq_name); +kfree (entry); +return PTR_ERR (wq); + } +kfree (wq_name); + entry->wq = wq;
Quoted 8 lines
kernel/kmod.c static void helper_unlock(void) · line 606
Quoted 5 lines
kernel/kmod.c +5
-0+ + do { + new_token = get_random_int(); + if (!new_token) + continue;Why a random value here? Is there some attack that can be done by guessing the token?
Good question for which I have no good answer.
I was originally concerned about reuse after overflow from continued issue and thought using a random token would be better but ....
Quoted 4 lines
ISTM that that could end up with you reusing a token soon after it has been removed from the hash if you were really unlucky. If so, then that could potentially be dangerous.
I cover that by not using a token for which an entry already exists. But a counter is probably equally as good.
Quoted 5 lines
If there isn't a potential attack vector that involves guessing this value, then a simple counter might mean less chance that the token would be reused (particularly if the token were 64 bits).
Quoted 9 lines
kernel/kmod.c static void helper_unlock(void) · line 613
Quoted 7 lines
kernel/kmod.c +7
-0+spin_lock (&umh_wq_hash_lock); + entry =umh_wq_find_entry (new_token); +if (likely (IS_ERR (entry))) { + hash =hash_32 (new_token,UMH_WQ_HASH_SHIFT ); + bucket = &umh_wq_hash[hash]; +hlist_add_head (&entry->umh_wq_hlist, bucket); + entry->token = (long ) new_token;Why cast to long here? Shouldn't that be (int)?
Yes, I missed that when re-doing the patch, ;)
Quoted 56 lines
Quoted 54 lines
kernel/kmod.c +42
-1+spin_unlock (&umh_wq_hash_lock); +break ; + } +spin_unlock (&umh_wq_hash_lock); + }while (1 ); + +return (int ) new_token; +} +EXPORT_SYMBOL (umh_wq_get_token); + +/** +* umh_ns_put_token - stop a service thread if it's not longer in use +* and remove it from the serive thread list +* @token: token of a service thread. +*/ +void umh_wq_put_token (int token) +{ +struct umh_wq_entry *entry; + +if (!token) +return ; + +spin_lock (&umh_wq_hash_lock); + entry =umh_wq_find_entry (token); +if (unlikely (IS_ERR (entry))) +spin_unlock (&umh_wq_hash_lock); +else { +if (--entry->count) +spin_unlock (&umh_wq_hash_lock); +else { +hlist_del (&entry->umh_wq_hlist); +spin_unlock (&umh_wq_hash_lock); +destroy_workqueue (entry->wq); +kfree (entry); + } + } +return ; +} +EXPORT_SYMBOL (umh_wq_put_token); +/** * call_usermodehelper_setup - prepare to call a usermode helper * @path: path to usermode executable @@ -689,6 +865,7 @@ struct ctl_table usermodehelper_table[] = {void __init usermodehelper_init (void ) { - khelper_wq =create_singlethread_workqueue ("khelper" ); +umh_wq_hash_init (); + khelper_wq =create_singlethread_workqueue (KHELPER );BUG_ON (!khelper_wq); }
David Howells
maintainerRe: [RFC PATCH 5 1/7] kmod - add workqueue service thread store
In reply to Ian KentQuoted 12 lines · Ian Kent
kernel/kmod.c +12
-0+static struct umh_wq_entry *umh_wq_find_entry (int token) +{ +struct umh_wq_entry *this, *entry; +struct hlist_head *bucket; +unsigned int hash; + + hash =hash_32 ((unsigned long ) token,UMH_WQ_HASH_SHIFT ); + bucket = &umh_wq_hash[hash]; + + entry =ERR_PTR (-ENOENT ); +if (hlist_empty (bucket)) + goto out;
kernel/kmod.c static void helper_unlock(void) · line 524
Quoted 10 lines
kernel/kmod.c +10
-0+ +hlist_for_each_entry (this, bucket, umh_wq_hlist) { +if (this->token == token) { + entry = this; +break ; + } + } +out: +return entry; +}
Can "struct umh_wq_entry *" be used as the token?
David
Re: [RFC PATCH 5 1/7] kmod - add workqueue service thread store
Quoted 18 lines · David Howells
Quoted 16 lines · Ian Kent
kernel/kmod.c +16
-0+static struct umh_wq_entry *umh_wq_find_entry (int token) +{ +struct umh_wq_entry *this, *entry; +struct hlist_head *bucket; +unsigned int hash; + + hash =hash_32 ((unsigned long ) token,UMH_WQ_HASH_SHIFT ); + bucket = &umh_wq_hash[hash]; + + entry =ERR_PTR (-ENOENT ); +if (hlist_empty (bucket)) + goto out; + +hlist_for_each_entry (this, bucket, umh_wq_hlist) { +if (this->token == token) { + entry = this;
Quoted 8 lines
kernel/kmod.c static void helper_unlock(void) · line 524
Quoted 6 lines
kernel/kmod.c +6
-0+break ; + } + } +out: +return entry; +}Can "struct
umh_wq_entry*" be used as the token?
Probably not, for example.
Couldn't a user set a different workqueue_struct and have it used for execution. Not sure what that would get the user but it sounds like the original reason we couldn't allow execution directly within the caller environment.
Quoted 2 lines
David
RFC PATCH v5 2/7 kmod - teach usermodehelper to use service workqueues
In reply to Ian KentThe call_usermodehelper() function executes all binaries in the global "init" root context. This doesn't allow a binary to be run within a namespace environment such as a container. To do this the namespace environment of the contaner must be available to provide the required execution environment.
This can be done by creating a service thread, identified by issuing a token identifier, used when executing the helper with a function that takes the token as a parameter.
--- include/linux/kmod.h | 5 +++++ kernel/kmod.c | 38 +++++++++++++++++++++++++++++++------- 2 files changed, 36 insertions(+), 7 deletions(-)
include/linux/kmod.h +5 -0
@@ -56,6 +56,7 @@ struct file;struct subprocess_info {struct work_struct work;struct completion *complete; +int wq_token;char *path;char **argv;char **envp;
@@ -72,6 +73,10 @@ extern void umh_wq_put_token(int token);extern int call_usermodehelper (char *path,char **argv,char **envp,int wait); +extern int +call_usermodehelper_service (char *path,char **argv, +char **envp,int token,int wait); +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),
kernel/kmod.c +31 -7
@@ -712,6 +712,7 @@ EXPORT_SYMBOL(call_usermodehelper_setup);*/ int call_usermodehelper_exec (struct subprocess_info *sub_info,int wait) { +static struct workqueue_struct *wq;DECLARE_COMPLETION_ONSTACK (done);int retval =0 ;
@@ -720,7 +721,8 @@ int call_usermodehelper_exec(struct subprocess_info *sub_info, int wait)return -EINVAL ; }helper_lock (); -if (!khelper_wq || usermodehelper_disabled) { + wq =umh_find_wq (sub_info->wq_token, wait); +if (!wq || usermodehelper_disabled) { retval = -EBUSY ; goto out; }
@@ -732,7 +734,7 @@ int call_usermodehelper_exec(struct subprocess_info *sub_info, int wait) sub_info->complete = (wait ==UMH_NO_WAIT ) ?NULL : &done; sub_info->wait = wait; -queue_work (khelper_wq, &sub_info->work); +queue_work (wq, &sub_info->work);if (wait ==UMH_NO_WAIT )/* task has freed sub_info */ goto unlock;
@@ -759,19 +761,21 @@ unlock:EXPORT_SYMBOL (call_usermodehelper_exec);/** -* call_usermodehelper() - prepare and start a usermode application +* call_usermodehelper_service() - start a usermode application within +* a service environment. * @path: path to usermode executable * @argv: arg vector for process * @envp: environment for process +* @token: token corresponding to a service environment obtained by a +* call to umh_wq_get_token(). * @wait: wait for the application to finish and return status. * 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. -* -* This function is the equivalent to use call_usermodehelper_setup() and -* call_usermodehelper_exec(). */ -int call_usermodehelper (char *path,char **argv,char **envp,int wait) +int call_usermodehelper_service (char *path, +char **argv,char **envp, +int token,int wait) {struct subprocess_info *info;gfp_t gfp_mask = (wait ==UMH_NO_WAIT ) ?GFP_ATOMIC :GFP_KERNEL ;
@@ -780,9 +784,29 @@ int call_usermodehelper(char *path, char **argv, char **envp, int wait) NULL, NULL, NULL);if (info ==NULL )return -ENOMEM ; + info->wq_token = token;return call_usermodehelper_exec (info, wait); } +EXPORT_SYMBOL (call_usermodehelper_service); + +/** +* call_usermodehelper() - prepare and start a usermode application +* @path: path to usermode executable +* @argv: arg vector for process +* @envp: environment for process +* @wait: wait for the application to finish and return status. +* 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. +* +* This function is the equivalent to using call_usermodehelper_setup() and +* call_usermodehelper_exec(). +*/ +int call_usermodehelper (char *path,char **argv,char **envp,int wait) +{ +return call_usermodehelper_service (path, argv, envp,0 , wait); +}EXPORT_SYMBOL (call_usermodehelper); static int proc_cap_handler(struct ctl_table *table, int write,
RFC PATCH v5 3/7 nfsd - use service thread if not executing in init namespace
In reply to Ian KentFrom: Ian Kent <[email protected]>
If nfsd is running within a container the client tracking operations should run within their originating container also. To do that get a token to a service thread created within the container environment for usermode helper calls.
--- fs/nfsd/netns.h | 3 +++ fs/nfsd/nfs4recover.c | 48 +++++++++++++++++++++++++++++++----------------- fs/nfsd/nfsctl.c | 6 ++++++ 3 files changed, 40 insertions(+), 17 deletions(-)
fs/nfsd/netns.h +3 -0
@@ -112,6 +112,9 @@ struct nfsd_net {u32 clientid_counter;struct svc_serv *nfsd_serv; + +/* Namespace token */ +int umh_token; };/* Simple check to find out if a given net was properly initialized */
fs/nfsd/nfs4recover.c +31 -17
110 lines not on this page · raw message
fs/nfsd/nfsctl.c +6 -0
@@ -1224,6 +1224,8 @@ static __net_init int nfsd_init_net(struct net *net) goto out_idmap_error; nn->nfsd4_lease =90 ;/* default lease time */ nn->nfsd4_grace =90 ; +if (net != &init_net) + nn->umh_token =umh_wq_get_token (nn->umh_token,"knfsd" );return 0 ; out_idmap_error:
@@ -1234,6 +1236,10 @@ out_export_error: static __net_exit void nfsd_exit_net(struct net *net) { +struct nfsd_net *nn =net_generic (net, nfsd_net_id); + +umh_wq_put_token (nn->umh_token); + nn->umh_token =0 ;nfsd_idmap_shutdown (net);nfsd_export_shutdown (net); }
Re: [RFC PATCH 5 3/7] nfsd - use service thread if not executing in init namespace
Quoted 51 lines · Ian Kent
From: Ian Kent <[email protected]>
If nfsd is running within a container the client tracking operations should run within their originating container also. To do that get a token to a service thread created within the container environment for usermode helper calls.
Signed-off-by Ian Kent <[email protected]>Cc Benjamin Coddington <[email protected]>Cc Al Viro <[email protected]>Cc J. Bruce Fields <[email protected]>Cc David Howells <[email protected]>Cc Trond Myklebust <[email protected]>Cc Oleg Nesterov <[email protected]>Cc Eric W. Biederman <[email protected]>Cc Jeff Layton <[email protected]>--- fs/nfsd/netns.h | 3 +++ fs/nfsd/nfs4recover.c | 48 +++++++++++++++++++++++++++++++----------------- fs/nfsd/nfsctl.c | 6 ++++++ 3 files changed, 40 insertions(+), 17 deletions(-)fs/nfsd/netns.h +3
-0@@ -112,6 +112,9 @@ struct nfsd_net {u32 clientid_counter;struct svc_serv *nfsd_serv; + +/* Namespace token */ +int umh_token; };/* Simple check to find out if a given net was properly initialized */ fs/nfsd/nfs4recover.c +2
-1@@ -1184,7 +1184,8 @@ nfsd4_cltrack_grace_start(time_t grace_start) }static int -nfsd4_umh_cltrack_upcall (char *cmd,char *arg,char *env0,char *env1) +nfsd4_umh_cltrack_upcall (char *cmd,char *arg, +char *env0,char *env1,int token) {char *envp[3 ];char *argv[4 ];
fs/nfsd/nfs4recover.c nfsd4_umh_cltrack_upcall(char *cmd, char *arg, char *env0, char *env1) · line 1217
Quoted 9 lines
fs/nfsd/nfs4recover.c +5
-1@@ -1209,7 +1210,11 @@ nfsd4_umh_cltrack_upcall(char *cmd, char *arg, char *env0, char *env1) argv[2 ] = arg; argv[3 ] =NULL ; - ret =call_usermodehelper (argv[0 ], argv, envp,UMH_WAIT_PROC ); +if (token >0 ) + ret =call_usermodehelper_service (argv[0 ], argv, envp, + token,UMH_WAIT_PROC ); +else + ret =call_usermodehelper (argv[0 ], argv, envp,UMH_WAIT_PROC );
Do we really need to handle the init_net case specially, or could we just let it create a workqueue in that case as well?
--b.
Quoted 124 lines
fs/nfsd/nfs4recover.c +24
-15/* * Disable the upcall mechanism if we're getting an ENOENT or EACCES * error. The admin can re-enable it on the fly by using sysfs @@ -1252,14 +1257,8 @@ nfsd4_umh_cltrack_init(struct net *net)struct nfsd_net *nn =net_generic (net, nfsd_net_id);char *grace_start =nfsd4_cltrack_grace_start (nn->boot_time); -/* XXX: The usermode helper s not working in container yet. */ -if (net != &init_net) { -WARN (1 ,KERN_ERR "NFSD: attempt to initialize umh client " -"tracking in a container!\n" ); -return -EINVAL ; - } - - ret =nfsd4_umh_cltrack_upcall ("init" ,NULL , grace_start,NULL ); + ret =nfsd4_umh_cltrack_upcall ("init" ,NULL , + grace_start,NULL , nn->umh_token);kfree (grace_start);return ret; }@@ -1285,6 +1284,7 @@ nfsd4_umh_cltrack_create(struct nfs4_client *clp) {char *hexid, *has_session, *grace_start;struct nfsd_net *nn =net_generic (clp->net, nfsd_net_id); +int ret;/* * With v4.0 clients, there's little difference in outcome between a @@ -1312,7 +1312,10 @@ nfsd4_umh_cltrack_create(struct nfs4_client *clp) grace_start =nfsd4_cltrack_grace_start (nn->boot_time);nfsd4_cltrack_upcall_lock (clp); -if (!nfsd4_umh_cltrack_upcall ("create" , hexid, has_session, grace_start)) + ret =nfsd4_umh_cltrack_upcall ("create" , + hexid, has_session, grace_start, + nn->umh_token); +if (!ret)set_bit (NFSD4_CLIENT_STABLE , &clp->cl_flags);nfsd4_cltrack_upcall_unlock (clp);@@ -1324,7 +1327,9 @@ nfsd4_umh_cltrack_create(struct nfs4_client *clp)static void nfsd4_umh_cltrack_remove (struct nfs4_client *clp) { +struct nfsd_net *nn =net_generic (clp->net, nfsd_net_id);char *hexid; +int ret;if (!test_bit (NFSD4_CLIENT_STABLE , &clp->cl_flags))return ;@@ -1336,9 +1341,13 @@ nfsd4_umh_cltrack_remove(struct nfs4_client *clp) }nfsd4_cltrack_upcall_lock (clp); -if (test_bit (NFSD4_CLIENT_STABLE , &clp->cl_flags) && -nfsd4_umh_cltrack_upcall ("remove" , hexid,NULL ,NULL ) ==0 ) -clear_bit (NFSD4_CLIENT_STABLE , &clp->cl_flags); +if (test_bit (NFSD4_CLIENT_STABLE , &clp->cl_flags)) { + ret =nfsd4_umh_cltrack_upcall ("remove" , + hexid,NULL ,NULL , + nn->umh_token); +if (ret ==0 ) +clear_bit (NFSD4_CLIENT_STABLE , &clp->cl_flags); + }nfsd4_cltrack_upcall_unlock (clp);kfree (hexid);@@ -1347,8 +1356,9 @@ nfsd4_umh_cltrack_remove(struct nfs4_client *clp)static int nfsd4_umh_cltrack_check (struct nfs4_client *clp) { -int ret; +struct nfsd_net *nn =net_generic (clp->net, nfsd_net_id);char *hexid, *has_session, *legacy; +int ret;if (test_bit (NFSD4_CLIENT_STABLE , &clp->cl_flags))return 0 ;@@ -1366,7 +1376,9 @@ nfsd4_umh_cltrack_check(struct nfs4_client *clp)if (test_bit (NFSD4_CLIENT_STABLE , &clp->cl_flags)) { ret =0 ; }else { - ret =nfsd4_umh_cltrack_upcall ("check" , hexid, has_session, legacy); + ret =nfsd4_umh_cltrack_upcall ("check" , hexid, + has_session, legacy, + nn->umh_token);if (ret ==0 )set_bit (NFSD4_CLIENT_STABLE , &clp->cl_flags); }@@ -1386,7 +1398,9 @@ nfsd4_umh_cltrack_grace_done(struct nfsd_net *nn)sprintf (timestr,"%ld" , nn->boot_time); legacy =nfsd4_cltrack_legacy_topdir (); -nfsd4_umh_cltrack_upcall ("gracedone" , timestr, legacy,NULL ); +nfsd4_umh_cltrack_upcall ("gracedone" , + timestr, legacy,NULL , + nn->umh_token);kfree (legacy); }fs/nfsd/nfsctl.c +6
-0@@ -1224,6 +1224,8 @@ static __net_init int nfsd_init_net(struct net *net) goto out_idmap_error; nn->nfsd4_lease =90 ;/* default lease time */ nn->nfsd4_grace =90 ; +if (net != &init_net) + nn->umh_token =umh_wq_get_token (nn->umh_token,"knfsd" );return 0 ; out_idmap_error:@@ -1234,6 +1236,10 @@ out_export_error: static __net_exit void nfsd_exit_net(struct net *net) { +struct nfsd_net *nn =net_generic (net, nfsd_net_id); + +umh_wq_put_token (nn->umh_token); + nn->umh_token =0 ;nfsd_idmap_shutdown (net);nfsd_export_shutdown (net); }
Re: [RFC PATCH 5 3/7] nfsd - use service thread if not executing in init namespace
Quoted 55 lines · J. Bruce Fields
Quoted 54 lines · Ian Kent
From: Ian Kent <[email protected]>
If nfsd is running within a container the client tracking operations should run within their originating container also. To do that get a token to a service thread created within the container environment for usermode helper calls.
Signed-off-by Ian Kent <[email protected]>Cc Benjamin Coddington <[email protected]>Cc Al Viro <[email protected]>Cc J. Bruce Fields <[email protected]>Cc David Howells <[email protected]>Cc Trond Myklebust <[email protected]>Cc Oleg Nesterov <[email protected]>Cc Eric W. Biederman <[email protected]>Cc Jeff Layton <[email protected]>--- fs/nfsd/netns.h | 3 +++ fs/nfsd/nfs4recover.c | 48 +++++++++++++++++++++++++++++++----------------- fs/nfsd/nfsctl.c | 6 ++++++ 3 files changed, 40 insertions(+), 17 deletions(-)fs/nfsd/netns.h +3
-0@@ -112,6 +112,9 @@ struct nfsd_net {u32 clientid_counter;struct svc_serv *nfsd_serv; + +/* Namespace token */ +int umh_token; };/* Simple check to find out if a given net was properly initialized */ fs/nfsd/nfs4recover.c +2
-1@@ -1184,7 +1184,8 @@ nfsd4_cltrack_grace_start(time_t grace_start) }static int -nfsd4_umh_cltrack_upcall (char *cmd,char *arg,char *env0,char *env1) +nfsd4_umh_cltrack_upcall (char *cmd,char *arg, +char *env0,char *env1,int token) {char *envp[3 ];char *argv[4 ];@@ -1209,7 +1210,11 @@ nfsd4_umh_cltrack_upcall(char *cmd, char *arg, char *env0, char *env1) argv[2 ] = arg; argv[3 ] =NULL ;
Quoted 9 lines
fs/nfsd/nfs4recover.c nfsd4_umh_cltrack_upcall(char *cmd, char *arg, char *env0, char *env1) · line 1217
Quoted 6 lines
fs/nfsd/nfs4recover.c +5
-1- ret =call_usermodehelper (argv[0 ], argv, envp,UMH_WAIT_PROC ); +if (token >0 ) + ret =call_usermodehelper_service (argv[0 ], argv, envp, + token,UMH_WAIT_PROC ); +else + ret =call_usermodehelper (argv[0 ], argv, envp,UMH_WAIT_PROC );Do we really need to handle the
init_netcase specially, or could we just let it create a workqueue in that case as well?
That's pretty much up to you but there's still the need to get a token to create the work queue (and put it to terminate it) or just pass 0 to call_usermodehelper_service().
Quoted 127 lines
--b.
Quoted 124 lines
fs/nfsd/nfs4recover.c +24
-15/* * Disable the upcall mechanism if we're getting an ENOENT or EACCES * error. The admin can re-enable it on the fly by using sysfs @@ -1252,14 +1257,8 @@ nfsd4_umh_cltrack_init(struct net *net)struct nfsd_net *nn =net_generic (net, nfsd_net_id);char *grace_start =nfsd4_cltrack_grace_start (nn->boot_time); -/* XXX: The usermode helper s not working in container yet. */ -if (net != &init_net) { -WARN (1 ,KERN_ERR "NFSD: attempt to initialize umh client " -"tracking in a container!\n" ); -return -EINVAL ; - } - - ret =nfsd4_umh_cltrack_upcall ("init" ,NULL , grace_start,NULL ); + ret =nfsd4_umh_cltrack_upcall ("init" ,NULL , + grace_start,NULL , nn->umh_token);kfree (grace_start);return ret; }@@ -1285,6 +1284,7 @@ nfsd4_umh_cltrack_create(struct nfs4_client *clp) {char *hexid, *has_session, *grace_start;struct nfsd_net *nn =net_generic (clp->net, nfsd_net_id); +int ret;/* * With v4.0 clients, there's little difference in outcome between a @@ -1312,7 +1312,10 @@ nfsd4_umh_cltrack_create(struct nfs4_client *clp) grace_start =nfsd4_cltrack_grace_start (nn->boot_time);nfsd4_cltrack_upcall_lock (clp); -if (!nfsd4_umh_cltrack_upcall ("create" , hexid, has_session, grace_start)) + ret =nfsd4_umh_cltrack_upcall ("create" , + hexid, has_session, grace_start, + nn->umh_token); +if (!ret)set_bit (NFSD4_CLIENT_STABLE , &clp->cl_flags);nfsd4_cltrack_upcall_unlock (clp);@@ -1324,7 +1327,9 @@ nfsd4_umh_cltrack_create(struct nfs4_client *clp)static void nfsd4_umh_cltrack_remove (struct nfs4_client *clp) { +struct nfsd_net *nn =net_generic (clp->net, nfsd_net_id);char *hexid; +int ret;if (!test_bit (NFSD4_CLIENT_STABLE , &clp->cl_flags))return ;@@ -1336,9 +1341,13 @@ nfsd4_umh_cltrack_remove(struct nfs4_client *clp) }nfsd4_cltrack_upcall_lock (clp); -if (test_bit (NFSD4_CLIENT_STABLE , &clp->cl_flags) && -nfsd4_umh_cltrack_upcall ("remove" , hexid,NULL ,NULL ) ==0 ) -clear_bit (NFSD4_CLIENT_STABLE , &clp->cl_flags); +if (test_bit (NFSD4_CLIENT_STABLE , &clp->cl_flags)) { + ret =nfsd4_umh_cltrack_upcall ("remove" , + hexid,NULL ,NULL , + nn->umh_token); +if (ret ==0 ) +clear_bit (NFSD4_CLIENT_STABLE , &clp->cl_flags); + }nfsd4_cltrack_upcall_unlock (clp);kfree (hexid);@@ -1347,8 +1356,9 @@ nfsd4_umh_cltrack_remove(struct nfs4_client *clp)static int nfsd4_umh_cltrack_check (struct nfs4_client *clp) { -int ret; +struct nfsd_net *nn =net_generic (clp->net, nfsd_net_id);char *hexid, *has_session, *legacy; +int ret;if (test_bit (NFSD4_CLIENT_STABLE , &clp->cl_flags))return 0 ;@@ -1366,7 +1376,9 @@ nfsd4_umh_cltrack_check(struct nfs4_client *clp)if (test_bit (NFSD4_CLIENT_STABLE , &clp->cl_flags)) { ret =0 ; }else { - ret =nfsd4_umh_cltrack_upcall ("check" , hexid, has_session, legacy); + ret =nfsd4_umh_cltrack_upcall ("check" , hexid, + has_session, legacy, + nn->umh_token);if (ret ==0 )set_bit (NFSD4_CLIENT_STABLE , &clp->cl_flags); }@@ -1386,7 +1398,9 @@ nfsd4_umh_cltrack_grace_done(struct nfsd_net *nn)sprintf (timestr,"%ld" , nn->boot_time); legacy =nfsd4_cltrack_legacy_topdir (); -nfsd4_umh_cltrack_upcall ("gracedone" , timestr, legacy,NULL ); +nfsd4_umh_cltrack_upcall ("gracedone" , + timestr, legacy,NULL , + nn->umh_token);kfree (legacy); }fs/nfsd/nfsctl.c +6
-0@@ -1224,6 +1224,8 @@ static __net_init int nfsd_init_net(struct net *net) goto out_idmap_error; nn->nfsd4_lease =90 ;/* default lease time */ nn->nfsd4_grace =90 ; +if (net != &init_net) + nn->umh_token =umh_wq_get_token (nn->umh_token,"knfsd" );return 0 ; out_idmap_error:@@ -1234,6 +1236,10 @@ out_export_error: static __net_exit void nfsd_exit_net(struct net *net) { +struct nfsd_net *nn =net_generic (net, nfsd_net_id); + +umh_wq_put_token (nn->umh_token); + nn->umh_token =0 ;nfsd_idmap_shutdown (net);nfsd_export_shutdown (net); }
Re: [RFC PATCH 5 3/7] nfsd - use service thread if not executing in init namespace
Quoted 66 lines · Ian Kent
Quoted 55 lines · J. Bruce Fields
Quoted 54 lines · Ian Kent
From: Ian Kent <[email protected]>
If nfsd is running within a container the client tracking operations should run within their originating container also. To do that get a token to a service thread created within the container environment for usermode helper calls.
Signed-off-by Ian Kent <[email protected]>Cc Benjamin Coddington <[email protected]>Cc Al Viro <[email protected]>Cc J. Bruce Fields <[email protected]>Cc David Howells <[email protected]>Cc Trond Myklebust <[email protected]>Cc Oleg Nesterov <[email protected]>Cc Eric W. Biederman <[email protected]>Cc Jeff Layton <[email protected]>--- fs/nfsd/netns.h | 3 +++ fs/nfsd/nfs4recover.c | 48 +++++++++++++++++++++++++++++++----------------- fs/nfsd/nfsctl.c | 6 ++++++ 3 files changed, 40 insertions(+), 17 deletions(-)fs/nfsd/netns.h +3
-0@@ -112,6 +112,9 @@ struct nfsd_net {u32 clientid_counter;struct svc_serv *nfsd_serv; + +/* Namespace token */ +int umh_token; };/* Simple check to find out if a given net was properly initialized */ fs/nfsd/nfs4recover.c +2
-1@@ -1184,7 +1184,8 @@ nfsd4_cltrack_grace_start(time_t grace_start) }static int -nfsd4_umh_cltrack_upcall (char *cmd,char *arg,char *env0,char *env1) +nfsd4_umh_cltrack_upcall (char *cmd,char *arg, +char *env0,char *env1,int token) {char *envp[3 ];char *argv[4 ];@@ -1209,7 +1210,11 @@ nfsd4_umh_cltrack_upcall(char *cmd, char *arg, char *env0, char *env1) argv[2 ] = arg; argv[3 ] =NULL ;Quoted 9 lines
fs/nfsd/nfs4recover.c nfsd4_umh_cltrack_upcall(char *cmd, char *arg, char *env0, char *env1) · line 1217
Quoted 6 lines
fs/nfsd/nfs4recover.c +5
-1- ret =call_usermodehelper (argv[0 ], argv, envp,UMH_WAIT_PROC ); +if (token >0 ) + ret =call_usermodehelper_service (argv[0 ], argv, envp, + token,UMH_WAIT_PROC ); +else + ret =call_usermodehelper (argv[0 ], argv, envp,UMH_WAIT_PROC );Do we really need to handle the
init_netcase specially, or could we just let it create a workqueue in that case as well?
Quoted 3 lines
That's pretty much up to you but there's still the need to get a token to create the work queue (and put it to terminate it) or just pass 0 to
call_usermodehelper_service().
Creating a new workqueue in the init_net case would mean inheriting stuff from the rpc.nfsd environment that we didn't used to. (But what stuff exactly?) So for backwards compatibility reasons perhaps it's safer to do what you've done even if the result seems a little inconsistent.
--b.
RFC PATCH v5 4/7 nfs - cache_lib use service thread if not executing in init namespace
In reply to Ian KentFrom: Ian Kent <[email protected]>
If pipefs is registered within a container pipefs requests should be run within their originating container also. To do that get a token to a service thread created within the container environment for usermode helper calls.
--- fs/nfs/cache_lib.c | 7 ++++++- include/linux/sunrpc/cache.h | 2 ++ net/sunrpc/cache.c | 5 +++++ 3 files changed, 13 insertions(+), 1 deletion(-)
fs/nfs/cache_lib.c +6 -1
@@ -48,7 +48,12 @@ int nfs_cache_upcall(struct cache_detail *cd, char *entry_name)if (nfs_cache_getent_prog[0 ] =='\0' ) goto out; - ret =call_usermodehelper (argv[0 ], argv, envp,UMH_WAIT_EXEC ); +if (cd->u.pipefs.umh_token) { +int token = cd->u.pipefs.umh_token; + ret =call_usermodehelper_service (argv[0 ], argv, envp, + token,UMH_WAIT_EXEC ); + }else + ret =call_usermodehelper (argv[0 ], argv, envp,UMH_WAIT_EXEC );/* * Disable the upcall mechanism if we're getting an ENOENT or * EACCES error. The admin can re-enable it on the fly by using
include/linux/sunrpc/cache.h +2 -0
@@ -68,6 +68,8 @@ struct cache_detail_procfs {struct cache_detail_pipefs {struct dentry *dir; +/* Namespace token */ +int umh_token; };struct cache_detail {
net/sunrpc/cache.c +5 -0
@@ -1811,6 +1811,9 @@ int sunrpc_cache_register_pipefs(struct dentry *parent,if (IS_ERR (dir))return PTR_ERR (dir); cd->u.pipefs.dir = dir; +if (cd->net != &init_net) + cd->u.pipefs.umh_token = +umh_wq_get_token (cd->u.pipefs.umh_token,"pipefs" );return 0 ; }EXPORT_SYMBOL_GPL (sunrpc_cache_register_pipefs);
@@ -1819,6 +1822,8 @@ void sunrpc_cache_unregister_pipefs(struct cache_detail *cd) {rpc_remove_cache_dir (cd->u.pipefs.dir); cd->u.pipefs.dir =NULL ; +umh_wq_put_token (cd->u.pipefs.umh_token); + cd->u.pipefs.umh_token =0 ; }EXPORT_SYMBOL_GPL (sunrpc_cache_unregister_pipefs);
RFC PATCH v5 5/7 nfs - objlayout use service thread if not executing in init namespace
In reply to Ian KentFrom: Ian Kent <[email protected]>
If the caller is running within a container then execute the usermode helper callback within the container also.
--- fs/nfs/objlayout/objlayout.c | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-)
fs/nfs/objlayout/objlayout.c +13 -1
@@ -600,8 +600,14 @@ static int __objlayout_upcall(struct __auto_login *login) NULL };char *argv[8 ]; +int umh_token;int ret; +/* If running within a container use the container namespace */ + umh_token =0 ; +if (current->nsproxy->net_ns != &init_net) + umh_token =umh_wq_get_token (0 ,"objlayout" ); +if (unlikely (!osd_login_prog[0 ])) {dprintk ("%s: osd_login_prog is disabled\n" , __func__);return -EACCES ;
@@ -620,7 +626,13 @@ static int __objlayout_upcall(struct __auto_login *login) argv[6 ] = login->systemid_hex; argv[7 ] =NULL ; - ret =call_usermodehelper (argv[0 ], argv, envp,UMH_WAIT_PROC ); +if (!umh_token) + ret =call_usermodehelper (argv[0 ], argv, envp,UMH_WAIT_PROC ); +else { + ret =call_usermodehelper_service (argv[0 ], argv, envp, + umh_token,UMH_WAIT_PROC ); +umh_wq_put_token (umh_token); + }/* * Disable the upcall mechanism if we're getting an ENOENT or * EACCES error. The admin can re-enable it on the fly by using
RFC PATCH v5 6/7 KEYS - use correct memory allocation flag in call_usermodehelper_keys()
In reply to Ian KentFrom: Ian Kent <[email protected]>
When call_usermodehelper_keys() is called it assumes it won't be called with the flag UMH_NO_WAIT. Currently that's always the case.
Change this to check the flag and use the correct kernel memory allocation flag to guard against future changes.
--- security/keys/request_key.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)
security/keys/request_key.c +3 -1
@@ -76,8 +76,10 @@ static int call_usermodehelper_keys(char *path, char **argv, char **envp, struct key *session_keyring, int wait) {struct subprocess_info *info; +unsigned int gfp_mask = (wait &UMH_NO_WAIT ) ? +GFP_ATOMIC :GFP_KERNEL ; - info =call_usermodehelper_setup (path, argv, envp,GFP_KERNEL , + info =call_usermodehelper_setup (path, argv, envp, gfp_mask, umh_keys_init, umh_keys_cleanup, session_keyring);if (!info)
Re: [RFC PATCH 5 6/7] KEYS - use correct memory allocation flag in call_usermodehelper_keys()
Quoted 7 lines · Ian Kent
When
call_usermodehelper_keys()is called it assumes it won't be called with the flagUMH_NO_WAIT. Currently that's always the case.Change this to check the flag and use the correct kernel memory allocation flag to guard against future changes.
Signed-off-by Ian Kent <[email protected]>
I think this is okay.
RFC PATCH v5 7/7 KEYS: exec request key within service thread of key creator
In reply to Ian KentFrom: Ian Kent <[email protected]>
Containerized request key helper callbacks need the ability to execute a binary in a container's context. To do that get a token to a service thread created within the container environment for usermode helper calls.
--- include/linux/key.h | 3 +++ security/keys/gc.c | 2 ++ security/keys/key.c | 5 +++++ security/keys/request_key.c | 34 +++++++++++++++++++++++++++------- 4 files changed, 37 insertions(+), 7 deletions(-)
include/linux/key.h +3 -0
@@ -209,6 +209,9 @@ struct key { } payload;struct assoc_array keys; }; + +/* Namespace token */ +int umh_token; }; extern struct key *key_alloc(struct key_type *type,
security/keys/gc.c +2 -0
@@ -156,6 +156,8 @@ static noinline void key_gc_unused_keys(struct list_head *keys)kfree (key->description); +umh_wq_put_token (key->umh_token); +#ifdef KEY_DEBUGGING key->magic =KEY_DEBUG_MAGIC_X ;#endif
security/keys/key.c +5 -0
@@ -18,6 +18,8 @@#include <linux/workqueue.h> #include <linux/random.h> #include <linux/err.h> +#include <net/net_namespace.h> +#include <linux/nsproxy.h> #include "internal.h" struct kmem_cache *key_jar;
@@ -309,6 +311,9 @@ struct key *key_alloc(struct key_type *type, const char *desc,/* publish the key by giving it a serial number */ atomic_inc (&user->nkeys);key_alloc_serial (key); +/* If running within a container use the container namespace */ +if (current->nsproxy->net_ns != &init_net) + key->umh_token =umh_wq_get_token (0 ,"keys" ); error:return key;
security/keys/request_key.c +27 -7
@@ -70,26 +70,40 @@ static void umh_keys_cleanup(struct subprocess_info *info) }/* -* Call a usermode helper with a specific session keyring. +* Call a usermode helper with a specific session keyring and execute +* within a namespace. */ -static int call_usermodehelper_keys (char *path,char **argv,char **envp, -struct key *session_keyring,int wait) +static int call_usermodehelper_keys_service (char *path, +char **argv,char **envp, +struct key *session_keyring, +int token,unsigned int wait) {struct subprocess_info *info;unsigned int gfp_mask = (wait &UMH_NO_WAIT ) ?GFP_ATOMIC :GFP_KERNEL ; info =call_usermodehelper_setup (path, argv, envp, gfp_mask, - umh_keys_init, umh_keys_cleanup, - session_keyring); + umh_keys_init, umh_keys_cleanup, + session_keyring);if (!info)return -ENOMEM ; + info->wq_token = token;key_get (session_keyring);return call_usermodehelper_exec (info, wait); }/* +* Call a usermode helper with a specific session keyring. +*/ +static int call_usermodehelper_keys (char *path,char **argv,char **envp, +struct key *session_keyring,int wait) +{ +return call_usermodehelper_keys_service (path, argv, envp, + session_keyring,0 , wait); +} + +/* * Request userspace finish the construction of a key * - execute "/sbin/request-key <op> <key> <uid> <gid> <keyring> <keyring> <keyring>" */
@@ -174,8 +188,14 @@ static int call_sbin_request_key(struct key_construction *cons, argv[i] =NULL ;/* do it */ - ret =call_usermodehelper_keys (argv[0 ], argv, envp, keyring, -UMH_WAIT_PROC ); +/* If running within a container use the container namespace */ +if (key->umh_token) + ret =call_usermodehelper_keys_service (argv[0 ], argv, envp, + keyring, key->umh_token, +UMH_WAIT_PROC ); +else + ret =call_usermodehelper_keys (argv[0 ], argv, envp, + keyring,UMH_WAIT_PROC );kdebug ("usermode -> 0x%x" , ret);if (ret >=0 ) {/* ret is the exit/wait code */
David Howells
maintainerRe: [RFC PATCH 5 7/7] KEYS: exec request key within service thread of key creator
include/linux/key.h struct key { · line 214
Quoted 3 lines · Ian Kent
include/linux/key.h +3
-0+ +/* Namespace token */ +int umh_token;
If you could put it after data_len so that all the smaller-than-wordsize fields are together for better packing.
security/keys/gc.c static noinline void key_gc_unused_keys(struct list_head *keys) · line 159
Quoted 1 line
security/keys/gc.c +1
-0+umh_wq_put_token (key->umh_token);
Does gc.c need an extra #include for this?
security/keys/key.c struct key *key_alloc(struct key_type *type, const char *desc, · line 316
Quoted 3 lines
security/keys/key.c +3
-0+/* If running within a container use the container namespace */ +if (current->nsproxy->net_ns != &init_net) + key->umh_token =umh_wq_get_token (0 ,"keys" );
So keys live in the networking namespace?
security/keys/request_key.c static int call_sbin_request_key(struct key_construction *cons, · line 198
Quoted 10 lines
security/keys/request_key.c +8
-2- ret =call_usermodehelper_keys (argv[0 ], argv, envp, keyring, -UMH_WAIT_PROC ); +/* If running within a container use the container namespace */ +if (key->umh_token) + ret =call_usermodehelper_keys_service (argv[0 ], argv, envp, + keyring, key->umh_token, +UMH_WAIT_PROC ); +else + ret =call_usermodehelper_keys (argv[0 ], argv, envp, + keyring,UMH_WAIT_PROC );
call_usermodehelper_keys_service() would appear to be superfluous. If key->umh_token is 0, you call call_usermodehelper_keys() which then calls call_usermodehelper_keys_service() with a 0 token...
David
Re: [RFC PATCH 5 7/7] KEYS: exec request key within service thread of key creator
Quoted 8 lines · David Howells
include/linux/key.h struct key { · line 214
Quoted 3 lines · Ian Kent
include/linux/key.h +3
-0+ +/* Namespace token */ +int umh_token;If you could put it after
data_lenso that all the smaller-than-wordsize fields are together for better packing.
OK.
Quoted 4 lines
security/keys/gc.c static noinline void key_gc_unused_keys(struct list_head *keys) · line 159
Quoted 2 lines
security/keys/gc.c +1
-0+umh_wq_put_token (key->umh_token);Does
gc.cneed an extra #include for this?
Umm ... you'd think so, wonder how it compiled without kmod.h ....
Quoted 6 lines
security/keys/key.c struct key *key_alloc(struct key_type *type, const char *desc, · line 316
Quoted 4 lines
security/keys/key.c +3
-0+/* If running within a container use the container namespace */ +if (current->nsproxy->net_ns != &init_net) + key->umh_token =umh_wq_get_token (0 ,"keys" );So keys live in the networking namespace?
Perhaps checking the pid namespace would make more sense?
Quoted 15 lines
security/keys/request_key.c static int call_sbin_request_key(struct key_construction *cons, · line 198
Quoted 11 lines
security/keys/request_key.c +8
-2- ret =call_usermodehelper_keys (argv[0 ], argv, envp, keyring, -UMH_WAIT_PROC ); +/* If running within a container use the container namespace */ +if (key->umh_token) + ret =call_usermodehelper_keys_service (argv[0 ], argv, envp, + keyring, key->umh_token, +UMH_WAIT_PROC ); +else + ret =call_usermodehelper_keys (argv[0 ], argv, envp, + keyring,UMH_WAIT_PROC );
call_usermodehelper_keys_service()would appear to be superfluous. Ifkey->umh_tokenis 0, you callcall_usermodehelper_keys()which then callscall_usermodehelper_keys_service()with a 0 token...
Yeah, not really worth the additional function. IIRC there are no other callers of call_usermodehelper_keys().
Quoted 2 lines
David