[lkml]   [2010]   [Jul]   [1]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
Messages in this thread
Patch in this message
Subject[patch 082/149] signals: check_kill_permission(): dont check creds if same_thread_group()
2.6.32-stable review patch.  If anyone has any objections, please let us know.


From: Oleg Nesterov <>

commit 065add3941bdca54fe04ed3471a96bce9af88793 upstream.

Andrew Tridgell reports that aio_read(SIGEV_SIGNAL) can fail if the
notification from the helper thread races with setresuid(), see

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()

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: Andrew Tridgell <>
Signed-off-by: Oleg Nesterov <>
Acked-by: Roland McGrath <>
Acked-by: David Howells <>
Cc: Eric Paris <>
Cc: Jakub Jelinek <>
Cc: James Morris <>
Cc: Roland McGrath <>
Cc: Stephen Smalley <>
Signed-off-by: Andrew Morton <>
Signed-off-by: Linus Torvalds <>
Signed-off-by: Greg Kroah-Hartman <>

kernel/signal.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)

--- a/kernel/signal.c
+++ b/kernel/signal.c
@@ -591,7 +591,7 @@ static int rm_from_queue(unsigned long m
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;

@@ -605,8 +605,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) &&

 \ /
  Last update: 2010-07-01 20:43    [W:1.064 / U:3.468 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site