summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorSebastian Andrzej Siewior <sebastian@breakpoint.cc>2009-09-24 01:02:55 +0200
committerLinus Torvalds <torvalds@linux-foundation.org>2009-09-23 18:12:10 -0700
commit95e0d86badc410d525ea7218fd32df7bfbf9c837 (patch)
tree26764a72a3bc8bfcd2ece90f6faaae928f345066
parent0dd52d0df02733dfc2d5f3824e41b96492305384 (diff)
Revert "kmod: fix race in usermodehelper code"
This reverts commit c02e3f361c7 ("kmod: fix race in usermodehelper code") The patch is wrong. UMH_WAIT_EXEC is called with VFORK what ensures that the child finishes prior returing back to the parent. No race. In fact, the patch makes it even worse because it does the thing it claims not do: - It calls ->complete() on UMH_WAIT_EXEC - the complete() callback may de-allocated subinfo as seen in the following call chain: [<c009f904>] (__link_path_walk+0x20/0xeb4) from [<c00a094c>] (path_walk+0x48/0x94) [<c00a094c>] (path_walk+0x48/0x94) from [<c00a0a34>] (do_path_lookup+0x24/0x4c) [<c00a0a34>] (do_path_lookup+0x24/0x4c) from [<c00a158c>] (do_filp_open+0xa4/0x83c) [<c00a158c>] (do_filp_open+0xa4/0x83c) from [<c009ba90>] (open_exec+0x24/0xe0) [<c009ba90>] (open_exec+0x24/0xe0) from [<c009bfa8>] (do_execve+0x7c/0x2e4) [<c009bfa8>] (do_execve+0x7c/0x2e4) from [<c0026a80>] (kernel_execve+0x34/0x80) [<c0026a80>] (kernel_execve+0x34/0x80) from [<c004b514>] (____call_usermodehelper+0x130/0x148) [<c004b514>] (____call_usermodehelper+0x130/0x148) from [<c0024858>] (kernel_thread_exit+0x0/0x8) and the path pointer was NULL. Good that ARM's kernel_execve() doesn't check the pointer for NULL or else I wouldn't notice it. The only race there might be is with UMH_NO_WAIT but it is too late for me to investigate it now. UMH_WAIT_PROC could probably also use VFORK and we could save one exec. So the only race I see is with UMH_NO_WAIT and recent scheduler changes where the child does not always run first might have trigger here something but as I said, it is late.... Signed-off-by: Sebastian Andrzej Siewior <sebastian@breakpoint.cc> Acked-by: Neil Horman <nhorman@tuxdriver.com> Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
-rw-r--r--kernel/kmod.c13
1 files changed, 5 insertions, 8 deletions
diff --git a/kernel/kmod.c b/kernel/kmod.c
index 689d20f39305..9fcb53a11f87 100644
--- a/kernel/kmod.c
+++ b/kernel/kmod.c
@@ -143,7 +143,6 @@ struct subprocess_info {
static int ____call_usermodehelper(void *data)
{
struct subprocess_info *sub_info = data;
- enum umh_wait wait = sub_info->wait;
int retval;
BUG_ON(atomic_read(&sub_info->cred->usage) != 1);
@@ -185,14 +184,10 @@ static int ____call_usermodehelper(void *data)
*/
set_user_nice(current, 0);
- if (wait == UMH_WAIT_EXEC)
- complete(sub_info->complete);
-
retval = kernel_execve(sub_info->path, sub_info->argv, sub_info->envp);
/* Exec failed? */
- if (wait != UMH_WAIT_EXEC)
- sub_info->retval = retval;
+ sub_info->retval = retval;
do_exit(0);
}
@@ -271,14 +266,16 @@ static void __call_usermodehelper(struct work_struct *work)
switch (wait) {
case UMH_NO_WAIT:
- case UMH_WAIT_EXEC:
break;
case UMH_WAIT_PROC:
if (pid > 0)
break;
sub_info->retval = pid;
- break;
+ /* FALLTHROUGH */
+
+ case UMH_WAIT_EXEC:
+ complete(sub_info->complete);
}
}