signal memory leak

From: Stepan Kasal (kasal@matsrv.math.cas.cz)
Date: Tue Mar 21 2000 - 10:33:14 EST


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(&current->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/



This archive was generated by hypermail 2b29 : Thu Mar 23 2000 - 21:00:33 EST