From: Oleg Nesterov <oleg@redhat.com>
Date: Thu, 22 Jan 2009 15:21:32 +0100
Subject: [misc] minor signal handling vulnerability
Message-id: 20090122142132.GB16847@redhat.com
O-Subject: Re: [kernel team] [rhel5 security patch] CVE-2009-0028 EMBARGOED Linux kernel minor signal handling vulnerability
Bugzilla: 479964
CVE: CVE-2009-0028
RH-Acked-by: Jon Masters <jcm@redhat.com>
RH-Acked-by: David Howells <dhowells@redhat.com>

 "From Chris Evans:

  It's a relatively minor signal issue where a child can send its parent
  process an arbitrary signal, even if the parent has a totally separate
  real and effective user id. This could be a nuisance in the case where
  long-running root daemons spawn direct child processes owned by
  untrusted users [*]. There may even be worse consequences if privileged
  processes have weak signal handling code for signals not normally
  triggerable by untrusted users."

        clone(.., CLONE_PARENT|<random_signal> ...)

  man clone:
        The low byte of flags contains the number of the termination
        signal sent to the parent when the child dies.

  There are 2 senarios:

        1) After fork(), both parent and child exec().  In this case
        execution domain of both parent and child are incremented, so
        checks in exit_notify() fail to catch this.

        2) Child exec()s a suid image.  Again, checks in exit_notify() are
        circumvented.

Fixes bz479964 / CVE-2009-0028
Tested/Confirmed on x86_64.

>
> 	1) After fork(), both parent and child exec().  In this case
> 	execution domain of both parent and child are incremented, so
> 	checks in exit_notify() fail to catch this.
>
> 	2) Child exec()s a suid image.  Again, checks in exit_notify() are
> 	circumvented.
>

To clarify, this patch of course can't fix case 2) completely, but I don't
see the "real" problems if we fix the CLONE_PARENT case.

Thinking more, I'd suggest the more "conservative" change for rhel,
please see the patch below.

current->group_leader->exit_signal looks more logical, but we have other
problems in this area which should be fixed. In particular, there is a
window between setting p->exit_signal and setting p->real_parent under
tasklist_lock. In this window "current" can be reparented to /sbin/init.

Init is immune to unhandled signals, but it can be confused by this
signal and we can create a dangling zombie.

Oleg.

diff --git a/kernel/fork.c b/kernel/fork.c
index f038aff..0e99e60 100644
--- a/kernel/fork.c
+++ b/kernel/fork.c
@@ -1280,7 +1280,9 @@ static struct task_struct *copy_process(unsigned long clone_flags,
 	p->parent_exec_id = p->self_exec_id;
 
 	/* ok, now we should be set up.. */
-	p->exit_signal = (clone_flags & CLONE_THREAD) ? -1 : (clone_flags & CSIGNAL);
+	p->exit_signal = (clone_flags & CLONE_THREAD) ? -1 :
+			 (clone_flags & CLONE_PARENT) ? SIGCHLD :
+			 (clone_flags & CSIGNAL);
 	p->pdeath_signal = 0;
 	p->exit_state = 0;