From: Oleg Nesterov <oleg@redhat.com>
To: Andrew Morton <akpm@linux-foundation.org>
Cc: Andi Kleen <andi@firstfloor.org>,
David Howells <dhowells@redhat.com>,
Neil Horman <nhorman@tuxdriver.com>,
Roland McGrath <roland@redhat.com>,
Rusty Russell <rusty@rustcorp.com.au>,
linux-kernel@vger.kernel.org
Subject: [PATCH] wait_for_helper: SIGCHLD from user-space can lead to use-after-free
Date: Wed, 10 Mar 2010 18:16:34 +0100 [thread overview]
Message-ID: <20100310171634.GA1039@redhat.com> (raw)
(this patch is orthogonal to other umh changes in -mm)
1. wait_for_helper() does allow_signal(SIGCHLD) to ensure the child can't
autoreap itself.
However, this means that a spurious SIGCHILD from user-space can
set TIF_SIGPENDING and:
- kernel_thread() or sys_wait4() can fail due to signal_pending()
- worse, wait4() can fail before ____call_usermodehelper() execs
or exits. In this case the caller may kfree(subprocess_info)
while the child still uses this memory.
Change the code to use SIG_DFL instead of magic "(void __user *)2"
set by allow_signal(). This means that SIGCHLD won't be delivered,
yet the child won't autoreap itsefl.
The problem is minor, only root can send a signal to this kthread.
2. If sys_wait4(&ret) fails it doesn't populate "ret", in this case
wait_for_helper() reports a random value from uninitialized var.
With this patch sys_wait4() should never fail, but still it makes
sense to initialize ret = -ECHILD so that the caller can notice
the problem.
Signed-off-by: Oleg Nesterov <oleg@redhat.com>
---
kernel/kmod.c | 9 +++++----
1 file changed, 5 insertions(+), 4 deletions(-)
--- mm/kernel/kmod.c~4_SIGCHLD_RACE 2010-03-09 22:23:37.000000000 +0100
+++ mm/kernel/kmod.c 2010-03-10 17:36:37.000000000 +0100
@@ -194,15 +194,16 @@ static int wait_for_helper(void *data)
struct subprocess_info *sub_info = data;
pid_t pid;
- /* Install a handler: if SIGCLD isn't handled sys_wait4 won't
- * populate the status, but will return -ECHILD. */
- allow_signal(SIGCHLD);
+ /* If SIGCLD is ignored sys_wait4 won't populate the status. */
+ spin_lock_irq(¤t->sighand->siglock);
+ current->sighand->action[SIGCHLD-1].sa.sa_handler = SIG_DFL;
+ spin_unlock_irq(¤t->sighand->siglock);
pid = kernel_thread(____call_usermodehelper, sub_info, SIGCHLD);
if (pid < 0) {
sub_info->retval = pid;
} else {
- int ret;
+ int ret = -ECHILD;
/*
* Normally it is bogus to call wait4() from in-kernel because
next reply other threads:[~2010-03-10 17:18 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2010-03-10 17:16 Oleg Nesterov [this message]
2010-03-10 18:07 ` [PATCH] wait_for_helper: SIGCHLD from user-space can lead to use-after-free Neil Horman
2010-03-10 20:32 ` Roland McGrath
2010-03-10 21:19 ` Oleg Nesterov
2010-03-10 21:27 ` Roland McGrath
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=20100310171634.GA1039@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 \
--cc=rusty@rustcorp.com.au \
/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.