Skip to contentSkip to conversation

Avoid signal race in arpd initialization.

PATCH In review Tobias Stoeckmann 2 participants 2 messages Mar 11, 2015 – Mar 15, 2015

Subsystems (2)
Networking Network drivers
lore
Share & tools
lore

Tobias Stoeckmann

author
#0 lore raw

PATCH 1/1 Avoid signal race in arpd initialization.

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(-)
misc/arpd.c +9 -8
index 7919eb8..cfd1e39 100644
@@ -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;
Signature · 1 line
2.3.2
#1 lore raw
Quoted 7 lines · On Wed, 11 Mar 2015 21:58:45 +0100 Tobias Stoeckmann <[email protected]>…

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.