[PATCH] lib/tevent: handle tevent_common_add_signal on different event contexts.

[Rusty Russell]

On Mon, 31 Aug 2009 05:20:45 pm Matt Kraai wrote:
> Hi,
> 
> In the following section of code, won't ev be NULL the first time
> write is called, causing a NULL-pointer dereference?

Good point!  I'm not sure how that passed "make test"; it should have
broken horribly.  Hmm, re-testing here with s4's "make quicktest" reveals
that it passes :(  There's a file called testsuite.c in lib/tevent.... hmm,
"bin/smbtorture ncalrpc LOCAL-EVENT" catches it.

I note that Volker fixed this in 23abcd2318c69753aa2a144e1dc0f9cf9efdb705
(I've been away for 3 weeks, sorry, thanks very much Volker!!)

It's still not quite right, however: Volker, how's this?

Subject: [PATCH] lib/tevent: a cleaner fix for be4ac227842530d484659f2db683453366326d8b segv

Revert 23abcd2318c69753aa2a144e1dc0f9cf9efdb705 and fix logic bug.

The current code loops through the event contexts, when it sees a different
one, it notifies the current one (ev) and updates ev to point to the new one.

This is dumb, because:
(1) ev starts as NULL, so this code crashes, and
(2) The final context will not be notified.

The correct fix for this is to update ev to the new one, then notify it.
Volker's fix works because we currently always have one event context.

Signed-off-by: Rusty Russell <rusty at rustcorp.com.au>

diff --git a/lib/tevent/tevent_signal.c b/lib/tevent/tevent_signal.c
index ef9c0cf..b3d4823 100644
--- a/lib/tevent/tevent_signal.c
+++ b/lib/tevent/tevent_signal.c
@@ -89,18 +89,12 @@ static void tevent_common_signal_handler(int signum)
 	SIG_INCREMENT(sig_state->signal_count[signum]);
 	SIG_INCREMENT(sig_state->got_signal);
 
-	if (sig_state->sig_handlers[signum] != NULL) {
-		ev = sig_state->sig_handlers[signum]->se->event_ctx;
-		/* doesn't matter if this pipe overflows */
-		res = write(ev->pipe_fds[1], &c, 1);
-	}
-
 	/* Write to each unique event context. */
 	for (sl = sig_state->sig_handlers[signum]; sl; sl = sl->next) {
 		if (sl->se->event_ctx != ev) {
+			ev = sl->se->event_ctx;
 			/* doesn't matter if this pipe overflows */
 			res = write(ev->pipe_fds[1], &c, 1);
-			ev = sl->se->event_ctx;
 		}
 	}
 }








> 
> @@ -80,10 +79,20 @@ static void tevent_common_signal_handler(int signum)
>  {
>  	char c = 0;
>  	ssize_t res;
> +	struct tevent_common_signal_list *sl;
> +	struct tevent_context *ev = NULL;
> +
>  	SIG_INCREMENT(sig_state->signal_count[signum]);
>  	SIG_INCREMENT(sig_state->got_signal);
> -	/* doesn't matter if this pipe overflows */
> -	res = write(sig_state->pipe_hack[1], &c, 1);
> +
> +	/* Write to each unique event context. */
> +	for (sl = sig_state->sig_handlers[signum]; sl; sl = sl->next) {
> +		if (sl->se->event_ctx != ev) {
> +			/* doesn't matter if this pipe overflows */
> +			res = write(ev->pipe_fds[1], &c, 1);
> +			ev = sl->se->event_ctx;
> +		}
> +	}
>  }
>  
>  #ifdef SA_SIGINFO
> 
>