|
Message-ID: <20150912201932.GA27562@openwall.com> Date: Sat, 12 Sep 2015 23:19:32 +0300 From: Solar Designer <solar@...nwall.com> To: john-dev@...ts.openwall.com Subject: bt.c signal handler Sayantan - Your use of a signal handler in bt.c is buggy: The signal_stop variable should be "volatile int" (like we use in signals.c), or we might want to convert all of them to "volatile sig_atomic_t". The "volatile" keyword is required - without it, compiler optimization may break your code from working any time (I've seen it happen in similar cases). The compiler may simply pre-load signal_stop into a register in create_tables() and then it would not detect this variable changing inside the loop. Another bug is that stdio is not async signal safe. You shouldn't use stdio streams in signal handlers. You also shouldn't use printf family functions in signal handlers (these functions are supposed to be thread-safe when you invoke them on buffers not shared with any other thread, but they might not be async signal safe). I suggest that you only set the variable in the signal handler, and postpone the fprintf() until the program notices that signal_stop is set. Alternatively, you may use write(). When you're detecting whether signal_stop is set, you currently do: if (signal_stop) { signal_stop = 0; alarm(0); Depending on how your timer is initialized (one-shot vs. multiple), this might contain a race condition: what if the timer fires a second time after you've reset "signal_stop = 0;" but before alarm(0); completes? I think you want to swap these two lines: do alarm(0); first, and reset signal_stop to 0 next. That way, you'd ignore a possible second signal. Alexander
Powered by blists - more mailing lists
Confused about mailing lists and their use? Read about mailing lists on Wikipedia and check out these guidelines on proper formatting of your messages.