Sophie

Sophie

distrib > Scientific%20Linux > 5x > x86_64 > by-pkgid > 27922b4260f65d317aabda37e42bbbff > files > 2102

kernel-2.6.18-238.el5.src.rpm

From: Oleg Nesterov <oleg@redhat.com>
Date: Sun, 30 May 2010 15:33:53 -0400
Subject: [misc] signals: avoid unnecessary credentials check
Message-id: <20100530153353.GA7513@redhat.com>
Patchwork-id: 25902
O-Subject: [RHEL5 PATCH] bz#459901: signals: check_kill_permission(): don't
	check creds if same_thread_group()
Bugzilla: 459901
RH-Acked-by: Roland McGrath <roland@redhat.com>
RH-Acked-by: Jiri Olsa <jolsa@redhat.com>
RH-Acked-by: Ivan Vecera <ivecera@redhat.com>

see https://bugzilla.redhat.com/show_bug.cgi?id=459901

Upstream: trivial backport of 065add3941bdca54fe04ed3471a96bce9af88793

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 credentials. But there is not any
security reason to check ->xuid'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, fix the indentation.

Reported-by: Andrew Tridgell <tridge@samba.org>
Upstream-acked-by: Roland McGrath <roland@redhat.com>
Upstream-acked-by: David Howells <dhowells@redhat.com>
Signed-off-by: Oleg Nesterov <oleg@redhat.com>

diff --git a/kernel/signal.c b/kernel/signal.c
index df009a9..b0261a5 100644
--- a/kernel/signal.c
+++ b/kernel/signal.c
@@ -487,12 +487,13 @@ static int check_kill_permission(int sig, struct siginfo *info,
 		if (error)
 			return error;
 		error = -EPERM;
-		if (((sig != SIGCONT) ||
+		if ((current->tgid != t->tgid)
+		    && ((sig != SIGCONT) ||
 			(current->signal->session != t->signal->session))
 		    && (current->euid ^ t->suid) && (current->euid ^ t->uid)
 		    && (current->uid ^ t->suid) && (current->uid ^ t->uid)
 		    && !capable(CAP_KILL))
-		return error;
+			return error;
 	}
 
 	return security_task_kill(t, info, sig, 0);