Messages in this thread Patch in this message | | | Date | Tue, 21 Mar 2000 16:33:14 +0100 | From | Stepan Kasal <> | Subject | signal memory leak |
| |
Hi, as Alan pointed out in his todo list 2.4-pre, signal code contains memory leak: if you send SIGCONT to a task which has SIGSTOP siginfo struct in it's sigqueue, the siginfo struct is not deleted. (This bug was introduced in the times when we started creating siginfo struct even for non-rt signals.)
The following patch fixes it. It's against 2.3.99-pre2. It also fixes the bug that sys_kill() might return EAGAIN if we are out of queue space, which is not allowed. And I've atached some cosmetic changes.
--- linux/kernel/signal.c.steve Mon Mar 13 17:48:41 2000 +++ linux/kernel/signal.c Wed Mar 15 17:36:43 2000 @@ -89,7 +89,7 @@ * Dequeue a signal and return the element to the caller, which is * expected to free it. * - * All callers of must be holding current->sigmask_lock. + * All callers must be holding current->sigmask_lock. */ int @@ -148,19 +148,19 @@ kmem_cache_free(signal_queue_cachep,q); atomic_dec(&nr_queued_signals); - /* then see if this signal is still pending. */ - q = *pp; - while (q) { - if (q->info.si_signo == sig) { - reset = 0; - break; - } - q = q->next; - } + /* Then see if this signal is still pending. + (Non rt signals may not be queued twice.) + */ + if (sig >= SIGRTMIN) + for (q = *pp; q; q = q->next) + if (q->info.si_signo == sig) { + reset = 0; + break; + } + } else { - /* Ok, it wasn't in the queue. It must have - been sent either by a non-rt mechanism and - we ran out of queue space. So zero out the + /* Ok, it wasn't in the queue. We must have + been out of queue space. So zero out the info. */ info->si_signo = sig; info->si_errno = 0; @@ -169,9 +169,10 @@ info->si_uid = 0; } - if (reset) + if (reset) { sigdelset(¤t->signal, sig); - recalc_sigpending(current); + recalc_sigpending(current); + } /* XXX: Once POSIX.1b timers are in, if si_code == SI_TIMER, we need to xchg out the timer overrun values. */ @@ -195,6 +196,43 @@ } /* + * Remove signal sig from queue and from t->signal. + * Returns 1 if sig was found in t->signal. + * + * All callers must be holding t->sigmask_lock. + */ +static int rm_sig_from_queue(int sig, struct task_struct *t) +{ + struct signal_queue *q, **pp; + + if (sig >= SIGRTMIN) { + printk(KERN_CRIT "SIG: rm_sig_from_queue() doesn't support rt signals\n"); + return 0; + } + + if (!sigismember(&t->signal, sig)) + return 0; + + sigdelset(&t->signal, sig); + + pp = &t->sigqueue; + q = t->sigqueue; + + /* Find the one we're interested in ... + It may appear only once. */ + for ( ; q ; pp = &q->next, q = q->next) + if (q->info.si_signo == sig) + break; + if (q) { + if ((*pp = q->next) == NULL) + t->sigqueue_tail = pp; + kmem_cache_free(signal_queue_cachep,q); + atomic_dec(&nr_queued_signals); + } + return 1; +} + +/* * Determine whether a signal should be posted or not. * * Signals with SIG_IGN can be ignored, except for the @@ -272,18 +310,16 @@ if (t->state == TASK_STOPPED) wake_up_process(t); t->exit_code = 0; - sigdelsetmask(&t->signal, (sigmask(SIGSTOP)|sigmask(SIGTSTP)| - sigmask(SIGTTOU)|sigmask(SIGTTIN))); - /* Inflict this corner case with recalculations, not mainline */ - recalc_sigpending(t); + if (rm_sig_from_queue(SIGSTOP, t) || rm_sig_from_queue(SIGTSTP, t) || + rm_sig_from_queue(SIGTTOU, t) || rm_sig_from_queue(SIGTTIN, t)) + recalc_sigpending(t); break; case SIGSTOP: case SIGTSTP: case SIGTTIN: case SIGTTOU: /* If we're stopping again, cancel SIGCONT */ - sigdelset(&t->signal, SIGCONT); - /* Inflict this corner case with recalculations, not mainline */ - recalc_sigpending(t); + if (rm_sig_from_queue(SIGCONT, t)) + recalc_sigpending(t); break; } @@ -337,8 +373,12 @@ q->info = *info; break; } - } else { - /* Queue overflow, we have to abort. */ + } else if (sig >= SIGRTMIN && info && (unsigned long)info != 1 + && info->si_code < 0) { + /* + * Queue overflow, abort. We may abort if the signal was rt + * and sent by user using something other than kill(). + */ ret = -EAGAIN; goto out; } @@ -405,7 +445,7 @@ } /* - * kill_pg() sends a signal to a process group: this is what the tty + * kill_pg_info() sends a signal to a process group: this is what the tty * control characters do (^C, ^Z etc) */ @@ -436,7 +476,7 @@ } /* - * kill_sl() sends a signal to the session leader: this is used + * kill_sl_info() sends a signal to the session leader: this is used * to send SIGHUP to the controlling process of a terminal when * the connection is lost. */ @@ -483,7 +523,7 @@ } /* - * kill_something() interprets pid in interesting ways just like kill(2). + * kill_something_info() interprets pid in interesting ways just like kill(2). * * POSIX specifies that kill(-1,sig) is unspecified, but what we have * is probably wrong. Should make it like BSD or SYSV. --- linux/arch/i386/kernel/signal.c.steve Mon Feb 28 16:50:08 2000 +++ linux/arch/i386/kernel/signal.c Tue Mar 14 18:24:12 2000 @@ -597,10 +597,6 @@ * Note that 'init' is a special process: it doesn't get signals it doesn't * want to handle. Thus you cannot kill init even with a SIGKILL even by * mistake. - * - * Note that we go through the signals twice: once to check the signals that - * the kernel can handle, and then we build all the user-level signal handling - * stack-frames in one go after that. */ int do_signal(struct pt_regs *regs, sigset_t *oldset) { --- linux/arch/i386/kernel/entry.S.steve Mon Feb 21 19:59:36 2000 +++ linux/arch/i386/kernel/entry.S Sat Mar 11 14:22:42 2000 @@ -174,9 +174,7 @@ jmp ret_from_sys_call - ALIGN - .globl ret_from_fork -ret_from_fork: +ENTRY(ret_from_fork) pushl %ebx call SYMBOL_NAME(schedule_tail) addl $4, %esp @@ -202,10 +200,7 @@ jne tracesys call *SYMBOL_NAME(sys_call_table)(,%eax,4) movl %eax,EAX(%esp) # save the return value - ALIGN - .globl ret_from_sys_call - .globl ret_from_intr -ret_from_sys_call: +ENTRY(ret_from_sys_call) #ifdef __SMP__ movl processor(%ebx),%eax shll $5,%eax @@ -273,8 +268,7 @@ #endif jne handle_softirq - ALIGN -ret_from_intr: +ENTRY(ret_from_intr) GET_CURRENT(%ebx) movl EFLAGS(%esp),%eax # mix EFLAGS and CS movb CS(%esp),%al - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.rutgers.edu Please read the FAQ at http://www.tux.org/lkml/
| |