[PATCH] Avoid signal race in arpd initialization.

[PATCH] Avoid signal race in arpd initialization.

[Tobias Stoeckmann]

Signal handlers in arpd use siglongjmp() to return into main function
during polls. The environment for the jumps is set after the signal
handlers are installed. This leaves a small time frame in which an
uninitialized environment could be used for a siglongjmp() call, leading
to undefined behavior.

While at it, define flag variables as sig_atomic_t instead of int.
---
 misc/arpd.c | 17 +++++++++--------
 1 file changed, 9 insertions(+), 8 deletions(-)

diff --git a/misc/arpd.c b/misc/arpd.c
index 7919eb8..cfd1e39 100644
--- a/misc/arpd.c
+++ b/misc/arpd.c
@@ -64,9 +64,9 @@ struct rtnl_handle rth;
 struct pollfd pset[2];
 int udp_sock = -1;
 
-volatile int do_exit;
-volatile int do_sync;
-volatile int do_stats;
+volatile sig_atomic_t do_exit;
+volatile sig_atomic_t do_sync;
+volatile sig_atomic_t do_stats;
 
 struct {
 	unsigned long arp_new;
@@ -793,10 +793,6 @@ int main(int argc, char **argv)
 	}
 
 	openlog("arpd", LOG_PID | LOG_CONS, LOG_DAEMON);
-	catch_signal(SIGINT, sig_exit);
-	catch_signal(SIGTERM, sig_exit);
-	catch_signal(SIGHUP, sig_sync);
-	catch_signal(SIGUSR1, sig_stats);
 
 #define EVENTS (POLLIN|POLLPRI|POLLERR|POLLHUP)
 	pset[0].events = EVENTS;
@@ -804,7 +800,12 @@ int main(int argc, char **argv)
 	pset[1].events = EVENTS;
 	pset[1].revents = 0;
 
-	sigsetjmp(env, 1);
+	if (sigsetjmp(env, 1) == 0) {
+		catch_signal(SIGINT, sig_exit);
+		catch_signal(SIGTERM, sig_exit);
+		catch_signal(SIGHUP, sig_sync);
+		catch_signal(SIGUSR1, sig_stats);
+	}
 
 	for (;;) {
 		in_poll = 1;
-- 
2.3.2

Re: [PATCH] Avoid signal race in arpd initialization.

[Stephen Hemminger]

On Wed, 11 Mar 2015 21:58:45 +0100
Tobias Stoeckmann <tobias@stoeckmann.org> wrote:

> Signal handlers in arpd use siglongjmp() to return into main function
> during polls. The environment for the jumps is set after the signal
> handlers are installed. This leaves a small time frame in which an
> uninitialized environment could be used for a siglongjmp() call, leading
> to undefined behavior.
> 
> While at it, define flag variables as sig_atomic_t instead of int.

I understand, but this is such a corner case. And you made several
other changes. Don't think it is worth doing this change.

You would have to signal arpd in the very small window between
the signal setup and the setjmp call.