Commit 065add39 authored by Oleg Nesterov's avatar Oleg Nesterov Committed by Linus Torvalds

signals: check_kill_permission(): don't check creds if same_thread_group()

Andrew Tridgell reports that aio_read(SIGEV_SIGNAL) can fail if 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 permit sending a
signal to the task with the different cred->xids.  But there is not 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.

Roland said:
: The glibc (libpthread) that does set*id across threads has
: been in use for a while (2.3.4?), probably in distro's using kernels as old
: or older than any active -stable streams.  In the race in question, this
: kernel bug is breaking valid POSIX application expectations.
Reported-by: default avatarAndrew Tridgell <tridge@samba.org>
Signed-off-by: default avatarOleg Nesterov <oleg@redhat.com>
Acked-by: default avatarRoland McGrath <roland@redhat.com>
Acked-by: default avatarDavid Howells <dhowells@redhat.com>
Cc: Eric Paris <eparis@parisplace.org>
Cc: Jakub Jelinek <jakub@redhat.com>
Cc: James Morris <jmorris@namei.org>
Cc: Roland McGrath <roland@redhat.com>
Cc: Stephen Smalley <sds@tycho.nsa.gov>
Cc: <stable@kernel.org>		[all kernel versions]
Signed-off-by: default avatarAndrew Morton <akpm@linux-foundation.org>
Signed-off-by: default avatarLinus Torvalds <torvalds@linux-foundation.org>
parent e0129ef9
...@@ -642,7 +642,7 @@ static inline bool si_fromuser(const struct siginfo *info) ...@@ -642,7 +642,7 @@ static inline bool si_fromuser(const struct siginfo *info)
static int check_kill_permission(int sig, struct siginfo *info, static int check_kill_permission(int sig, struct siginfo *info,
struct task_struct *t) struct task_struct *t)
{ {
const struct cred *cred = current_cred(), *tcred; const struct cred *cred, *tcred;
struct pid *sid; struct pid *sid;
int error; int error;
...@@ -656,8 +656,10 @@ static int check_kill_permission(int sig, struct siginfo *info, ...@@ -656,8 +656,10 @@ static int check_kill_permission(int sig, struct siginfo *info,
if (error) if (error)
return error; return error;
cred = current_cred();
tcred = __task_cred(t); tcred = __task_cred(t);
if ((cred->euid ^ tcred->suid) && if (!same_thread_group(current, t) &&
(cred->euid ^ tcred->suid) &&
(cred->euid ^ tcred->uid) && (cred->euid ^ tcred->uid) &&
(cred->uid ^ tcred->suid) && (cred->uid ^ tcred->suid) &&
(cred->uid ^ tcred->uid) && (cred->uid ^ tcred->uid) &&
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment