All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH -mm 1/2] umh && creds: convert call_usermodehelper_keys() to use subprocess_info->init()
@ 2010-02-25 18:15 Oleg Nesterov
  2010-02-26 18:00 ` David Howells
  0 siblings, 1 reply; 20+ messages in thread
From: Oleg Nesterov @ 2010-02-25 18:15 UTC (permalink / raw)
  To: Andrew Morton, David Howells; +Cc: Andi Kleen, Neil Horman, linux-kernel

call_usermodehelper_keys() uses call_usermodehelper_setkeys() to change
subprocess_info->cred in advance. Now that we have info->init() we can
change this code to set tgcred->session_keyring in context of execing
kernel thread.

Note: since currently call_usermodehelper_keys() is never called with
UMH_NO_WAIT, umh_keys_cleanup() is not really needed, we could just move
key_get() from call_usermodehelper_keys() to umh_keys_init().

Compile tested.

Signed-off-by: Oleg Nesterov <oleg@redhat.com>
---

 include/linux/kmod.h        |   17 -----------------
 kernel/kmod.c               |   18 ------------------
 security/keys/request_key.c |   37 +++++++++++++++++++++++++++++++++++++
 3 files changed, 37 insertions(+), 35 deletions(-)

--- mm/include/linux/kmod.h~1_CONVERT_KEYS	2010-02-25 17:37:41.000000000 +0100
+++ mm/include/linux/kmod.h	2010-02-25 18:56:41.000000000 +0100
@@ -71,8 +71,6 @@ struct subprocess_info *call_usermodehel
 						  char **envp, gfp_t gfp_mask);
 
 /* Set various pieces of state into the subprocess_info structure */
-void call_usermodehelper_setkeys(struct subprocess_info *info,
-				 struct key *session_keyring);
 void call_usermodehelper_setfns(struct subprocess_info *info,
 		    int (*init)(struct subprocess_info *info),
 		    void (*cleanup)(struct subprocess_info *info),
@@ -108,21 +106,6 @@ call_usermodehelper(char *path, char **a
 				       wait, NULL, NULL, NULL);
 }
 
-static inline int
-call_usermodehelper_keys(char *path, char **argv, char **envp,
-			 struct key *session_keyring, enum umh_wait wait)
-{
-	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);
-	if (info == NULL)
-		return -ENOMEM;
-
-	call_usermodehelper_setkeys(info, session_keyring);
-	return call_usermodehelper_exec(info, wait);
-}
-
 extern void usermodehelper_init(void);
 
 extern int usermodehelper_disable(void);
--- mm/kernel/kmod.c~1_CONVERT_KEYS	2010-02-25 17:37:41.000000000 +0100
+++ mm/kernel/kmod.c	2010-02-25 18:56:41.000000000 +0100
@@ -386,24 +386,6 @@ struct subprocess_info *call_usermodehel
 EXPORT_SYMBOL(call_usermodehelper_setup);
 
 /**
- * call_usermodehelper_setkeys - set the session keys for usermode helper
- * @info: a subprocess_info returned by call_usermodehelper_setup
- * @session_keyring: the session keyring for the process
- */
-void call_usermodehelper_setkeys(struct subprocess_info *info,
-				 struct key *session_keyring)
-{
-#ifdef CONFIG_KEYS
-	struct thread_group_cred *tgcred = info->cred->tgcred;
-	key_put(tgcred->session_keyring);
-	tgcred->session_keyring = key_get(session_keyring);
-#else
-	BUG();
-#endif
-}
-EXPORT_SYMBOL(call_usermodehelper_setkeys);
-
-/**
  * call_usermodehelper_setfns - set a cleanup/init function
  * @info: a subprocess_info returned by call_usermodehelper_setup
  * @cleanup: a cleanup function
--- mm/security/keys/request_key.c~1_CONVERT_KEYS	2010-02-25 17:37:41.000000000 +0100
+++ mm/security/keys/request_key.c	2010-02-25 19:01:26.000000000 +0100
@@ -58,6 +58,43 @@ void complete_request_key(struct key_con
 }
 EXPORT_SYMBOL(complete_request_key);
 
+static int umh_keys_init(struct subprocess_info *info)
+{
+	struct thread_group_cred *tgcred = current_cred()->tgcred;
+	struct key *session_keyring = info->data;
+	/*
+	 * This is called in context of freshly forked kthread before
+	 * kernel_execve(), we can just change our ->session_keyring.
+	 */
+	WARN_ON(tgcred->session_keyring);
+	tgcred->session_keyring = session_keyring;
+
+	info->data = NULL;		/* for umh_keys_cleanup() */
+	return 0;
+}
+
+static void umh_keys_cleanup(struct subprocess_info *info)
+{
+	struct key *session_keyring = info->data;
+	key_put(session_keyring);	/* NULL if successs */
+}
+
+static inline int
+call_usermodehelper_keys(char *path, char **argv, char **envp,
+			 struct key *session_keyring, enum umh_wait wait)
+{
+	gfp_t gfp_mask = (wait == UMH_NO_WAIT) ? GFP_ATOMIC : GFP_KERNEL;
+	struct subprocess_info *info =
+		call_usermodehelper_setup(path, argv, envp, gfp_mask);
+
+	if (!info)
+		return -ENOMEM;
+
+	call_usermodehelper_setfns(info, umh_keys_init, umh_keys_cleanup,
+					key_get(session_keyring));
+	return call_usermodehelper_exec(info, wait);
+}
+
 /*
  * request userspace finish the construction of a key
  * - execute "/sbin/request-key <op> <key> <uid> <gid> <keyring> <keyring> <keyring>"


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH -mm 1/2] umh && creds: convert call_usermodehelper_keys() to use subprocess_info->init()
  2010-02-25 18:15 [PATCH -mm 1/2] umh && creds: convert call_usermodehelper_keys() to use subprocess_info->init() Oleg Nesterov
@ 2010-02-26 18:00 ` David Howells
  2010-02-26 18:23   ` Oleg Nesterov
  2010-02-26 18:41   ` David Howells
  0 siblings, 2 replies; 20+ messages in thread
From: David Howells @ 2010-02-26 18:00 UTC (permalink / raw)
  To: Oleg Nesterov
  Cc: dhowells, Andrew Morton, Andi Kleen, Neil Horman, linux-kernel

Oleg Nesterov <oleg@redhat.com> wrote:

> +	/*
> +	 * This is called in context of freshly forked kthread before
> +	 * kernel_execve(), we can just change our ->session_keyring.
> +	 */

Ummmm....  What gives you that idea?

You may be sharing init_cred...

You need to create, modify and commit a new set of creds.

David

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH -mm 1/2] umh && creds: convert call_usermodehelper_keys() to use subprocess_info->init()
  2010-02-26 18:00 ` David Howells
@ 2010-02-26 18:23   ` Oleg Nesterov
  2010-02-26 18:41   ` David Howells
  1 sibling, 0 replies; 20+ messages in thread
From: Oleg Nesterov @ 2010-02-26 18:23 UTC (permalink / raw)
  To: David Howells; +Cc: Andrew Morton, Andi Kleen, Neil Horman, linux-kernel

On 02/26, David Howells wrote:
>
> Oleg Nesterov <oleg@redhat.com> wrote:
>
> > +	/*
> > +	 * This is called in context of freshly forked kthread before
> > +	 * kernel_execve(), we can just change our ->session_keyring.
> > +	 */
>
> Ummmm....  What gives you that idea?
>
> You may be sharing init_cred...

How? Without CLONE_THREAD copy_creds() always creates the new
struct cred for the child?

Oleg.


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH -mm 1/2] umh && creds: convert call_usermodehelper_keys() to use subprocess_info->init()
  2010-02-26 18:00 ` David Howells
  2010-02-26 18:23   ` Oleg Nesterov
@ 2010-02-26 18:41   ` David Howells
  2010-02-26 18:52     ` Oleg Nesterov
  2010-02-26 20:03     ` [PATCH v2 -mm 0/2] umh && creds: kill sub_info->cred Oleg Nesterov
  1 sibling, 2 replies; 20+ messages in thread
From: David Howells @ 2010-02-26 18:41 UTC (permalink / raw)
  To: Oleg Nesterov
  Cc: dhowells, Andrew Morton, Andi Kleen, Neil Horman, linux-kernel

Oleg Nesterov <oleg@redhat.com> wrote:

> How? Without CLONE_THREAD copy_creds() always creates the new
> struct cred for the child?

Okay, that's a good point.

In which case, you should be calling install_session_keyring_to_cred(), just
so that the credentials installation is always done by the same piece of code.

David

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH -mm 1/2] umh && creds: convert call_usermodehelper_keys() to use subprocess_info->init()
  2010-02-26 18:41   ` David Howells
@ 2010-02-26 18:52     ` Oleg Nesterov
  2010-02-26 20:03     ` [PATCH v2 -mm 0/2] umh && creds: kill sub_info->cred Oleg Nesterov
  1 sibling, 0 replies; 20+ messages in thread
From: Oleg Nesterov @ 2010-02-26 18:52 UTC (permalink / raw)
  To: David Howells; +Cc: Andrew Morton, Andi Kleen, Neil Horman, linux-kernel

On 02/26, David Howells wrote:
>
> In which case, you should be calling install_session_keyring_to_cred(), just
> so that the credentials installation is always done by the same piece of code.

OK, will redo and resend tomorrow, thanks.

Oleg.


^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH v2 -mm 0/2] umh && creds: kill sub_info->cred
  2010-02-26 18:41   ` David Howells
  2010-02-26 18:52     ` Oleg Nesterov
@ 2010-02-26 20:03     ` Oleg Nesterov
  2010-02-26 20:03       ` [PATCH v2 -mm 1/2] umh && creds: convert call_usermodehelper_keys() to use subprocess_info->init() Oleg Nesterov
                         ` (2 more replies)
  1 sibling, 3 replies; 20+ messages in thread
From: Oleg Nesterov @ 2010-02-26 20:03 UTC (permalink / raw)
  To: David Howells, Andrew Morton; +Cc: Andi Kleen, Neil Horman, linux-kernel

Changes:

	1/2: do not change ->session_keyring by hand, use
	     install_session_keyring_to_cred() helper as David suggested.

	2/2: unchanged


Andrew, this is on top of

	kmod-replace-call_usermodehelper_pipe-with-use-of-umh-init-function-and-resolve-limit.patch

plus trivial

	"[PATCH -mm] kmod-replace-call_usermodehelper_pipe-with-use-of-umh-init-function- and-resolve-limit.cleanup"
	http://marc.info/?l=linux-kernel&m=126711062108298

cleanup.

Oleg.



^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH v2 -mm 1/2] umh && creds: convert call_usermodehelper_keys() to use subprocess_info->init()
  2010-02-26 20:03     ` [PATCH v2 -mm 0/2] umh && creds: kill sub_info->cred Oleg Nesterov
@ 2010-02-26 20:03       ` Oleg Nesterov
  2010-02-26 20:28         ` Neil Horman
  2010-02-26 20:04       ` [PATCH v2 -mm 2/2] umh && creds: kill subprocess_info->cred logic Oleg Nesterov
  2010-02-26 20:42       ` [PATCH v2 -mm 1/2] umh && creds: convert call_usermodehelper_keys() to use subprocess_info->init() David Howells
  2 siblings, 1 reply; 20+ messages in thread
From: Oleg Nesterov @ 2010-02-26 20:03 UTC (permalink / raw)
  To: David Howells, Andrew Morton; +Cc: Andi Kleen, Neil Horman, linux-kernel

call_usermodehelper_keys() uses call_usermodehelper_setkeys() to change
subprocess_info->cred in advance. Now that we have info->init() we can
change this code to set tgcred->session_keyring in context of execing
kernel thread.

Note: since currently call_usermodehelper_keys() is never called with
UMH_NO_WAIT, call_usermodehelper_keys()->key_get() and umh_keys_cleanup()
are not really needed, we could rely on install_session_keyring_to_cred()
which does key_get() on success.

Compile tested.

Signed-off-by: Oleg Nesterov <oleg@redhat.com>
---

 include/linux/kmod.h         |   17 -----------------
 kernel/kmod.c                |   18 ------------------
 security/keys/internal.h     |    1 +
 security/keys/process_keys.c |    3 +--
 security/keys/request_key.c  |   33 +++++++++++++++++++++++++++++++++
 5 files changed, 35 insertions(+), 37 deletions(-)

--- mm/include/linux/kmod.h~1_CONVERT_KEYS	2010-02-25 17:37:41.000000000 +0100
+++ mm/include/linux/kmod.h	2010-02-26 20:18:48.000000000 +0100
@@ -71,8 +71,6 @@ struct subprocess_info *call_usermodehel
 						  char **envp, gfp_t gfp_mask);
 
 /* Set various pieces of state into the subprocess_info structure */
-void call_usermodehelper_setkeys(struct subprocess_info *info,
-				 struct key *session_keyring);
 void call_usermodehelper_setfns(struct subprocess_info *info,
 		    int (*init)(struct subprocess_info *info),
 		    void (*cleanup)(struct subprocess_info *info),
@@ -108,21 +106,6 @@ call_usermodehelper(char *path, char **a
 				       wait, NULL, NULL, NULL);
 }
 
-static inline int
-call_usermodehelper_keys(char *path, char **argv, char **envp,
-			 struct key *session_keyring, enum umh_wait wait)
-{
-	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);
-	if (info == NULL)
-		return -ENOMEM;
-
-	call_usermodehelper_setkeys(info, session_keyring);
-	return call_usermodehelper_exec(info, wait);
-}
-
 extern void usermodehelper_init(void);
 
 extern int usermodehelper_disable(void);
--- mm/kernel/kmod.c~1_CONVERT_KEYS	2010-02-25 17:37:41.000000000 +0100
+++ mm/kernel/kmod.c	2010-02-26 20:18:48.000000000 +0100
@@ -386,24 +386,6 @@ struct subprocess_info *call_usermodehel
 EXPORT_SYMBOL(call_usermodehelper_setup);
 
 /**
- * call_usermodehelper_setkeys - set the session keys for usermode helper
- * @info: a subprocess_info returned by call_usermodehelper_setup
- * @session_keyring: the session keyring for the process
- */
-void call_usermodehelper_setkeys(struct subprocess_info *info,
-				 struct key *session_keyring)
-{
-#ifdef CONFIG_KEYS
-	struct thread_group_cred *tgcred = info->cred->tgcred;
-	key_put(tgcred->session_keyring);
-	tgcred->session_keyring = key_get(session_keyring);
-#else
-	BUG();
-#endif
-}
-EXPORT_SYMBOL(call_usermodehelper_setkeys);
-
-/**
  * call_usermodehelper_setfns - set a cleanup/init function
  * @info: a subprocess_info returned by call_usermodehelper_setup
  * @cleanup: a cleanup function
--- mm/security/keys/internal.h~1_CONVERT_KEYS	2010-02-25 15:22:14.000000000 +0100
+++ mm/security/keys/internal.h	2010-02-26 20:30:52.000000000 +0100
@@ -115,6 +115,7 @@ extern struct key *find_keyring_by_name(
 extern int install_user_keyrings(void);
 extern int install_thread_keyring_to_cred(struct cred *);
 extern int install_process_keyring_to_cred(struct cred *);
+extern int install_session_keyring_to_cred(struct cred *, struct key *);
 
 extern struct key *request_key_and_link(struct key_type *type,
 					const char *description,
--- mm/security/keys/process_keys.c~1_CONVERT_KEYS	2010-02-25 15:22:14.000000000 +0100
+++ mm/security/keys/process_keys.c	2010-02-26 20:22:14.000000000 +0100
@@ -217,8 +217,7 @@ static int install_process_keyring(void)
 /*
  * install a session keyring directly to a credentials struct
  */
-static int install_session_keyring_to_cred(struct cred *cred,
-					   struct key *keyring)
+int install_session_keyring_to_cred(struct cred *cred, struct key *keyring)
 {
 	unsigned long flags;
 	struct key *old;
--- mm/security/keys/request_key.c~1_CONVERT_KEYS	2010-02-25 17:37:41.000000000 +0100
+++ mm/security/keys/request_key.c	2010-02-26 20:42:56.000000000 +0100
@@ -58,6 +58,39 @@ void complete_request_key(struct key_con
 }
 EXPORT_SYMBOL(complete_request_key);
 
+static int umh_keys_init(struct subprocess_info *info)
+{
+	struct cred *cred = (struct cred*)current_cred();
+	struct key *keyring = info->data;
+	/*
+	 * This is called in context of freshly forked kthread before
+	 * kernel_execve(), we can just change our ->session_keyring.
+	 */
+	return install_session_keyring_to_cred(cred, keyring);
+}
+
+static void umh_keys_cleanup(struct subprocess_info *info)
+{
+	struct key *keyring = info->data;
+	key_put(keyring);
+}
+
+static inline int
+call_usermodehelper_keys(char *path, char **argv, char **envp,
+			 struct key *session_keyring, enum umh_wait wait)
+{
+	gfp_t gfp_mask = (wait == UMH_NO_WAIT) ? GFP_ATOMIC : GFP_KERNEL;
+	struct subprocess_info *info =
+		call_usermodehelper_setup(path, argv, envp, gfp_mask);
+
+	if (!info)
+		return -ENOMEM;
+
+	call_usermodehelper_setfns(info, umh_keys_init, umh_keys_cleanup,
+					key_get(session_keyring));
+	return call_usermodehelper_exec(info, wait);
+}
+
 /*
  * request userspace finish the construction of a key
  * - execute "/sbin/request-key <op> <key> <uid> <gid> <keyring> <keyring> <keyring>"


^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH v2 -mm 2/2] umh && creds: kill subprocess_info->cred logic
  2010-02-26 20:03     ` [PATCH v2 -mm 0/2] umh && creds: kill sub_info->cred Oleg Nesterov
  2010-02-26 20:03       ` [PATCH v2 -mm 1/2] umh && creds: convert call_usermodehelper_keys() to use subprocess_info->init() Oleg Nesterov
@ 2010-02-26 20:04       ` Oleg Nesterov
  2010-02-26 20:29         ` Neil Horman
  2010-02-26 20:42       ` [PATCH v2 -mm 1/2] umh && creds: convert call_usermodehelper_keys() to use subprocess_info->init() David Howells
  2 siblings, 1 reply; 20+ messages in thread
From: Oleg Nesterov @ 2010-02-26 20:04 UTC (permalink / raw)
  To: David Howells, Andrew Morton; +Cc: Andi Kleen, Neil Horman, linux-kernel

Now that nobody ever changes subprocess_info->cred we can kill this
member and related code. ____call_usermodehelper() always runs in the
context of freshly forked kernel thread, it has the proper ->cred
copied from its parent kthread, keventd.

Signed-off-by: Oleg Nesterov <oleg@redhat.com>
---

 include/linux/cred.h |    1 
 include/linux/kmod.h |    1 
 kernel/cred.c        |   54 ---------------------------------------------------
 kernel/kmod.c        |   19 -----------------
 4 files changed, 75 deletions(-)

--- mm/include/linux/cred.h~2_KILL_INFO_CRED	2010-02-26 20:18:48.000000000 +0100
+++ mm/include/linux/cred.h	2010-02-26 20:53:04.000000000 +0100
@@ -156,7 +156,6 @@ extern int copy_creds(struct task_struct
 extern struct cred *cred_alloc_blank(void);
 extern struct cred *prepare_creds(void);
 extern struct cred *prepare_exec_creds(void);
-extern struct cred *prepare_usermodehelper_creds(void);
 extern int commit_creds(struct cred *);
 extern void abort_creds(struct cred *);
 extern const struct cred *override_creds(const struct cred *);
--- mm/include/linux/kmod.h~2_KILL_INFO_CRED	2010-02-26 20:18:48.000000000 +0100
+++ mm/include/linux/kmod.h	2010-02-26 20:53:04.000000000 +0100
@@ -55,7 +55,6 @@ enum umh_wait {
 struct subprocess_info {
 	struct work_struct work;
 	struct completion *complete;
-	struct cred *cred;
 	char *path;
 	char **argv;
 	char **envp;
--- mm/kernel/cred.c~2_KILL_INFO_CRED	2010-02-26 20:18:48.000000000 +0100
+++ mm/kernel/cred.c	2010-02-26 20:53:04.000000000 +0100
@@ -347,60 +347,6 @@ struct cred *prepare_exec_creds(void)
 }
 
 /*
- * prepare new credentials for the usermode helper dispatcher
- */
-struct cred *prepare_usermodehelper_creds(void)
-{
-#ifdef CONFIG_KEYS
-	struct thread_group_cred *tgcred = NULL;
-#endif
-	struct cred *new;
-
-#ifdef CONFIG_KEYS
-	tgcred = kzalloc(sizeof(*new->tgcred), GFP_ATOMIC);
-	if (!tgcred)
-		return NULL;
-#endif
-
-	new = kmem_cache_alloc(cred_jar, GFP_ATOMIC);
-	if (!new)
-		return NULL;
-
-	kdebug("prepare_usermodehelper_creds() alloc %p", new);
-
-	memcpy(new, &init_cred, sizeof(struct cred));
-
-	atomic_set(&new->usage, 1);
-	set_cred_subscribers(new, 0);
-	get_group_info(new->group_info);
-	get_uid(new->user);
-
-#ifdef CONFIG_KEYS
-	new->thread_keyring = NULL;
-	new->request_key_auth = NULL;
-	new->jit_keyring = KEY_REQKEY_DEFL_DEFAULT;
-
-	atomic_set(&tgcred->usage, 1);
-	spin_lock_init(&tgcred->lock);
-	new->tgcred = tgcred;
-#endif
-
-#ifdef CONFIG_SECURITY
-	new->security = NULL;
-#endif
-	if (security_prepare_creds(new, &init_cred, GFP_ATOMIC) < 0)
-		goto error;
-	validate_creds(new);
-
-	BUG_ON(atomic_read(&new->usage) != 1);
-	return new;
-
-error:
-	put_cred(new);
-	return NULL;
-}
-
-/*
  * Copy credentials for the new process created by fork()
  *
  * We share if we can, but under some circumstances we have to generate a new
--- mm/kernel/kmod.c~2_KILL_INFO_CRED	2010-02-26 20:18:48.000000000 +0100
+++ mm/kernel/kmod.c	2010-02-26 20:53:04.000000000 +0100
@@ -153,8 +153,6 @@ static int ____call_usermodehelper(void 
 	struct subprocess_info *sub_info = data;
 	int retval;
 
-	BUG_ON(atomic_read(&sub_info->cred->usage) != 1);
-
 	/* Unblock all signals */
 	spin_lock_irq(&current->sighand->siglock);
 	flush_signal_handlers(current, 1);
@@ -162,10 +160,6 @@ static int ____call_usermodehelper(void 
 	recalc_sigpending();
 	spin_unlock_irq(&current->sighand->siglock);
 
-	/* Install the credentials */
-	commit_creds(sub_info->cred);
-	sub_info->cred = NULL;
-
 	/* We can run anywhere, unlike our parent keventd(). */
 	set_cpus_allowed_ptr(current, cpu_all_mask);
 
@@ -193,8 +187,6 @@ void call_usermodehelper_freeinfo(struct
 {
 	if (info->cleanup)
 		(*info->cleanup)(info);
-	if (info->cred)
-		put_cred(info->cred);
 	kfree(info);
 }
 EXPORT_SYMBOL(call_usermodehelper_freeinfo);
@@ -250,8 +242,6 @@ static void __call_usermodehelper(struct
 	pid_t pid;
 	enum umh_wait wait = sub_info->wait;
 
-	BUG_ON(atomic_read(&sub_info->cred->usage) != 1);
-
 	/* CLONE_VFORK: wait until the usermode helper has execve'd
 	 * successfully We need the data structures to stay around
 	 * until that is done.  */
@@ -374,12 +364,6 @@ struct subprocess_info *call_usermodehel
 	sub_info->path = path;
 	sub_info->argv = argv;
 	sub_info->envp = envp;
-	sub_info->cred = prepare_usermodehelper_creds();
-	if (!sub_info->cred) {
-		kfree(sub_info);
-		return NULL;
-	}
-
   out:
 	return sub_info;
 }
@@ -430,9 +414,6 @@ int call_usermodehelper_exec(struct subp
 	DECLARE_COMPLETION_ONSTACK(done);
 	int retval = 0;
 
-	BUG_ON(atomic_read(&sub_info->cred->usage) != 1);
-	validate_creds(sub_info->cred);
-
 	helper_lock();
 	if (sub_info->path[0] == '\0')
 		goto out;


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v2 -mm 1/2] umh && creds: convert call_usermodehelper_keys() to use subprocess_info->init()
  2010-02-26 20:03       ` [PATCH v2 -mm 1/2] umh && creds: convert call_usermodehelper_keys() to use subprocess_info->init() Oleg Nesterov
@ 2010-02-26 20:28         ` Neil Horman
  0 siblings, 0 replies; 20+ messages in thread
From: Neil Horman @ 2010-02-26 20:28 UTC (permalink / raw)
  To: Oleg Nesterov; +Cc: David Howells, Andrew Morton, Andi Kleen, linux-kernel

On Fri, Feb 26, 2010 at 09:03:57PM +0100, Oleg Nesterov wrote:
> call_usermodehelper_keys() uses call_usermodehelper_setkeys() to change
> subprocess_info->cred in advance. Now that we have info->init() we can
> change this code to set tgcred->session_keyring in context of execing
> kernel thread.
> 
> Note: since currently call_usermodehelper_keys() is never called with
> UMH_NO_WAIT, call_usermodehelper_keys()->key_get() and umh_keys_cleanup()
> are not really needed, we could rely on install_session_keyring_to_cred()
> which does key_get() on success.
> 
> Compile tested.
> 
> Signed-off-by: Oleg Nesterov <oleg@redhat.com>
Acked-by: Neil Horman <nhorman@tuxdriver.com>


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v2 -mm 2/2] umh && creds: kill subprocess_info->cred logic
  2010-02-26 20:04       ` [PATCH v2 -mm 2/2] umh && creds: kill subprocess_info->cred logic Oleg Nesterov
@ 2010-02-26 20:29         ` Neil Horman
  0 siblings, 0 replies; 20+ messages in thread
From: Neil Horman @ 2010-02-26 20:29 UTC (permalink / raw)
  To: Oleg Nesterov; +Cc: David Howells, Andrew Morton, Andi Kleen, linux-kernel

On Fri, Feb 26, 2010 at 09:04:35PM +0100, Oleg Nesterov wrote:
> Now that nobody ever changes subprocess_info->cred we can kill this
> member and related code. ____call_usermodehelper() always runs in the
> context of freshly forked kernel thread, it has the proper ->cred
> copied from its parent kthread, keventd.
> 
> Signed-off-by: Oleg Nesterov <oleg@redhat.com>
Acked-by: Neil Horman <nhorman@tuxdriver.com>


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v2 -mm 1/2] umh && creds: convert call_usermodehelper_keys() to use subprocess_info->init()
  2010-02-26 20:03     ` [PATCH v2 -mm 0/2] umh && creds: kill sub_info->cred Oleg Nesterov
  2010-02-26 20:03       ` [PATCH v2 -mm 1/2] umh && creds: convert call_usermodehelper_keys() to use subprocess_info->init() Oleg Nesterov
  2010-02-26 20:04       ` [PATCH v2 -mm 2/2] umh && creds: kill subprocess_info->cred logic Oleg Nesterov
@ 2010-02-26 20:42       ` David Howells
  2010-02-26 20:53         ` Oleg Nesterov
  2010-02-26 23:24         ` David Howells
  2 siblings, 2 replies; 20+ messages in thread
From: David Howells @ 2010-02-26 20:42 UTC (permalink / raw)
  To: Oleg Nesterov
  Cc: dhowells, Andrew Morton, Andi Kleen, Neil Horman, linux-kernel

Oleg Nesterov <oleg@redhat.com> wrote:

> +static inline int
> +call_usermodehelper_keys(char *path, char **argv, char **envp,
> +			 struct key *session_keyring, enum umh_wait wait)

You should get rid of the inline there.  I think it's okay otherwise.

David

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v2 -mm 1/2] umh && creds: convert call_usermodehelper_keys() to use subprocess_info->init()
  2010-02-26 20:42       ` [PATCH v2 -mm 1/2] umh && creds: convert call_usermodehelper_keys() to use subprocess_info->init() David Howells
@ 2010-02-26 20:53         ` Oleg Nesterov
  2010-02-26 23:24         ` David Howells
  1 sibling, 0 replies; 20+ messages in thread
From: Oleg Nesterov @ 2010-02-26 20:53 UTC (permalink / raw)
  To: David Howells; +Cc: Andrew Morton, Andi Kleen, Neil Horman, linux-kernel

On 02/26, David Howells wrote:
>
> Oleg Nesterov <oleg@redhat.com> wrote:
>
> > +static inline int
> > +call_usermodehelper_keys(char *path, char **argv, char **envp,
> > +			 struct key *session_keyring, enum umh_wait wait)
>
> You should get rid of the inline there.  I think it's okay otherwise.

OK, please find v3 below.

(Neil, I retained your ack, hopefully you won't object...)

------------------------------------------------------------------------------
[PATCH v3 -mm 1/2] umh && creds: convert call_usermodehelper_keys() to use subprocess_info->init()

call_usermodehelper_keys() uses call_usermodehelper_setkeys() to change
subprocess_info->cred in advance. Now that we have info->init() we can
change this code to set tgcred->session_keyring in context of execing
kernel thread.

Note: since currently call_usermodehelper_keys() is never called with
UMH_NO_WAIT, call_usermodehelper_keys()->key_get() and umh_keys_cleanup()
are not really needed, we could rely on install_session_keyring_to_cred()
which does key_get() on success.

Compile tested.

Signed-off-by: Oleg Nesterov <oleg@redhat.com>
Acked-by: Neil Horman <nhorman@tuxdriver.com>
---

 include/linux/kmod.h         |   17 -----------------
 kernel/kmod.c                |   18 ------------------
 security/keys/internal.h     |    1 +
 security/keys/process_keys.c |    3 +--
 security/keys/request_key.c  |   32 ++++++++++++++++++++++++++++++++
 5 files changed, 34 insertions(+), 37 deletions(-)

--- mm/include/linux/kmod.h~1_CONVERT_KEYS	2010-02-25 17:37:41.000000000 +0100
+++ mm/include/linux/kmod.h	2010-02-26 21:45:28.000000000 +0100
@@ -71,8 +71,6 @@ struct subprocess_info *call_usermodehel
 						  char **envp, gfp_t gfp_mask);
 
 /* Set various pieces of state into the subprocess_info structure */
-void call_usermodehelper_setkeys(struct subprocess_info *info,
-				 struct key *session_keyring);
 void call_usermodehelper_setfns(struct subprocess_info *info,
 		    int (*init)(struct subprocess_info *info),
 		    void (*cleanup)(struct subprocess_info *info),
@@ -108,21 +106,6 @@ call_usermodehelper(char *path, char **a
 				       wait, NULL, NULL, NULL);
 }
 
-static inline int
-call_usermodehelper_keys(char *path, char **argv, char **envp,
-			 struct key *session_keyring, enum umh_wait wait)
-{
-	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);
-	if (info == NULL)
-		return -ENOMEM;
-
-	call_usermodehelper_setkeys(info, session_keyring);
-	return call_usermodehelper_exec(info, wait);
-}
-
 extern void usermodehelper_init(void);
 
 extern int usermodehelper_disable(void);
--- mm/kernel/kmod.c~1_CONVERT_KEYS	2010-02-25 17:37:41.000000000 +0100
+++ mm/kernel/kmod.c	2010-02-26 21:45:28.000000000 +0100
@@ -386,24 +386,6 @@ struct subprocess_info *call_usermodehel
 EXPORT_SYMBOL(call_usermodehelper_setup);
 
 /**
- * call_usermodehelper_setkeys - set the session keys for usermode helper
- * @info: a subprocess_info returned by call_usermodehelper_setup
- * @session_keyring: the session keyring for the process
- */
-void call_usermodehelper_setkeys(struct subprocess_info *info,
-				 struct key *session_keyring)
-{
-#ifdef CONFIG_KEYS
-	struct thread_group_cred *tgcred = info->cred->tgcred;
-	key_put(tgcred->session_keyring);
-	tgcred->session_keyring = key_get(session_keyring);
-#else
-	BUG();
-#endif
-}
-EXPORT_SYMBOL(call_usermodehelper_setkeys);
-
-/**
  * call_usermodehelper_setfns - set a cleanup/init function
  * @info: a subprocess_info returned by call_usermodehelper_setup
  * @cleanup: a cleanup function
--- mm/security/keys/internal.h~1_CONVERT_KEYS	2010-02-25 15:22:14.000000000 +0100
+++ mm/security/keys/internal.h	2010-02-26 20:30:52.000000000 +0100
@@ -115,6 +115,7 @@ extern struct key *find_keyring_by_name(
 extern int install_user_keyrings(void);
 extern int install_thread_keyring_to_cred(struct cred *);
 extern int install_process_keyring_to_cred(struct cred *);
+extern int install_session_keyring_to_cred(struct cred *, struct key *);
 
 extern struct key *request_key_and_link(struct key_type *type,
 					const char *description,
--- mm/security/keys/process_keys.c~1_CONVERT_KEYS	2010-02-25 15:22:14.000000000 +0100
+++ mm/security/keys/process_keys.c	2010-02-26 20:22:14.000000000 +0100
@@ -217,8 +217,7 @@ static int install_process_keyring(void)
 /*
  * install a session keyring directly to a credentials struct
  */
-static int install_session_keyring_to_cred(struct cred *cred,
-					   struct key *keyring)
+int install_session_keyring_to_cred(struct cred *cred, struct key *keyring)
 {
 	unsigned long flags;
 	struct key *old;
--- mm/security/keys/request_key.c~1_CONVERT_KEYS	2010-02-25 17:37:41.000000000 +0100
+++ mm/security/keys/request_key.c	2010-02-26 21:46:23.000000000 +0100
@@ -58,6 +58,38 @@ void complete_request_key(struct key_con
 }
 EXPORT_SYMBOL(complete_request_key);
 
+static int umh_keys_init(struct subprocess_info *info)
+{
+	struct cred *cred = (struct cred*)current_cred();
+	struct key *keyring = info->data;
+	/*
+	 * This is called in context of freshly forked kthread before
+	 * kernel_execve(), we can just change our ->session_keyring.
+	 */
+	return install_session_keyring_to_cred(cred, keyring);
+}
+
+static void umh_keys_cleanup(struct subprocess_info *info)
+{
+	struct key *keyring = info->data;
+	key_put(keyring);
+}
+
+static int call_usermodehelper_keys(char *path, char **argv, char **envp,
+			 struct key *session_keyring, enum umh_wait wait)
+{
+	gfp_t gfp_mask = (wait == UMH_NO_WAIT) ? GFP_ATOMIC : GFP_KERNEL;
+	struct subprocess_info *info =
+		call_usermodehelper_setup(path, argv, envp, gfp_mask);
+
+	if (!info)
+		return -ENOMEM;
+
+	call_usermodehelper_setfns(info, umh_keys_init, umh_keys_cleanup,
+					key_get(session_keyring));
+	return call_usermodehelper_exec(info, wait);
+}
+
 /*
  * request userspace finish the construction of a key
  * - execute "/sbin/request-key <op> <key> <uid> <gid> <keyring> <keyring> <keyring>"


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v2 -mm 1/2] umh && creds: convert call_usermodehelper_keys() to use subprocess_info->init()
  2010-02-26 20:42       ` [PATCH v2 -mm 1/2] umh && creds: convert call_usermodehelper_keys() to use subprocess_info->init() David Howells
  2010-02-26 20:53         ` Oleg Nesterov
@ 2010-02-26 23:24         ` David Howells
  2010-03-05 22:52           ` Oleg Nesterov
  1 sibling, 1 reply; 20+ messages in thread
From: David Howells @ 2010-02-26 23:24 UTC (permalink / raw)
  To: Oleg Nesterov
  Cc: dhowells, Andrew Morton, Andi Kleen, Neil Horman, linux-kernel


I'm not going to be able to test it before I get back from skiing in a week's
time, but it looks ackable otherwise.

It can be tested by doing something like this:

	[root@andromeda ~]# keyctl request2 user debug:a b @s
	102880396
	[root@andromeda ~]# keyctl show
	Session Keyring
	       -3 --alswrv      0     0  keyring: _ses
	404996112 --alswrv      0    -1   \_ keyring: _uid.0
	102880396 --alswrv      0     0   \_ user: debug:a
	[root@andromeda ~]# keyctl print 102880396
	Debug b

On a system with the keyutils package installed.

David

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v2 -mm 1/2] umh && creds: convert call_usermodehelper_keys() to use subprocess_info->init()
  2010-02-26 23:24         ` David Howells
@ 2010-03-05 22:52           ` Oleg Nesterov
  2010-03-05 23:09             ` [PATCH,RESEND " Oleg Nesterov
                               ` (3 more replies)
  0 siblings, 4 replies; 20+ messages in thread
From: Oleg Nesterov @ 2010-03-05 22:52 UTC (permalink / raw)
  To: David Howells; +Cc: Andrew Morton, Andi Kleen, Neil Horman, linux-kernel

On 02/26, David Howells wrote:
>
> I'm not going to be able to test it before I get back from skiing in a week's
> time, but it looks ackable otherwise.
>
> It can be tested by doing something like this:
>
> 	[root@andromeda ~]# keyctl request2 user debug:a b @s
> 	102880396
> 	[root@andromeda ~]# keyctl show
> 	Session Keyring
> 	       -3 --alswrv      0     0  keyring: _ses
> 	404996112 --alswrv      0    -1   \_ keyring: _uid.0
> 	102880396 --alswrv      0     0   \_ user: debug:a
> 	[root@andromeda ~]# keyctl print 102880396
> 	Debug b
>
> On a system with the keyutils package installed.

Thanks David.

Finally I reserved the machine for the testing,

	[root@hp-dl360-01 ~]# keyctl request2 user debug:a b @s
	623133085
	[root@hp-dl360-01 ~]# keyctl show
	Session Keyring
	       -3 --alswrv      0     0  keyring: _ses
	845914766 --alswrv      0    -1   \_ keyring: _uid.0
	623133085 --alswrv      0     0   \_ user: debug:a
	[root@hp-dl360-01 ~]# keyctl print 623133085
	Debug b

Hopefully this all is right.

Oleg.


^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH,RESEND -mm 1/2] umh && creds: convert call_usermodehelper_keys() to use subprocess_info->init()
  2010-03-05 22:52           ` Oleg Nesterov
@ 2010-03-05 23:09             ` Oleg Nesterov
  2010-03-08 17:44               ` Neil Horman
  2010-03-05 23:10             ` [PATCH,RESEND -mm 2/2] umh && creds: kill subprocess_info->cred logic Oleg Nesterov
                               ` (2 subsequent siblings)
  3 siblings, 1 reply; 20+ messages in thread
From: Oleg Nesterov @ 2010-03-05 23:09 UTC (permalink / raw)
  To: Andrew Morton; +Cc: Andi Kleen, Neil Horman, linux-kernel, David Howells

(on top of kmod-replace-call_usermodehelper_pipe-with-use-of-umh-init-function-and-resolve-limit.patch)

call_usermodehelper_keys() uses call_usermodehelper_setkeys() to change
subprocess_info->cred in advance. Now that we have info->init() we can
change this code to set tgcred->session_keyring in context of execing
kernel thread.

Note: since currently call_usermodehelper_keys() is never called with
UMH_NO_WAIT, call_usermodehelper_keys()->key_get() and umh_keys_cleanup()
are not really needed, we could rely on install_session_keyring_to_cred()
which does key_get() on success.

Signed-off-by: Oleg Nesterov <oleg@redhat.com>
Acked-by: Neil Horman <nhorman@tuxdriver.com>
---

 include/linux/kmod.h         |   17 -----------------
 kernel/kmod.c                |   18 ------------------
 security/keys/internal.h     |    1 +
 security/keys/process_keys.c |    3 +--
 security/keys/request_key.c  |   32 ++++++++++++++++++++++++++++++++
 5 files changed, 34 insertions(+), 37 deletions(-)

--- mm/include/linux/kmod.h~1_CONVERT_KEYS	2010-02-25 17:37:41.000000000 +0100
+++ mm/include/linux/kmod.h	2010-02-26 21:45:28.000000000 +0100
@@ -71,8 +71,6 @@ struct subprocess_info *call_usermodehel
 						  char **envp, gfp_t gfp_mask);
 
 /* Set various pieces of state into the subprocess_info structure */
-void call_usermodehelper_setkeys(struct subprocess_info *info,
-				 struct key *session_keyring);
 void call_usermodehelper_setfns(struct subprocess_info *info,
 		    int (*init)(struct subprocess_info *info),
 		    void (*cleanup)(struct subprocess_info *info),
@@ -108,21 +106,6 @@ call_usermodehelper(char *path, char **a
 				       wait, NULL, NULL, NULL);
 }
 
-static inline int
-call_usermodehelper_keys(char *path, char **argv, char **envp,
-			 struct key *session_keyring, enum umh_wait wait)
-{
-	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);
-	if (info == NULL)
-		return -ENOMEM;
-
-	call_usermodehelper_setkeys(info, session_keyring);
-	return call_usermodehelper_exec(info, wait);
-}
-
 extern void usermodehelper_init(void);
 
 extern int usermodehelper_disable(void);
--- mm/kernel/kmod.c~1_CONVERT_KEYS	2010-02-25 17:37:41.000000000 +0100
+++ mm/kernel/kmod.c	2010-02-26 21:45:28.000000000 +0100
@@ -386,24 +386,6 @@ struct subprocess_info *call_usermodehel
 EXPORT_SYMBOL(call_usermodehelper_setup);
 
 /**
- * call_usermodehelper_setkeys - set the session keys for usermode helper
- * @info: a subprocess_info returned by call_usermodehelper_setup
- * @session_keyring: the session keyring for the process
- */
-void call_usermodehelper_setkeys(struct subprocess_info *info,
-				 struct key *session_keyring)
-{
-#ifdef CONFIG_KEYS
-	struct thread_group_cred *tgcred = info->cred->tgcred;
-	key_put(tgcred->session_keyring);
-	tgcred->session_keyring = key_get(session_keyring);
-#else
-	BUG();
-#endif
-}
-EXPORT_SYMBOL(call_usermodehelper_setkeys);
-
-/**
  * call_usermodehelper_setfns - set a cleanup/init function
  * @info: a subprocess_info returned by call_usermodehelper_setup
  * @cleanup: a cleanup function
--- mm/security/keys/internal.h~1_CONVERT_KEYS	2010-02-25 15:22:14.000000000 +0100
+++ mm/security/keys/internal.h	2010-02-26 20:30:52.000000000 +0100
@@ -115,6 +115,7 @@ extern struct key *find_keyring_by_name(
 extern int install_user_keyrings(void);
 extern int install_thread_keyring_to_cred(struct cred *);
 extern int install_process_keyring_to_cred(struct cred *);
+extern int install_session_keyring_to_cred(struct cred *, struct key *);
 
 extern struct key *request_key_and_link(struct key_type *type,
 					const char *description,
--- mm/security/keys/process_keys.c~1_CONVERT_KEYS	2010-02-25 15:22:14.000000000 +0100
+++ mm/security/keys/process_keys.c	2010-02-26 20:22:14.000000000 +0100
@@ -217,8 +217,7 @@ static int install_process_keyring(void)
 /*
  * install a session keyring directly to a credentials struct
  */
-static int install_session_keyring_to_cred(struct cred *cred,
-					   struct key *keyring)
+int install_session_keyring_to_cred(struct cred *cred, struct key *keyring)
 {
 	unsigned long flags;
 	struct key *old;
--- mm/security/keys/request_key.c~1_CONVERT_KEYS	2010-02-25 17:37:41.000000000 +0100
+++ mm/security/keys/request_key.c	2010-02-26 21:46:23.000000000 +0100
@@ -58,6 +58,38 @@ void complete_request_key(struct key_con
 }
 EXPORT_SYMBOL(complete_request_key);
 
+static int umh_keys_init(struct subprocess_info *info)
+{
+	struct cred *cred = (struct cred*)current_cred();
+	struct key *keyring = info->data;
+	/*
+	 * This is called in context of freshly forked kthread before
+	 * kernel_execve(), we can just change our ->session_keyring.
+	 */
+	return install_session_keyring_to_cred(cred, keyring);
+}
+
+static void umh_keys_cleanup(struct subprocess_info *info)
+{
+	struct key *keyring = info->data;
+	key_put(keyring);
+}
+
+static int call_usermodehelper_keys(char *path, char **argv, char **envp,
+			 struct key *session_keyring, enum umh_wait wait)
+{
+	gfp_t gfp_mask = (wait == UMH_NO_WAIT) ? GFP_ATOMIC : GFP_KERNEL;
+	struct subprocess_info *info =
+		call_usermodehelper_setup(path, argv, envp, gfp_mask);
+
+	if (!info)
+		return -ENOMEM;
+
+	call_usermodehelper_setfns(info, umh_keys_init, umh_keys_cleanup,
+					key_get(session_keyring));
+	return call_usermodehelper_exec(info, wait);
+}
+
 /*
  * request userspace finish the construction of a key
  * - execute "/sbin/request-key <op> <key> <uid> <gid> <keyring> <keyring> <keyring>"


^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH,RESEND -mm 2/2] umh && creds: kill subprocess_info->cred logic
  2010-03-05 22:52           ` Oleg Nesterov
  2010-03-05 23:09             ` [PATCH,RESEND " Oleg Nesterov
@ 2010-03-05 23:10             ` Oleg Nesterov
  2010-03-08 17:47               ` Neil Horman
  2010-03-08 13:19             ` [PATCH,RESEND -mm 1/2] umh && creds: convert call_usermodehelper_keys() to use subprocess_info->init() David Howells
  2010-03-08 13:19             ` [PATCH,RESEND -mm 2/2] umh && creds: kill subprocess_info->cred logic David Howells
  3 siblings, 1 reply; 20+ messages in thread
From: Oleg Nesterov @ 2010-03-05 23:10 UTC (permalink / raw)
  To: Andrew Morton; +Cc: Andi Kleen, Neil Horman, linux-kernel, David Howells

Now that nobody ever changes subprocess_info->cred we can kill this
member and related code. ____call_usermodehelper() always runs in the
context of freshly forked kernel thread, it has the proper ->cred
copied from its parent kthread, keventd.

Signed-off-by: Oleg Nesterov <oleg@redhat.com>
Acked-by: Neil Horman <nhorman@tuxdriver.com>
---

 include/linux/cred.h |    1 
 include/linux/kmod.h |    1 
 kernel/cred.c        |   54 ---------------------------------------------------
 kernel/kmod.c        |   19 -----------------
 4 files changed, 75 deletions(-)

--- mm/include/linux/cred.h~2_KILL_INFO_CRED	2010-02-26 21:45:28.000000000 +0100
+++ mm/include/linux/cred.h	2010-02-26 22:07:38.000000000 +0100
@@ -156,7 +156,6 @@ extern int copy_creds(struct task_struct
 extern struct cred *cred_alloc_blank(void);
 extern struct cred *prepare_creds(void);
 extern struct cred *prepare_exec_creds(void);
-extern struct cred *prepare_usermodehelper_creds(void);
 extern int commit_creds(struct cred *);
 extern void abort_creds(struct cred *);
 extern const struct cred *override_creds(const struct cred *);
--- mm/include/linux/kmod.h~2_KILL_INFO_CRED	2010-02-26 21:45:28.000000000 +0100
+++ mm/include/linux/kmod.h	2010-02-26 22:07:38.000000000 +0100
@@ -55,7 +55,6 @@ enum umh_wait {
 struct subprocess_info {
 	struct work_struct work;
 	struct completion *complete;
-	struct cred *cred;
 	char *path;
 	char **argv;
 	char **envp;
--- mm/kernel/cred.c~2_KILL_INFO_CRED	2010-02-26 21:45:28.000000000 +0100
+++ mm/kernel/cred.c	2010-02-26 22:07:38.000000000 +0100
@@ -347,60 +347,6 @@ struct cred *prepare_exec_creds(void)
 }
 
 /*
- * prepare new credentials for the usermode helper dispatcher
- */
-struct cred *prepare_usermodehelper_creds(void)
-{
-#ifdef CONFIG_KEYS
-	struct thread_group_cred *tgcred = NULL;
-#endif
-	struct cred *new;
-
-#ifdef CONFIG_KEYS
-	tgcred = kzalloc(sizeof(*new->tgcred), GFP_ATOMIC);
-	if (!tgcred)
-		return NULL;
-#endif
-
-	new = kmem_cache_alloc(cred_jar, GFP_ATOMIC);
-	if (!new)
-		return NULL;
-
-	kdebug("prepare_usermodehelper_creds() alloc %p", new);
-
-	memcpy(new, &init_cred, sizeof(struct cred));
-
-	atomic_set(&new->usage, 1);
-	set_cred_subscribers(new, 0);
-	get_group_info(new->group_info);
-	get_uid(new->user);
-
-#ifdef CONFIG_KEYS
-	new->thread_keyring = NULL;
-	new->request_key_auth = NULL;
-	new->jit_keyring = KEY_REQKEY_DEFL_DEFAULT;
-
-	atomic_set(&tgcred->usage, 1);
-	spin_lock_init(&tgcred->lock);
-	new->tgcred = tgcred;
-#endif
-
-#ifdef CONFIG_SECURITY
-	new->security = NULL;
-#endif
-	if (security_prepare_creds(new, &init_cred, GFP_ATOMIC) < 0)
-		goto error;
-	validate_creds(new);
-
-	BUG_ON(atomic_read(&new->usage) != 1);
-	return new;
-
-error:
-	put_cred(new);
-	return NULL;
-}
-
-/*
  * Copy credentials for the new process created by fork()
  *
  * We share if we can, but under some circumstances we have to generate a new
--- mm/kernel/kmod.c~2_KILL_INFO_CRED	2010-02-26 21:45:28.000000000 +0100
+++ mm/kernel/kmod.c	2010-02-26 22:07:38.000000000 +0100
@@ -153,8 +153,6 @@ static int ____call_usermodehelper(void 
 	struct subprocess_info *sub_info = data;
 	int retval;
 
-	BUG_ON(atomic_read(&sub_info->cred->usage) != 1);
-
 	/* Unblock all signals */
 	spin_lock_irq(&current->sighand->siglock);
 	flush_signal_handlers(current, 1);
@@ -162,10 +160,6 @@ static int ____call_usermodehelper(void 
 	recalc_sigpending();
 	spin_unlock_irq(&current->sighand->siglock);
 
-	/* Install the credentials */
-	commit_creds(sub_info->cred);
-	sub_info->cred = NULL;
-
 	/* We can run anywhere, unlike our parent keventd(). */
 	set_cpus_allowed_ptr(current, cpu_all_mask);
 
@@ -193,8 +187,6 @@ void call_usermodehelper_freeinfo(struct
 {
 	if (info->cleanup)
 		(*info->cleanup)(info);
-	if (info->cred)
-		put_cred(info->cred);
 	kfree(info);
 }
 EXPORT_SYMBOL(call_usermodehelper_freeinfo);
@@ -250,8 +242,6 @@ static void __call_usermodehelper(struct
 	pid_t pid;
 	enum umh_wait wait = sub_info->wait;
 
-	BUG_ON(atomic_read(&sub_info->cred->usage) != 1);
-
 	/* CLONE_VFORK: wait until the usermode helper has execve'd
 	 * successfully We need the data structures to stay around
 	 * until that is done.  */
@@ -374,12 +364,6 @@ struct subprocess_info *call_usermodehel
 	sub_info->path = path;
 	sub_info->argv = argv;
 	sub_info->envp = envp;
-	sub_info->cred = prepare_usermodehelper_creds();
-	if (!sub_info->cred) {
-		kfree(sub_info);
-		return NULL;
-	}
-
   out:
 	return sub_info;
 }
@@ -430,9 +414,6 @@ int call_usermodehelper_exec(struct subp
 	DECLARE_COMPLETION_ONSTACK(done);
 	int retval = 0;
 
-	BUG_ON(atomic_read(&sub_info->cred->usage) != 1);
-	validate_creds(sub_info->cred);
-
 	helper_lock();
 	if (sub_info->path[0] == '\0')
 		goto out;


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH,RESEND -mm 1/2] umh && creds: convert call_usermodehelper_keys() to use subprocess_info->init()
  2010-03-05 22:52           ` Oleg Nesterov
  2010-03-05 23:09             ` [PATCH,RESEND " Oleg Nesterov
  2010-03-05 23:10             ` [PATCH,RESEND -mm 2/2] umh && creds: kill subprocess_info->cred logic Oleg Nesterov
@ 2010-03-08 13:19             ` David Howells
  2010-03-08 13:19             ` [PATCH,RESEND -mm 2/2] umh && creds: kill subprocess_info->cred logic David Howells
  3 siblings, 0 replies; 20+ messages in thread
From: David Howells @ 2010-03-08 13:19 UTC (permalink / raw)
  To: Oleg Nesterov
  Cc: dhowells, Andrew Morton, Andi Kleen, Neil Horman, linux-kernel

Oleg Nesterov <oleg@redhat.com> wrote:

> (on top of kmod-replace-call_usermodehelper_pipe-with-use-of-umh-init-function-and-resolve-limit.patch)
> 
> call_usermodehelper_keys() uses call_usermodehelper_setkeys() to change
> subprocess_info->cred in advance. Now that we have info->init() we can
> change this code to set tgcred->session_keyring in context of execing
> kernel thread.
> 
> Note: since currently call_usermodehelper_keys() is never called with
> UMH_NO_WAIT, call_usermodehelper_keys()->key_get() and umh_keys_cleanup()
> are not really needed, we could rely on install_session_keyring_to_cred()
> which does key_get() on success.
> 
> Signed-off-by: Oleg Nesterov <oleg@redhat.com>
> Acked-by: Neil Horman <nhorman@tuxdriver.com>

Acked-by: David Howells <dhowells@redhat.com>

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH,RESEND -mm 2/2] umh && creds: kill subprocess_info->cred logic
  2010-03-05 22:52           ` Oleg Nesterov
                               ` (2 preceding siblings ...)
  2010-03-08 13:19             ` [PATCH,RESEND -mm 1/2] umh && creds: convert call_usermodehelper_keys() to use subprocess_info->init() David Howells
@ 2010-03-08 13:19             ` David Howells
  3 siblings, 0 replies; 20+ messages in thread
From: David Howells @ 2010-03-08 13:19 UTC (permalink / raw)
  To: Oleg Nesterov
  Cc: dhowells, Andrew Morton, Andi Kleen, Neil Horman, linux-kernel

Oleg Nesterov <oleg@redhat.com> wrote:

> Now that nobody ever changes subprocess_info->cred we can kill this
> member and related code. ____call_usermodehelper() always runs in the
> context of freshly forked kernel thread, it has the proper ->cred
> copied from its parent kthread, keventd.
> 
> Signed-off-by: Oleg Nesterov <oleg@redhat.com>
> Acked-by: Neil Horman <nhorman@tuxdriver.com>

Acked-by: David Howells <dhowells@redhat.com>

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH,RESEND -mm 1/2] umh && creds: convert call_usermodehelper_keys() to use subprocess_info->init()
  2010-03-05 23:09             ` [PATCH,RESEND " Oleg Nesterov
@ 2010-03-08 17:44               ` Neil Horman
  0 siblings, 0 replies; 20+ messages in thread
From: Neil Horman @ 2010-03-08 17:44 UTC (permalink / raw)
  To: Oleg Nesterov; +Cc: Andrew Morton, Andi Kleen, linux-kernel, David Howells

On Sat, Mar 06, 2010 at 12:09:57AM +0100, Oleg Nesterov wrote:
> (on top of kmod-replace-call_usermodehelper_pipe-with-use-of-umh-init-function-and-resolve-limit.patch)
> 
> call_usermodehelper_keys() uses call_usermodehelper_setkeys() to change
> subprocess_info->cred in advance. Now that we have info->init() we can
> change this code to set tgcred->session_keyring in context of execing
> kernel thread.
> 
> Note: since currently call_usermodehelper_keys() is never called with
> UMH_NO_WAIT, call_usermodehelper_keys()->key_get() and umh_keys_cleanup()
> are not really needed, we could rely on install_session_keyring_to_cred()
> which does key_get() on success.
> 
> Signed-off-by: Oleg Nesterov <oleg@redhat.com>
> Acked-by: Neil Horman <nhorman@tuxdriver.com>
> ---
> 
Acked-by: Neil Horman <nhorman@tuxdriver.com>

>  include/linux/kmod.h         |   17 -----------------
>  kernel/kmod.c                |   18 ------------------
>  security/keys/internal.h     |    1 +
>  security/keys/process_keys.c |    3 +--
>  security/keys/request_key.c  |   32 ++++++++++++++++++++++++++++++++
>  5 files changed, 34 insertions(+), 37 deletions(-)
> 
> --- mm/include/linux/kmod.h~1_CONVERT_KEYS	2010-02-25 17:37:41.000000000 +0100
> +++ mm/include/linux/kmod.h	2010-02-26 21:45:28.000000000 +0100
> @@ -71,8 +71,6 @@ struct subprocess_info *call_usermodehel
>  						  char **envp, gfp_t gfp_mask);
>  
>  /* Set various pieces of state into the subprocess_info structure */
> -void call_usermodehelper_setkeys(struct subprocess_info *info,
> -				 struct key *session_keyring);
>  void call_usermodehelper_setfns(struct subprocess_info *info,
>  		    int (*init)(struct subprocess_info *info),
>  		    void (*cleanup)(struct subprocess_info *info),
> @@ -108,21 +106,6 @@ call_usermodehelper(char *path, char **a
>  				       wait, NULL, NULL, NULL);
>  }
>  
> -static inline int
> -call_usermodehelper_keys(char *path, char **argv, char **envp,
> -			 struct key *session_keyring, enum umh_wait wait)
> -{
> -	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);
> -	if (info == NULL)
> -		return -ENOMEM;
> -
> -	call_usermodehelper_setkeys(info, session_keyring);
> -	return call_usermodehelper_exec(info, wait);
> -}
> -
>  extern void usermodehelper_init(void);
>  
>  extern int usermodehelper_disable(void);
> --- mm/kernel/kmod.c~1_CONVERT_KEYS	2010-02-25 17:37:41.000000000 +0100
> +++ mm/kernel/kmod.c	2010-02-26 21:45:28.000000000 +0100
> @@ -386,24 +386,6 @@ struct subprocess_info *call_usermodehel
>  EXPORT_SYMBOL(call_usermodehelper_setup);
>  
>  /**
> - * call_usermodehelper_setkeys - set the session keys for usermode helper
> - * @info: a subprocess_info returned by call_usermodehelper_setup
> - * @session_keyring: the session keyring for the process
> - */
> -void call_usermodehelper_setkeys(struct subprocess_info *info,
> -				 struct key *session_keyring)
> -{
> -#ifdef CONFIG_KEYS
> -	struct thread_group_cred *tgcred = info->cred->tgcred;
> -	key_put(tgcred->session_keyring);
> -	tgcred->session_keyring = key_get(session_keyring);
> -#else
> -	BUG();
> -#endif
> -}
> -EXPORT_SYMBOL(call_usermodehelper_setkeys);
> -
> -/**
>   * call_usermodehelper_setfns - set a cleanup/init function
>   * @info: a subprocess_info returned by call_usermodehelper_setup
>   * @cleanup: a cleanup function
> --- mm/security/keys/internal.h~1_CONVERT_KEYS	2010-02-25 15:22:14.000000000 +0100
> +++ mm/security/keys/internal.h	2010-02-26 20:30:52.000000000 +0100
> @@ -115,6 +115,7 @@ extern struct key *find_keyring_by_name(
>  extern int install_user_keyrings(void);
>  extern int install_thread_keyring_to_cred(struct cred *);
>  extern int install_process_keyring_to_cred(struct cred *);
> +extern int install_session_keyring_to_cred(struct cred *, struct key *);
>  
>  extern struct key *request_key_and_link(struct key_type *type,
>  					const char *description,
> --- mm/security/keys/process_keys.c~1_CONVERT_KEYS	2010-02-25 15:22:14.000000000 +0100
> +++ mm/security/keys/process_keys.c	2010-02-26 20:22:14.000000000 +0100
> @@ -217,8 +217,7 @@ static int install_process_keyring(void)
>  /*
>   * install a session keyring directly to a credentials struct
>   */
> -static int install_session_keyring_to_cred(struct cred *cred,
> -					   struct key *keyring)
> +int install_session_keyring_to_cred(struct cred *cred, struct key *keyring)
>  {
>  	unsigned long flags;
>  	struct key *old;
> --- mm/security/keys/request_key.c~1_CONVERT_KEYS	2010-02-25 17:37:41.000000000 +0100
> +++ mm/security/keys/request_key.c	2010-02-26 21:46:23.000000000 +0100
> @@ -58,6 +58,38 @@ void complete_request_key(struct key_con
>  }
>  EXPORT_SYMBOL(complete_request_key);
>  
> +static int umh_keys_init(struct subprocess_info *info)
> +{
> +	struct cred *cred = (struct cred*)current_cred();
> +	struct key *keyring = info->data;
> +	/*
> +	 * This is called in context of freshly forked kthread before
> +	 * kernel_execve(), we can just change our ->session_keyring.
> +	 */
> +	return install_session_keyring_to_cred(cred, keyring);
> +}
> +
> +static void umh_keys_cleanup(struct subprocess_info *info)
> +{
> +	struct key *keyring = info->data;
> +	key_put(keyring);
> +}
> +
> +static int call_usermodehelper_keys(char *path, char **argv, char **envp,
> +			 struct key *session_keyring, enum umh_wait wait)
> +{
> +	gfp_t gfp_mask = (wait == UMH_NO_WAIT) ? GFP_ATOMIC : GFP_KERNEL;
> +	struct subprocess_info *info =
> +		call_usermodehelper_setup(path, argv, envp, gfp_mask);
> +
> +	if (!info)
> +		return -ENOMEM;
> +
> +	call_usermodehelper_setfns(info, umh_keys_init, umh_keys_cleanup,
> +					key_get(session_keyring));
> +	return call_usermodehelper_exec(info, wait);
> +}
> +
>  /*
>   * request userspace finish the construction of a key
>   * - execute "/sbin/request-key <op> <key> <uid> <gid> <keyring> <keyring> <keyring>"
> 
> 

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH,RESEND -mm 2/2] umh && creds: kill subprocess_info->cred logic
  2010-03-05 23:10             ` [PATCH,RESEND -mm 2/2] umh && creds: kill subprocess_info->cred logic Oleg Nesterov
@ 2010-03-08 17:47               ` Neil Horman
  0 siblings, 0 replies; 20+ messages in thread
From: Neil Horman @ 2010-03-08 17:47 UTC (permalink / raw)
  To: Oleg Nesterov; +Cc: Andrew Morton, Andi Kleen, linux-kernel, David Howells

On Sat, Mar 06, 2010 at 12:10:54AM +0100, Oleg Nesterov wrote:
> Now that nobody ever changes subprocess_info->cred we can kill this
> member and related code. ____call_usermodehelper() always runs in the
> context of freshly forked kernel thread, it has the proper ->cred
> copied from its parent kthread, keventd.
> 
> Signed-off-by: Oleg Nesterov <oleg@redhat.com>
> Acked-by: Neil Horman <nhorman@tuxdriver.com>
> ---
> 

Acked-by: Neil Horman <nhorman@tuxdriver.com>

> 

^ permalink raw reply	[flat|nested] 20+ messages in thread

end of thread, other threads:[~2010-03-08 17:47 UTC | newest]

Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-02-25 18:15 [PATCH -mm 1/2] umh && creds: convert call_usermodehelper_keys() to use subprocess_info->init() Oleg Nesterov
2010-02-26 18:00 ` David Howells
2010-02-26 18:23   ` Oleg Nesterov
2010-02-26 18:41   ` David Howells
2010-02-26 18:52     ` Oleg Nesterov
2010-02-26 20:03     ` [PATCH v2 -mm 0/2] umh && creds: kill sub_info->cred Oleg Nesterov
2010-02-26 20:03       ` [PATCH v2 -mm 1/2] umh && creds: convert call_usermodehelper_keys() to use subprocess_info->init() Oleg Nesterov
2010-02-26 20:28         ` Neil Horman
2010-02-26 20:04       ` [PATCH v2 -mm 2/2] umh && creds: kill subprocess_info->cred logic Oleg Nesterov
2010-02-26 20:29         ` Neil Horman
2010-02-26 20:42       ` [PATCH v2 -mm 1/2] umh && creds: convert call_usermodehelper_keys() to use subprocess_info->init() David Howells
2010-02-26 20:53         ` Oleg Nesterov
2010-02-26 23:24         ` David Howells
2010-03-05 22:52           ` Oleg Nesterov
2010-03-05 23:09             ` [PATCH,RESEND " Oleg Nesterov
2010-03-08 17:44               ` Neil Horman
2010-03-05 23:10             ` [PATCH,RESEND -mm 2/2] umh && creds: kill subprocess_info->cred logic Oleg Nesterov
2010-03-08 17:47               ` Neil Horman
2010-03-08 13:19             ` [PATCH,RESEND -mm 1/2] umh && creds: convert call_usermodehelper_keys() to use subprocess_info->init() David Howells
2010-03-08 13:19             ` [PATCH,RESEND -mm 2/2] umh && creds: kill subprocess_info->cred logic David Howells

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.