Small problem with tcp_poll and RST

Small problem with tcp_poll and RST

[Tom Marshall]

The code in tcp_poll seems to suffer from a race condition which can
result in POLLIN but not POLLOUT for an outbound socket connection to
a closed peer.

This can happen if, for example, the RST comes in immediately after
checking sk->sk_err.  It is a small window of opportunity and so it
only happens rarely.

Note this code has remained pretty much unchanged in 2.6.x for years,
and the problem readily reproduces on a wide variety of systems (RHEL
5.x, Ubuntu 10.04, etc.)

I suppose it is arguable whether this is a bug or whether it deserves
to be fixed, but it did cause an issue with some (admittedly broken)
userspace code at my company.

I do not fully understand the intricacies of the interactions between
the TCP state machine and the tcp_poll function (which runs unlocked).
 However, I did find that the below appears to fix the issue.  Since
the overhead is minimal when the socket state does not change, it
should have very little performance impact.

        unsigned char oldstate;
again:
        oldstate = sk->sk_state;

        /* body of tcp_poll */

        if (sk->sk_state != oldstate)
                goto again;

Thanks!

Re: Small problem with tcp_poll and RST

[Eric Dumazet]

Le mercredi 15 septembre 2010 à 13:17 -0700, Tom Marshall a écrit :
> The code in tcp_poll seems to suffer from a race condition which can
> result in POLLIN but not POLLOUT for an outbound socket connection to
> a closed peer.
> 
> This can happen if, for example, the RST comes in immediately after
> checking sk->sk_err.  It is a small window of opportunity and so it
> only happens rarely.
> 
> Note this code has remained pretty much unchanged in 2.6.x for years,
> and the problem readily reproduces on a wide variety of systems (RHEL
> 5.x, Ubuntu 10.04, etc.)
> 
> I suppose it is arguable whether this is a bug or whether it deserves
> to be fixed, but it did cause an issue with some (admittedly broken)
> userspace code at my company.
> 
> I do not fully understand the intricacies of the interactions between
> the TCP state machine and the tcp_poll function (which runs unlocked).
>  However, I did find that the below appears to fix the issue.  Since
> the overhead is minimal when the socket state does not change, it
> should have very little performance impact.
> 
>         unsigned char oldstate;
> again:
>         oldstate = sk->sk_state;
> 
>         /* body of tcp_poll */
> 
>         if (sk->sk_state != oldstate)
>                 goto again;
> 

Hi Tom

Unfortunately this might shorten the race window.

Are you saying an application is blocked on a poll(events=POLLOUT) ?

If yes, we have a bug elsewhere (in RST processing)