kernel/kmod.c: fix a race condition in usermodehelper.

There is a race between call_usermodehelper_keys, __call_usermodehelper
and wait_for_helper. It should only happen if preemption is enabled or
on a virtualized system.

If the cpu is preempted or put to sleep by the hypervisor in
__call_usermodehelper between the creation of the wait_for_helper
thread and the second check on sub_info->wait, the whole execution
of wait_for_helper including the complete call and the continuation
after the wait_for_completion in call_usermodehelper_keys can have
happened before __call_usermodehelper checks sub_info->wait for the
second time. Since sub_info can already have been clobbered,
sub_info->wait could be zero and complete is called a second time
with an invalid argument. This has happened on s390. It took me only
three days to find out ..

Thanks to Arnd Bergmann for his help to spot this bug.

Kenneth Lee also sent the same patch independently.

Signed-off-by: Martin Schwidefsky <schwidefsky@de.ibm.com>
Signed-off-by: Adrian Bunk <bunk@stusta.de>

diff --git a/kernel/kmod.c b/kernel/kmod.c
index 51a892063aaaa2f5b1dfa1cf9f6667dcef88cd68..5a5ec776033627c50b9f43fe3de0733ef517da0a 100644 (file)
--- a/kernel/kmod.c
+++ b/kernel/kmod.c
@@ -197,12 +197,13 @@ static int wait_for_helper(void *data)
 static void __call_usermodehelper(void *data)
 {
        struct subprocess_info *sub_info = data;
+       int 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 (sub_info->wait)
+       if (wait)
                pid = kernel_thread(wait_for_helper, sub_info,
                                    CLONE_FS | CLONE_FILES | SIGCHLD);
        else
@@ -212,7 +213,7 @@ static void __call_usermodehelper(void *data)
        if (pid < 0) {
                sub_info->retval = pid;
                complete(sub_info->complete);
-       } else if (!sub_info->wait)
+       } else if (!wait)
                complete(sub_info->complete);
 }