From: Oleg Nesterov <oleg@redhat.com>
To: Andrew Morton <akpm@linux-foundation.org>
Cc: Andrew Tridgell <tridge@samba.org>,
David Howells <dhowells@redhat.com>,
Eric Paris <eparis@parisplace.org>,
Jakub Jelinek <jakub@redhat.com>,
James Morris <jmorris@namei.org>,
Roland McGrath <roland@redhat.com>,
Stephen Smalley <sds@tycho.nsa.gov>,
linux-kernel@vger.kernel.org
Subject: [PATCH] signals: check_kill_permission: don't check creds if same_thread_group()
Date: Mon, 17 May 2010 21:54:14 +0200 [thread overview]
Message-ID: <20100517195414.GA21504@redhat.com> (raw)
Andrew Tridgell reports that aio_read(SIGEV_SIGNAL) can fail if the
the notification from the helper thread races with setresuid(), see
http://samba.org/~tridge/junkcode/aio_uid.c
This happens because check_kill_permission() doesn't allow to send
a signal to the task with the different cred->xids. But there is no
any security reason to check ->cred's when the task sends a signal
(private or group-wide) to its sub-thread. Whatever we do, any thread
can bypass all security checks and send SIGKILL to all threads, or
it can block a signal SIG and do kill(gettid(), SIG) to deliver this
signal to another sub-thread. Not to mention that CLONE_THREAD implies
CLONE_VM.
Change check_kill_permission() to avoid the credentials check when
the sender and the target are from the same thread group.
Also, move "cred = current_cred()" down to avoid calling get_current()
twice.
Note: David Howells pointed out we could relax this even more, the
CLONE_SIGHAND (without CLONE_THREAD) case probably does not need
these checks too.
Reported-by: Andrew Tridgell <tridge@samba.org>
Signed-off-by: Oleg Nesterov <oleg@redhat.com>
---
kernel/signal.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
--- 34-rc1/kernel/signal.c~CKP_CK_STG 2010-05-09 21:09:31.000000000 +0200
+++ 34-rc1/kernel/signal.c 2010-05-17 17:02:09.000000000 +0200
@@ -642,7 +642,7 @@ static inline bool si_fromuser(const str
static int check_kill_permission(int sig, struct siginfo *info,
struct task_struct *t)
{
- const struct cred *cred = current_cred(), *tcred;
+ const struct cred *cred, *tcred;
struct pid *sid;
int error;
@@ -656,8 +656,10 @@ static int check_kill_permission(int sig
if (error)
return error;
+ cred = current_cred();
tcred = __task_cred(t);
- if ((cred->euid ^ tcred->suid) &&
+ if (!same_thread_group(current, t) &&
+ (cred->euid ^ tcred->suid) &&
(cred->euid ^ tcred->uid) &&
(cred->uid ^ tcred->suid) &&
(cred->uid ^ tcred->uid) &&
next reply other threads:[~2010-05-17 19:57 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2010-05-17 19:54 Oleg Nesterov [this message]
2010-05-18 1:25 ` [PATCH] signals: check_kill_permission: don't check creds if same_thread_group() Roland McGrath
2010-05-18 8:55 ` David Howells
2010-05-18 13:39 ` Oleg Nesterov
2010-05-18 13:50 ` David Howells
2010-05-18 14:08 ` Oleg Nesterov
2010-05-20 19:42 ` Andrew Morton
2010-05-20 20:02 ` 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=20100517195414.GA21504@redhat.com \
--to=oleg@redhat.com \
--cc=akpm@linux-foundation.org \
--cc=dhowells@redhat.com \
--cc=eparis@parisplace.org \
--cc=jakub@redhat.com \
--cc=jmorris@namei.org \
--cc=linux-kernel@vger.kernel.org \
--cc=roland@redhat.com \
--cc=sds@tycho.nsa.gov \
--cc=tridge@samba.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.