diff mbox

Avoid signal race in arpd initialization.

Message ID 20150311205845.GA23722@localhost
State Rejected, archived
Delegated to: stephen hemminger
Headers show

Commit Message

Tobias Stoeckmann March 11, 2015, 8:58 p.m. UTC 
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(-)

Comments

Stephen Hemminger March 15, 2015, 7:25 p.m. UTC | #1 
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.


--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

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;