Subject: more siginfo q;s
To: None <tech-kern@netbsd.org>
From: Christos Zoulas <christos@zoulas.com>
List: tech-kern
Date: 09/14/2003 01:23:57
In order to do non-trapsignal siginfo delivery we need to store the siginfo
state somewhere, so that when the signal finally gets delivered we can restore
it from there. To do this I put a tiny hashtable in sigctx. Is that overkill?
Also we need to allocate memory from an interrupt context sometime, so the
ksiginfo pool was made to use kmem. Should there be a limit on the number
of ksiginfo's allocated in the pool for a process to avoid denial of service?

Here's the patch...

Index: kern/core_elf32.c
===================================================================
RCS file: /cvsroot/src/sys/kern/core_elf32.c,v
retrieving revision 1.11
diff -u -u -r1.11 core_elf32.c
--- kern/core_elf32.c	2003/09/06 22:03:09	1.11
+++ kern/core_elf32.c	2003/09/14 05:13:24
@@ -303,8 +303,8 @@
 	if (offset) {
 		cpi.cpi_version = NETBSD_ELFCORE_PROCINFO_VERSION;
 		cpi.cpi_cpisize = sizeof(cpi);
-		cpi.cpi_signo = p->p_sigctx.ps_siginfo.ksi_signo;
-		cpi.cpi_sigcode = p->p_sigctx.ps_siginfo.ksi_trap;
+		cpi.cpi_signo = p->p_sigctx.ps_signo;
+		cpi.cpi_sigcode = p->p_sigctx.ps_code;
 		cpi.cpi_siglwp = p->p_sigctx.ps_lwp;
 
 		memcpy(&cpi.cpi_sigpend, &p->p_sigctx.ps_siglist,
Index: kern/core_netbsd.c
===================================================================
RCS file: /cvsroot/src/sys/kern/core_netbsd.c,v
retrieving revision 1.6
diff -u -u -r1.6 core_netbsd.c
--- kern/core_netbsd.c	2003/09/06 22:03:09	1.6
+++ kern/core_netbsd.c	2003/09/14 05:13:24
@@ -82,8 +82,8 @@
 	cs.core.c_midmag = 0;
 	strncpy(cs.core.c_name, p->p_comm, MAXCOMLEN);
 	cs.core.c_nseg = 0;
-	cs.core.c_signo = p->p_sigctx.ps_siginfo.ksi_signo;
-	cs.core.c_ucode = p->p_sigctx.ps_siginfo.ksi_trap;
+	cs.core.c_signo = p->p_sigctx.ps_signo;
+	cs.core.c_ucode = p->p_sigctx.ps_code;
 	cs.core.c_cpusize = 0;
 	cs.core.c_tsize = (u_long)ctob(vm->vm_tsize);
 	cs.core.c_dsize = (u_long)ctob(vm->vm_dsize);
Index: kern/kern_sig.c
===================================================================
RCS file: /cvsroot/src/sys/kern/kern_sig.c,v
retrieving revision 1.151
diff -u -u -r1.151 kern_sig.c
--- kern/kern_sig.c	2003/09/13 15:32:41	1.151
+++ kern/kern_sig.c	2003/09/14 05:13:25
@@ -82,10 +82,17 @@
 static void	child_psignal(struct proc *, int);
 static void	proc_stop(struct proc *);
 static int	build_corename(struct proc *, char [MAXPATHLEN]);
+static void	ksiginfo_exithook(struct proc *, void *);
+static void	ksiginfo_save(struct proc *, ksiginfo_t *);
+static void	ksiginfo_del(struct proc *p, ksiginfo_t *);
+static void	ksiginfo_put(struct proc *, ksiginfo_t *);
+static ksiginfo_t *ksiginfo_get(struct proc *, int);
+
 sigset_t	contsigmask, stopsigmask, sigcantmask;
 
 struct pool	sigacts_pool;	/* memory pool for sigacts structures */
 struct pool	siginfo_pool;	/* memory pool for siginfo structures */
+struct pool	ksiginfo_pool;	/* memory pool for ksiginfo structures */
 
 /*
  * Can process p, with pcred pc, send the signal signum to process q?
@@ -99,16 +106,111 @@
 	    ((signum) == SIGCONT && (q)->p_session == (p)->p_session))
 
 /*
+ * return the first ksiginfo struct from our hash table
+ */
+static ksiginfo_t *
+ksiginfo_get(struct proc *p, int signo)
+{
+	ksiginfo_t *ksi, **hp = p->p_sigctx.ps_siginfo;
+	size_t id = signo % SIGINFO_BUCKETS;
+
+	if ((ksi = hp[id]) == NULL)
+		return NULL;
+
+	for (;;) {
+		if (ksi->ksi_signo == signo)
+			return ksi;
+		if ((ksi = ksi->ksi_next) == hp[id])
+			return NULL;
+	}
+}
+
+static void
+ksiginfo_put(struct proc *p, ksiginfo_t *ksi)
+{
+	ksiginfo_t **hp = p->p_sigctx.ps_siginfo;
+	size_t id = ksi->ksi_signo % SIGINFO_BUCKETS;
+
+	if (hp[id] == NULL)
+		hp[id] = ksi->ksi_next = ksi->ksi_prev = ksi;
+	else {
+		ksi->ksi_prev = hp[id]->ksi_prev;
+		hp[id]->ksi_prev->ksi_next = ksi;
+		hp[id]->ksi_prev = ksi;
+		ksi->ksi_next = hp[id];
+	}
+}
+
+static void
+ksiginfo_del(struct proc *p, ksiginfo_t *ksi)
+{
+	ksiginfo_t **hp = p->p_sigctx.ps_siginfo;
+	size_t id = ksi->ksi_signo % SIGINFO_BUCKETS;
+
+	if (ksi->ksi_next == ksi)
+		hp[id] = NULL;
+	else {
+		ksi->ksi_prev->ksi_next = ksi->ksi_next;
+		ksi->ksi_next->ksi_prev = ksi->ksi_prev;
+	}
+}
+
+static void
+ksiginfo_save(struct proc *p, ksiginfo_t *ksi)
+{
+	ksiginfo_t *kpool = NULL;
+	if ((SIGACTION_PS(p->p_sigacts, ksi->ksi_signo).sa_flags & SA_SIGINFO)
+	    == 0)
+		return;
+	if (
+#ifdef notyet	/* XXX: QUEUING */
+	    ksi->ksi_signo >= SIGRTMIN ||
+#endif
+	    (kpool = ksiginfo_get(p, ksi->ksi_signo)) == NULL) {
+		if ((kpool = pool_get(&ksiginfo_pool, PR_NOWAIT)) == NULL)
+			return;
+		*kpool = *ksi;
+		ksiginfo_put(p, kpool);
+	} else
+		*kpool = *ksi;
+}
+
+/*
+ * free all pending ksiginfo on exit
+ */
+static void
+ksiginfo_exithook(struct proc *p, void *v)
+{
+	size_t i;
+	ksiginfo_t **hp = p->p_sigctx.ps_siginfo;
+
+	for (i = 0; i < SIGINFO_BUCKETS; i++) {
+		ksiginfo_t *ksi;
+		if ((ksi = hp[i]) == NULL)
+			continue;
+		for (;;) {
+			pool_put(&ksiginfo_pool, ksi);
+			if ((ksi = ksi->ksi_next) == hp[i])
+				break;
+		}
+		hp[i] = NULL;
+	}
+}
+
+/*
  * Initialize signal-related data structures.
  */
 void
 signal_init(void)
 {
-
 	pool_init(&sigacts_pool, sizeof(struct sigacts), 0, 0, 0, "sigapl",
 	    &pool_allocator_nointr);
 	pool_init(&siginfo_pool, sizeof(siginfo_t), 0, 0, 0, "siginfo",
 	    &pool_allocator_nointr);
+	pool_init(&ksiginfo_pool, sizeof(ksiginfo_t), 0, 0, 0, "ksiginfo",
+	    NULL);
+        exithook_establish(ksiginfo_exithook, NULL);
+	exechook_establish(ksiginfo_exithook, NULL);
 }
 
 /*
@@ -812,9 +914,10 @@
 		}
 		(void) spl0();		/* XXXSMP */
 	} else {
-		/* XXX for core dump/debugger */
-		p->p_sigctx.ps_siginfo = *ksi;
 		p->p_sigctx.ps_lwp = l->l_lid;
+		/* XXX for core dump/debugger */
+		p->p_sigctx.ps_signo = ksi->ksi_signo;
+		p->p_sigctx.ps_code = ksi->ksi_trap;
 		kpsignal(p, ksi, NULL);
 	}
 }
@@ -961,11 +1064,12 @@
 	if ((prop & SA_CANTMASK) == 0
 	    && p->p_sigctx.ps_sigwaited < 0
 	    && sigismember(&p->p_sigctx.ps_sigwait, signum)
-	    &&  p->p_stat != SSTOP) {
+	    && p->p_stat != SSTOP) {
+		if (action == SIG_CATCH)
+			ksiginfo_save(p, ksi);
 		sigdelset(&p->p_sigctx.ps_siglist, signum);
 		p->p_sigctx.ps_sigwaited = signum;
 		sigemptyset(&p->p_sigctx.ps_sigwait);
-
 		if (dolock)
 			wakeup_one(&p->p_sigctx.ps_sigwait);
 		else
@@ -977,8 +1081,11 @@
 	 * Defer further processing for signals which are held,
 	 * except that stopped processes must be continued by SIGCONT.
 	 */
-	if (action == SIG_HOLD && ((prop & SA_CONT) == 0 || p->p_stat != SSTOP))
+	if (action == SIG_HOLD &&
+	    ((prop & SA_CONT) == 0 || p->p_stat != SSTOP)) {
+		ksiginfo_save(p, ksi);
 		return;
+	}
 	/* XXXSMP: works, but icky */
 	if (dolock)
 		SCHED_LOCK(s);
@@ -1040,7 +1147,7 @@
 			 */
 			if ((prop & SA_CONT) && action == SIG_DFL) {
 				sigdelset(&p->p_sigctx.ps_siglist, signum);
-				goto out;
+				goto done;
 			}
 
 			/*
@@ -1052,8 +1159,9 @@
 				 * If a child holding parent blocked,
 				 * stopping could cause deadlock.
 				 */
-				if (p->p_flag & P_PPWAIT)
+				if (p->p_flag & P_PPWAIT) {
 					goto out;
+				}
 				sigdelset(&p->p_sigctx.ps_siglist, signum);
 				p->p_xstat = signum;
 				if ((p->p_pptr->p_flag & P_NOCLDSTOP) == 0) {
@@ -1064,7 +1172,7 @@
 					child_psignal(p, 0);
 				}
 				proc_stop(p);	/* XXXSMP: recurse? */
-				goto out;
+				goto done;
 			}
 
 			if (l == NULL) {
@@ -1077,7 +1185,7 @@
 				 */
 				if (allsusp && (signum == SIGKILL))
 					lwp_continue(suspended);
-				goto out;
+				goto done;
 			}
 			/*
 			 * All other (caught or default) signals
@@ -1092,7 +1200,7 @@
 			 * then no further action is necessary.
 			 */
 			if (p->p_flag & P_TRACED)
-				goto out;
+				goto done;
 
 			/*
 			 * Kill signal always sets processes running,
@@ -1102,7 +1210,7 @@
 				l = proc_unstop(p);
 				if (l)
 					goto runfast;
-				goto out;
+				goto done;
 			}
 			
 			if (prop & SA_CONT) {
@@ -1135,7 +1243,7 @@
 				 * (If we did the shell could get confused.)
 				 */
 				sigdelset(&p->p_sigctx.ps_siglist, signum);
-				goto out;
+				goto done;
 			}
 
 			/*
@@ -1158,15 +1266,26 @@
 	/*NOTREACHED*/
 
  runfast:
+	if (action == SIG_CATCH) {
+		ksiginfo_save(p, ksi);
+		action = SIG_HOLD;
+	}
 	/*
 	 * Raise priority to at least PUSER.
 	 */
 	if (l->l_priority > PUSER)
 		l->l_priority = PUSER;
  run:
+	if (action == SIG_CATCH) {
+		ksiginfo_save(p, ksi);
+		action = SIG_HOLD;
+	}
 	
 	setrunnable(l);		/* XXXSMP: recurse? */
  out:
+	if (action == SIG_CATCH)
+		ksiginfo_save(p, ksi);
+ done:
 	/* XXXSMP: works, but icky */
 	if (dolock)
 		SCHED_UNLOCK(s);
@@ -1607,7 +1726,7 @@
 		sigexit(l, signum);
 		/* NOTREACHED */
 	} else {
-		ksiginfo_t ksi;
+		ksiginfo_t *ksi;
 		/*
 		 * If we get here, the signal must be caught.
 		 */
@@ -1631,16 +1750,25 @@
 		} else
 			returnmask = &p->p_sigctx.ps_sigmask;
 		p->p_stats->p_ru.ru_nsignals++;
-		if (p->p_sigctx.ps_siginfo.ksi_signo != signum) {
-			memset(&ksi, 0, sizeof(ksi));
-			ksi.ksi_signo = signum;
+		ksi = ksiginfo_get(p, signum);
+		if (ksi == NULL) {
+			ksiginfo_t ksi1;
+			/*
+			 * we did not save any siginfo for this, either
+			 * because the signal was not caught, or because the
+			 * user did not request SA_SIGINFO
+			 */
+			(void)memset(&ksi1, 0, sizeof(ksi1));
+			ksi1.ksi_signo = signum;
+			kpsendsig(l, &ksi1, returnmask);
 		} else {
-			ksi = p->p_sigctx.ps_siginfo;
-			memset(&p->p_sigctx.ps_siginfo, 0,
-			    sizeof(p->p_sigctx.ps_siginfo));
-			p->p_sigctx.ps_lwp = 0;
-		}
-		kpsendsig(l, &ksi, returnmask);
+			kpsendsig(l, ksi, returnmask);
+			ksiginfo_del(p, ksi);
+			pool_put(&ksiginfo_pool, ksi);
+		}
+		p->p_sigctx.ps_lwp = 0;
+		p->p_sigctx.ps_code = 0;
+		p->p_sigctx.ps_signo = 0;
 		(void) splsched();	/* XXXSMP */
 		sigplusset(&SIGACTION_PS(ps, signum).sa_mask,
 		    &p->p_sigctx.ps_sigmask);
@@ -1749,7 +1877,7 @@
 	exitsig = signum;
 	p->p_acflag |= AXSIG;
 	if (sigprop[signum] & SA_CORE) {
-		p->p_sigctx.ps_siginfo.ksi_signo = signum;
+		p->p_sigctx.ps_signo = signum;
 		if ((error = coredump(l)) == 0)
 			exitsig |= WCOREFLAG;
 
Index: sys/siginfo.h
===================================================================
RCS file: /cvsroot/src/sys/sys/siginfo.h,v
retrieving revision 1.2
diff -u -u -r1.2 siginfo.h
--- sys/siginfo.h	2003/09/06 22:01:20	1.2
+++ sys/siginfo.h	2003/09/14 05:13:25
@@ -79,6 +79,8 @@
 			int	_fd;
 		} _poll;
 	} _reason;
+	struct ksiginfo *ksi_next;
+	struct ksiginfo *ksi_prev;
 };
 
 typedef union siginfo {
Index: sys/signalvar.h
===================================================================
RCS file: /cvsroot/src/sys/sys/signalvar.h,v
retrieving revision 1.42
diff -u -u -r1.42 signalvar.h
--- sys/signalvar.h	2003/09/06 22:01:20	1.42
+++ sys/signalvar.h	2003/09/14 05:13:26
@@ -52,22 +52,26 @@
 	int	sa_refcnt;		/* reference count */
 };
 
+#define SIGINFO_BUCKETS	7
+
 /*
  * Process signal state.
  */
-struct	sigctx {
+struct sigctx {
 	/* This needs to be zeroed on fork */
 	sigset_t ps_siglist;		/* Signals arrived but not delivered. */
 	char	ps_sigcheck;		/* May have deliverable signals. */
 	int	ps_sigwaited;		/* Delivered signal from wait set */
 	sigset_t ps_sigwait;		/* Signals being waited for */
+	struct ksiginfo *ps_siginfo[SIGINFO_BUCKETS]; /* for SA_SIGINFO */
 
 	/* This should be copied on fork */
 #define	ps_startcopy	ps_sigstk
 	struct	sigaltstack ps_sigstk;	/* sp & on stack state variable */
 	sigset_t ps_oldmask;		/* saved mask from before sigpause */
 	int	ps_flags;		/* signal flags, below */
-	struct ksiginfo ps_siginfo;	/* for core dump/debugger XXX */
+	int	ps_signo;		/* for core dump/debugger XXX */
+	int	ps_code;		/* for core dump/debugger XXX */
 	int	ps_lwp;			/* for core dump/debugger XXX */
 	void	*ps_sigcode;		/* address of signal trampoline */
 	sigset_t ps_sigmask;		/* Current signal mask. */