Re: [lwip-devel] Bad memory ref in tcp_input() after tcp_close()

[Per Ekman]

Valery Ushakov <address@hidden> writes:

> On Wed, Jan 22, 2014 at 09:30:33 +0100, Per Ekman wrote:
>
>> > - The application receive callback calls tcp_close()
>> >
>> > - tcp_close_shutdown() sees that the pcb is in CLOSE_WAIT and that
>> >   pcb->rcv_wnd != TCP_WND which causes it to free the pcb
>> >   and return ERR_OK
>
> So the callback hasn't acknowledged the data with tcp_recved() before
> calling tcp_close(), right?

Quite possibly.

>> > - The application receive callback returns ERR_OK to tcp_input()
>> >   which proceeds to dereference the freed pcb and passes it to
>> >   tcp_output() (tcp_input():386)
>
> Arguably, callback must not return ERR_OK in this case, since by
> calling tcp_close() without acknowledging incoming data it effectively
> aborts the connection.

Well, the rawapi.txt does not say this and also the comments in
tcp_close_shutdown() states that it handles this case (by sending a
RST). From the applications standpoint it just calls tcp_close(), not
tcp_abort().

> If the close is really intended to be a reaction on incoming FIN,
> callback should wait until it's called with NULL pbuf (not snoop on
> the pcb state, as my guess would be :).  It should also acknowledge
> incoming data.  If the close is indeed intended to abort the
> connection, then callback should return ERR_ABRT.

The close is caused by information in the incoming data so it cannot
wait for a incoming FIN. The code is not snooping on pcb state. In this
particular case the code triggering the close is called before the code
that calls tcp_recved() on the incoming data that caused the close due
to the way the tcp handling has been abstracted in our application. This
easily happens with the rawapi when upper-layer code manages the
connections. We could work around it by defering the close but it seems
to me that the lwip documentation allows our current implementation.

> Do I miss something in this picture?  TCP state transitions always
> make my head reel.

Mine too. It might be that the underlying error is that the pcb is in
CLOSE_WAIT when it shouldn't be but from a purely TCP perspective it
seems reasonable that it can be in CLOSE_WAIT and have unacked rx data.

Sincerely
Per Ekman
H&D Wireless AB