diff --git a/CHANGELOG b/CHANGELOG index ef25485b..559c3d29 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -45,6 +45,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 a4bb025e..578413a1 100644 --- a/src/core/tcp.c +++ b/src/core/tcp.c @@ -1026,13 +1026,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 ff36e878..35289132 100644 --- a/src/core/tcp_in.c +++ b/src/core/tcp_in.c @@ -303,11 +303,27 @@ 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 @@ -317,6 +333,9 @@ tcp_input(struct pbuf *p, struct netif *inp) snmp_inc_tcpinerrs(); pbuf_free(p); return; + } 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; + } } } @@ -425,6 +449,10 @@ aborted: LWIP_ASSERT("tcp_input: tcp_pcbs_sane()", tcp_pcbs_sane()); PERF_STOP("tcp_input"); +dropped: + TCP_STATS_INC(tcp.drop); + snmp_inc_tcpinerrs(); + pbuf_free(p); } /** diff --git a/src/include/lwip/pbuf.h b/src/include/lwip/pbuf.h index 024179f2..faeb5a0c 100644 --- a/src/include/lwip/pbuf.h +++ b/src/include/lwip/pbuf.h @@ -69,6 +69,8 @@ typedef enum { #define PBUF_FLAG_IS_CUSTOM 0x02U /** indicates this pbuf is UDP multicast to be looped back */ #define PBUF_FLAG_MCASTLOOP 0x04U +/** indicates this pbuf includes a TCP FIN flag */ +#define PBUF_FLAG_TCP_FIN 0x20U struct pbuf { /** next pbuf in singly linked pbuf chain */