From 918470affc19c0023b0983b9e29cba24ea919d6a Mon Sep 17 00:00:00 2001 From: Simon Goldschmidt Date: Fri, 23 Sep 2011 19:14:17 +0200 Subject: [PATCH] fixed bug #33871: rejecting TCP_EVENT_RECV() for the last packet including FIN can lose data --- CHANGELOG | 4 ++++ src/core/tcp.c | 23 +++++++++++++++++++-- src/core/tcp_in.c | 44 +++++++++++++++++++++++++++++++---------- src/include/lwip/pbuf.h | 2 ++ 4 files changed, 61 insertions(+), 12 deletions(-) diff --git a/CHANGELOG b/CHANGELOG index c3c14259..df44d8d4 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -65,6 +65,10 @@ HISTORY ++ Bugfixes: + 2011-09-23: Simon Goldschmidt + * pbuf.h, tcp.c, tcp_in.c: fixed bug #33871: rejecting TCP_EVENT_RECV() for + the last packet including FIN can lose data + 2011-09-22: Simon Goldschmidt * tcp_impl.h: fixed bug #34355: nagle does not take snd_buf/snd_queuelen into account diff --git a/src/core/tcp.c b/src/core/tcp.c index 43244939..3a03ac1e 100644 --- a/src/core/tcp.c +++ b/src/core/tcp.c @@ -1054,13 +1054,32 @@ tcp_fasttmr(void) if (pcb->refused_data != NULL) { /* Notify again application with data previously received. */ err_t err; + u8_t refused_flags = pcb->refused_data->flags; + /* set pcb->refused_data to NULL in case the callback frees it and then + closes the pcb */ + struct pbuf *refused_data = pcb->refused_data; + pcb->refused_data = NULL; LWIP_DEBUGF(TCP_INPUT_DEBUG, ("tcp_fasttmr: notify kept packet\n")); - TCP_EVENT_RECV(pcb, pcb->refused_data, ERR_OK, err); + TCP_EVENT_RECV(pcb, refused_data, ERR_OK, err); if (err == ERR_OK) { - pcb->refused_data = NULL; + /* did refused_data include a FIN? If so, handle it now. */ + if (refused_flags & PBUF_FLAG_TCP_FIN) { + /* correct rcv_wnd as the application won't call tcp_recved() + for the FIN's seqno */ + if (pcb->rcv_wnd != TCP_WND) { + pcb->rcv_wnd++; + } + TCP_EVENT_CLOSED(pcb, err); + if (err == ERR_ABRT) { + pcb = NULL; + } + } } else if (err == ERR_ABRT) { /* if err == ERR_ABRT, 'pcb' is already deallocated */ pcb = NULL; + } else { + /* data is still refused */ + pcb->refused_data = refused_data; } } diff --git a/src/core/tcp_in.c b/src/core/tcp_in.c index 0cfb437b..c659b494 100644 --- a/src/core/tcp_in.c +++ b/src/core/tcp_in.c @@ -306,17 +306,36 @@ tcp_input(struct pbuf *p, struct netif *inp) /* If there is data which was previously "refused" by upper layer */ if (pcb->refused_data != NULL) { + u8_t refused_flags = pcb->refused_data->flags; + /* set pcb->refused_data to NULL in case the callback frees it and then + closes the pcb */ + struct pbuf *refused_data = pcb->refused_data; + pcb->refused_data = NULL; /* Notify again application with data previously received. */ LWIP_DEBUGF(TCP_INPUT_DEBUG, ("tcp_input: notify kept packet\n")); - TCP_EVENT_RECV(pcb, pcb->refused_data, ERR_OK, err); + TCP_EVENT_RECV(pcb, refused_data, ERR_OK, err); if (err == ERR_OK) { - pcb->refused_data = NULL; + /* did refused_data include a FIN? */ + if (refused_flags & PBUF_FLAG_TCP_FIN) { + /* correct rcv_wnd as the application won't call tcp_recved() + for the FIN's seqno */ + if (pcb->rcv_wnd != TCP_WND) { + pcb->rcv_wnd++; + } + TCP_EVENT_CLOSED(pcb, err); + if (err == ERR_ABRT) { + goto dropped; + } + } } else if ((err == ERR_ABRT) || (tcplen > 0)) { /* if err == ERR_ABRT, 'pcb' is already deallocated */ /* Drop incoming packets because pcb is "full" (only if the incoming segment contains data). */ LWIP_DEBUGF(TCP_INPUT_DEBUG, ("tcp_input: drop incoming packets, because pcb is \"full\"\n")); goto dropped; + } else { + /* data is still refused, pbuf is still valid (go on for ACK-only packets) */ + pcb->refused_data = refused_data; } } tcp_input_pcb = pcb; @@ -375,14 +394,19 @@ tcp_input(struct pbuf *p, struct netif *inp) /* If a FIN segment was received, we call the callback function with a NULL buffer to indicate EOF. */ if (recv_flags & TF_GOT_FIN) { - /* correct rcv_wnd as the application won't call tcp_recved() - for the FIN's seqno */ - if (pcb->rcv_wnd != TCP_WND) { - pcb->rcv_wnd++; - } - TCP_EVENT_CLOSED(pcb, err); - if (err == ERR_ABRT) { - goto aborted; + if (pcb->refused_data != NULL) { + /* Delay this if we have refused data. */ + pcb->refused_data->flags |= PBUF_FLAG_TCP_FIN; + } else { + /* correct rcv_wnd as the application won't call tcp_recved() + for the FIN's seqno */ + if (pcb->rcv_wnd != TCP_WND) { + pcb->rcv_wnd++; + } + TCP_EVENT_CLOSED(pcb, err); + if (err == ERR_ABRT) { + goto aborted; + } } } diff --git a/src/include/lwip/pbuf.h b/src/include/lwip/pbuf.h index 41c49099..786b288b 100644 --- a/src/include/lwip/pbuf.h +++ b/src/include/lwip/pbuf.h @@ -80,6 +80,8 @@ typedef enum { #define PBUF_FLAG_LLBCAST 0x08U /** indicates this pbuf was received as link-level multicast */ #define PBUF_FLAG_LLMCAST 0x10U +/** indicates this pbuf includes a TCP FIN flag */ +#define PBUF_FLAG_TCP_FIN 0x20U struct pbuf { /** next pbuf in singly linked pbuf chain */