Originally Published: Tuesday, 14 September 1999 Author: Quentin Cregan
Published to: news_enhance_security/Security News Page: 1/1 - [Printable]

Solar Designer releases a patch against 2.2.12 for exit_signal problems

In a posting to the LSAP list today, a patch for exit_signal was released... "After some more thinking, I think I came up with a solution for the exit_signal problem. (The problem is that users could send arbitrary signals to SUID programs, even if those programs did a setuid() to switch to the new UID completely.)"

   Page 1 of 1  

Hi,

After some more thinking, I think I came up with a solution for the exit_signal problem. (The problem is that users could send arbitrary signals to SUID programs, even if those programs did a setuid() to switch to the new UID completely.)

I'm including a patch, below. It would be nice if someone could review it to make sure I understand the required locking and such correctly (read comments in the patch).

I've declared the counters as "long long" for correctness (so that they "can't" overflow), but in reality, even if an overflow was possible, the privileged program itself should have done stupid things for it to happen. However, there's nothing that says those can have a security impact. Another approach to this, which will be slightly faster on 32-bit systems, is to declare the counters "int", but check for the overflow in execve(2) and scan the entire task list to reset ppriv fields of our children to zero, and set our new priv to 1. This is faster, as the overflow is guaranteed to be very rare.

diff -urPX nopatch linux-2.2.12/fs/exec.c linux/fs/exec.c --- linux-2.2.12/fs/exec.c Tue Aug 31 04:05:31 1999 +++ linux/fs/exec.c Tue Sep 14 12:43:28 1999 @@ -658,6 +752,15 @@ if (cap_raised && !capable(CAP_SETPCAP)) return -EPERM; } +/* + * Increment the privileged execution counter, so that our old children + * know not to send bad exit_signal's to us. Also, wait on the lock if + * there's an exit_signal being sent to us now, to make sure it doesn't + * get sent to the new privileged program. + */ + spin_lock(¤t->priv_lock); + current->priv++; + spin_unlock(¤t->priv_lock); }

memset(bprm->buf,0,sizeof(bprm->buf)); diff -urPX nopatch linux-2.2.12/include/linux/sched.h linux/include/linux/sched.h --- linux-2.2.12/include/linux/sched.h Tue Aug 31 04:05:32 1999 +++ linux/include/linux/sched.h Tue Sep 14 12:45:37 1999 @@ -328,6 +332,11 @@ struct signal_queue *sigqueue, **sigqueue_tail; unsigned long sas_ss_sp; size_t sas_ss_size; + +/* Privileged execution counters, for exit_signal permission checking */ + spinlock_t priv_lock; + volatile long long priv; + long long ppriv; };

/* @@ -394,6 +405,7 @@ /* files */ &init_files, \ /* mm */ &init_mm, \ /* signals */ SPIN_LOCK_UNLOCKED, &init_signals, {{0}}, {{0}}, NULL, &init_task.sigqueue, 0, 0, \ +/* priv */ SPIN_LOCK_UNLOCKED, 0, 0, \ }

union task_union { diff -urPX nopatch linux-2.2.12/kernel/fork.c linux/kernel/fork.c --- linux-2.2.12/kernel/fork.c Tue Aug 31 04:05:32 1999 +++ linux/kernel/fork.c Tue Sep 14 12:17:11 1999 @@ -634,6 +644,10 @@ sigemptyset(&p->signal); p->sigqueue = NULL; p->sigqueue_tail = &p->sigqueue; + + spin_lock_init(&p->priv_lock); + p->priv = 0; + p->ppriv = current->priv;

p->it_real_value = p->it_virt_value = p->it_prof_value = 0; p->it_real_incr = p->it_virt_incr = p->it_prof_incr = 0; diff -urPX nopatch linux-2.2.12/kernel/signal.c linux/kernel/signal.c --- linux-2.2.12/kernel/signal.c Mon Aug 9 23:04:41 1999 +++ linux/kernel/signal.c Tue Sep 14 12:49:28 1999 @@ -579,6 +579,23 @@ { struct siginfo info; int why; + int locked = 0; + + if (sig && sig != SIGCHLD) { +/* + * We do some locking here to ensure that there's no race between the + * check and actually sending the signal. Currently, this is probably + * redundant as notify_parent() is only used either with the big lock + * obtained, or with the signal set to SIGCHLD. + */ + locked = 1; + spin_lock(&tsk->p_pptr->priv_lock); + if (tsk->p_pptr->priv != tsk->ppriv) { + spin_unlock(&tsk->p_pptr->priv_lock); + locked = 0; + sig = 0; + } + }

info.si_signo = sig; info.si_errno = 0; @@ -611,6 +628,8 @@ info.si_code = why;

send_sig_info(sig, &info, tsk->p_pptr); + if (locked) spin_unlock(&tsk->p_pptr->priv_lock); + wake_up_interruptible(&tsk->p_pptr->wait_chldexit); }

Signed, Solar Designer





   Page 1 of 1