All of lore.kernel.org
 help / color / mirror / Atom feed
From: Oleg Nesterov <oleg@redhat.com>
To: Andrew Morton <akpm@linux-foundation.org>
Cc: linux-kernel@vger.kernel.org, andi@firstfloor.org,
	David Howells <dhowells@redhat.com>,
	Neil Horman <nhorman@tuxdriver.com>,
	Roland McGrath <roland@redhat.com>
Subject: [PATCH 5/6] call_usermodehelper: simplify/fix UMH_NO_WAIT case
Date: Mon, 15 Mar 2010 20:48:47 +0100	[thread overview]
Message-ID: <20100315194847.GF10896@redhat.com> (raw)
In-Reply-To: <20100315194609.GA10896@redhat.com>

__call_usermodehelper(UMH_NO_WAIT) has 2 problems:

	- if kernel_thread() fails, call_usermodehelper_freeinfo()
	  is not called.

	- for unknown reason UMH_NO_WAIT has UMH_WAIT_PROC logic,
	  we spawn yet another thread which waits until the user
	  mode application exits.

Change the UMH_NO_WAIT code to use ____call_usermodehelper() instead of
wait_for_helper(), and do call_usermodehelper_freeinfo() unconditionally.
We can rely on CLONE_VFORK, do_fork(CLONE_VFORK) until the child exits
or execs.

With or without this patch UMH_NO_WAIT does not report the error if
kernel_thread() fails, this is correct since the caller doesn't wait
for result.

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

 kernel/kmod.c |   10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

--- 34-rc1/kernel/kmod.c~5_UMH_NO_WAIT	2010-03-15 20:28:31.000000000 +0100
+++ 34-rc1/kernel/kmod.c	2010-03-15 20:32:23.000000000 +0100
@@ -205,10 +205,7 @@ static int wait_for_helper(void *data)
 			sub_info->retval = ret;
 	}
 
-	if (sub_info->wait == UMH_NO_WAIT)
-		call_usermodehelper_freeinfo(sub_info);
-	else
-		complete(sub_info->complete);
+	complete(sub_info->complete);
 	return 0;
 }
 
@@ -217,13 +214,13 @@ static void __call_usermodehelper(struct
 {
 	struct subprocess_info *sub_info =
 		container_of(work, struct subprocess_info, work);
-	pid_t pid;
 	enum umh_wait wait = sub_info->wait;
+	pid_t pid;
 
 	/* CLONE_VFORK: wait until the usermode helper has execve'd
 	 * successfully We need the data structures to stay around
 	 * until that is done.  */
-	if (wait == UMH_WAIT_PROC || wait == UMH_NO_WAIT)
+	if (wait == UMH_WAIT_PROC)
 		pid = kernel_thread(wait_for_helper, sub_info,
 				    CLONE_FS | CLONE_FILES | SIGCHLD);
 	else
@@ -232,6 +229,7 @@ static void __call_usermodehelper(struct
 
 	switch (wait) {
 	case UMH_NO_WAIT:
+		call_usermodehelper_freeinfo(sub_info);
 		break;
 
 	case UMH_WAIT_PROC:


  parent reply	other threads:[~2010-03-15 19:50 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-03-15 12:29 [PATCH 0/2] exec: refactor how call_usermodehelper works, and update the sense of the core_pipe recursion check (v4 rediff) Neil Horman
2010-03-15 12:33 ` [PATCH 1/2] kmod: add init function to usermodehelper Neil Horman
2010-03-15 17:34   ` Oleg Nesterov
2010-03-15 17:56     ` Neil Horman
2010-03-15 12:36 ` [PATCH 2/2] exec: replace call_usermodehelper_pipe with use of umh init function and resolve limit Neil Horman
2010-03-15 17:39   ` Oleg Nesterov
2010-03-15 19:46 ` [PATCH 0/6] umh: keys, signals, misc Oleg Nesterov
2010-03-15 19:46   ` [PATCH 1/6] umh: creds: convert call_usermodehelper_keys() to use subprocess_info->init() Oleg Nesterov
2010-03-15 19:47   ` [PATCH 2/6] umh: creds: kill subprocess_info->cred logic Oleg Nesterov
2010-03-15 19:47   ` [PATCH 3/6] call_usermodehelper: no need to unblock signals Oleg Nesterov
2010-03-15 19:48   ` [PATCH 4/6] wait_for_helper: SIGCHLD from user-space can lead to use-after-free Oleg Nesterov
2010-03-15 19:48   ` Oleg Nesterov [this message]
2010-03-15 19:49   ` [PATCH 6/6] call_usermodehelper: UMH_WAIT_EXEC ignores kernel_thread() failure Oleg Nesterov
2010-03-16 19:37   ` [PATCH 0/4] do_coredump: cleanups Oleg Nesterov
2010-03-16 19:38     ` [PATCH 1/4] coredump: factor out the not-ispipe file checks Oleg Nesterov
2010-03-16 19:38     ` [PATCH 2/4] coredump: cleanup "ispipe" code Oleg Nesterov
2010-03-16 19:39     ` [PATCH 3/4] coredump: factor out put_cred() calls Oleg Nesterov
2010-03-16 19:39     ` [PATCH 4/4] coredump: shift down_write(mmap_sem) into coredump_wait() Oleg Nesterov

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20100315194847.GF10896@redhat.com \
    --to=oleg@redhat.com \
    --cc=akpm@linux-foundation.org \
    --cc=andi@firstfloor.org \
    --cc=dhowells@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=nhorman@tuxdriver.com \
    --cc=roland@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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.