Re: race between tcp_close and delack?

On Fri, Oct 10, 2008 at 01:29:00PM +0200, Manuel Bouyer wrote:
> gasp, you're right. So this isn't my bug :( Sigh.
I found another path which could lead to a similar race.
The tcp timers are armed/disarmed, but we don't check if the callback is
being invoked. One of them at last can cause the panic I'm seeing in
tcp_output():
if (win == 0) {
TCP_TIMER_DISARM(tp, TCPT_REXMT);
tp->t_rxtshift = 0;
tp->snd_nxt = tp->snd_una;
if (TCP_TIMER_ISARMED(tp, TCPT_PERSIST) == 0)
tcp_setpersist(tp);
}
If TCPT_REXMT's callback is invoking at this point, it'll be run just
after tcp_output() exists. tcp_timer_rexmt() will restart the TCPT_REXMT
timer, so now we have both TCPT_REXMT and TCPT_PERSIST pending.
There are other places where we stop TCPT_REXMT, and potentially
arm another timer in the same splsoftnet() block (e.g. tcp_input
stops TCPT_REXMT and may call tcp_output, which may arm TCPT_PERSIST,
tcp_sack does it too).
There are also in tcp_output() possible paths where we disarm TCPT_PERSIST,
and later TCPT_REXMT while TCPT_PERSIST may still be invoking. I think
there could also cause the tcp_setpersist panic, as tcp_timer_persist can
call tcp_setpersist().
As TCP_TIMER_DISARM() will clear the pending and expirted states, I think a
possible way to close this race is to check callout_expired()
in the callout handler, and abort the handler if it's not expired.
See attached patch.
It looks like the same issue exists in netbsd-4 and current.
Comments ? Did I miss sometheing ?
--
Manuel Bouyer, LIP6, Universite Paris VI.
Manuel.Bouyer%lip6.fr@localhost
NetBSD: 26 ans d'experience feront toujours la difference
--