All of lore.kernel.org
 help / color / mirror / Atom feed
From: Oleg Nesterov <oleg@redhat.com>
To: Andrew Morton <akpm@linux-foundation.org>,
	Martin Schwidefsky <schwidefsky@de.ibm.com>
Cc: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>,
	linux-kernel@vger.kernel.org, torvalds@linux-foundation.org
Subject: [PATCH 2/2] usermodehelper: kill the kmod_thread_locker logic
Date: Fri, 17 Oct 2014 21:16:43 +0200	[thread overview]
Message-ID: <20141017191643.GC22270@redhat.com> (raw)
In-Reply-To: <20141017191557.GA22270@redhat.com>

Now that we do not call kernel_thread(CLONE_VFORK) from the worker
thread we can not deadlock if do_execve() in turn triggers another
call_usermodehelper(), we can remove the kmod_thread_locker code.

Note: we should probably kill khelper_wq and simply use one of the
global workqueues, say, system_unbound_wq, this special wq for umh
buys nothing nowadays.

Signed-off-by: Oleg Nesterov <oleg@redhat.com>
---
 kernel/kmod.c |   33 +++------------------------------
 1 files changed, 3 insertions(+), 30 deletions(-)

diff --git a/kernel/kmod.c b/kernel/kmod.c
index 4621771..2777f40 100644
--- a/kernel/kmod.c
+++ b/kernel/kmod.c
@@ -47,13 +47,6 @@ extern int max_threads;
 
 static struct workqueue_struct *khelper_wq;
 
-/*
- * kmod_thread_locker is used for deadlock avoidance.  There is no explicit
- * locking to protect this global - it is private to the singleton khelper
- * thread and should only ever be modified by that thread.
- */
-static const struct task_struct *kmod_thread_locker;
-
 #define CAP_BSET	(void *)1
 #define CAP_PI		(void *)2
 
@@ -273,13 +266,6 @@ out:
 	do_exit(0);
 }
 
-static int call_helper(void *data)
-{
-	/* Worker thread started blocking khelper thread. */
-	kmod_thread_locker = current;
-	return ____call_usermodehelper(data);
-}
-
 /* Keventd can't block, but this (a child) can. */
 static int wait_for_helper(void *data)
 {
@@ -327,11 +313,9 @@ static void __call_usermodehelper(struct work_struct *work)
 	if (sub_info->wait & UMH_WAIT_PROC)
 		pid = kernel_thread(wait_for_helper, sub_info,
 				    CLONE_FS | CLONE_FILES | SIGCHLD);
-	else {
-		pid = kernel_thread(call_helper, sub_info, SIGCHLD);
-		/* Worker thread stopped blocking khelper thread. */
-		kmod_thread_locker = NULL;
-	}
+	else
+		pid = kernel_thread(____call_usermodehelper, sub_info,
+				    SIGCHLD);
 
 	if (pid < 0) {
 		sub_info->retval = pid;
@@ -565,17 +549,6 @@ int call_usermodehelper_exec(struct subprocess_info *sub_info, int wait)
 		goto out;
 	}
 	/*
-	 * Worker thread must not wait for khelper thread at below
-	 * wait_for_completion() if the thread was created with CLONE_VFORK
-	 * flag, for khelper thread is already waiting for the thread at
-	 * wait_for_completion() in do_fork().
-	 */
-	if (wait != UMH_NO_WAIT && current == kmod_thread_locker) {
-		retval = -EBUSY;
-		goto out;
-	}
-
-	/*
 	 * Set the completion pointer only if there is a waiter.
 	 * This makes it possible to use umh_complete to free
 	 * the data structure in case of UMH_NO_WAIT.
-- 
1.5.5.1



  parent reply	other threads:[~2014-10-17 19:19 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-10-16 14:00 [PATCH] kernel/kmod: fix use-after-free of the sub_info structure Martin Schwidefsky
2014-10-16 16:57 ` Tetsuo Handa
2014-10-16 17:42   ` Oleg Nesterov
2014-10-16 21:30     ` [PATCH] kernel/kmod: fix use-after-free of the sub_infostructure Tetsuo Handa
2014-10-16 21:58       ` Oleg Nesterov
2014-10-17  7:04         ` Martin Schwidefsky
2014-10-17  7:36         ` Martin Schwidefsky
2014-10-17 12:55           ` [PATCH] kernel/kmod: fix use-after-free of the sub_info structure Tetsuo Handa
2014-10-17 15:21           ` [PATCH] kernel/kmod: fix use-after-free of the sub_infostructure Oleg Nesterov
2014-10-17 19:15           ` [PATCH 0/2] (Was: kernel/kmod: fix use-after-free of the sub_infostructure) Oleg Nesterov
2014-10-17 19:16             ` [PATCH 1/2] usermodehelper: don't use CLONE_VFORK for ____call_usermodehelper() Oleg Nesterov
2014-10-17 19:16             ` Oleg Nesterov [this message]
2014-10-17 23:54             ` [PATCH 0/2] (Was: kernel/kmod: fix use-after-free of thesub_infostructure) Tetsuo Handa
2014-10-17  7:02       ` [PATCH] kernel/kmod: fix use-after-free of the sub_infostructure Martin Schwidefsky
2014-10-16 17:37 ` [PATCH] kernel/kmod: fix use-after-free of the sub_info structure Oleg Nesterov
2014-10-16 20:16   ` 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=20141017191643.GC22270@redhat.com \
    --to=oleg@redhat.com \
    --cc=akpm@linux-foundation.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=penguin-kernel@I-love.SAKURA.ne.jp \
    --cc=schwidefsky@de.ibm.com \
    --cc=torvalds@linux-foundation.org \
    /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.