diff --git a/CHANGELOG b/CHANGELOG index cb57ccb6..de4000f8 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -249,10 +249,13 @@ HISTORY ++ Bugfixes: + 2015-08-28: Simon Goldschmidt + * tcp: fixed bug #45559: Window scaling casts u32_t to u16_t without checks + 2015-08-26: Simon Goldschmidt * ip6_frag.h/.c: fixed bug bug #41009: IPv6 reassembly broken on 64-bit platforms: define IPV6_FRAG_COPYHEADER==1 on these platforms to copy the IPv6 header - instead of referencing it, which gives more room for struct ip6_reass_helper + instead of referencing it, which gives more room for struct ip6_reass_helper 2015-08-25: Simon Goldschmidt * sockets.c: fixed bug #45827: recvfrom: TCP window is updated with MSG_PEEK diff --git a/src/core/tcp.c b/src/core/tcp.c index 9fe644ef..6c18d76f 100644 --- a/src/core/tcp.c +++ b/src/core/tcp.c @@ -175,7 +175,7 @@ tcp_close_shutdown(struct tcp_pcb *pcb, u8_t rst_on_unacked_data) err_t err; if (rst_on_unacked_data && ((pcb->state == ESTABLISHED) || (pcb->state == CLOSE_WAIT))) { - if ((pcb->refused_data != NULL) || (pcb->rcv_wnd != TCP_WND)) { + if ((pcb->refused_data != NULL) || (pcb->rcv_wnd != TCP_WND_MAX(pcb))) { /* Not all data received by application, send RST to tell the remote side about this. */ LWIP_ASSERT("pcb->flags & TF_RXCLOSED", pcb->flags & TF_RXCLOSED); @@ -673,15 +673,15 @@ tcp_recved(struct tcp_pcb *pcb, u16_t len) pcb->state != LISTEN); pcb->rcv_wnd += len; - if (pcb->rcv_wnd > TCP_WND) { - pcb->rcv_wnd = TCP_WND; + if (pcb->rcv_wnd > TCP_WND_MAX(pcb)) { + pcb->rcv_wnd = TCP_WND_MAX(pcb); } else if(pcb->rcv_wnd == 0) { /* rcv_wnd overflowed */ if ((pcb->state == CLOSE_WAIT) || (pcb->state == LAST_ACK)) { /* In passive close, we allow this, since the FIN bit is added to rcv_wnd by the stack itself, since it is not mandatory for an application to call tcp_recved() for the FIN bit, but e.g. the netconn API does so. */ - pcb->rcv_wnd = TCP_WND; + pcb->rcv_wnd = TCP_WND_MAX(pcb); } else { LWIP_ASSERT("tcp_recved: len wrapped rcv_wnd\n", 0); } @@ -699,7 +699,7 @@ tcp_recved(struct tcp_pcb *pcb, u16_t len) } LWIP_DEBUGF(TCP_DEBUG, ("tcp_recved: received %"U16_F" bytes, wnd %"U16_F" (%"U16_F").\n", - len, pcb->rcv_wnd, TCP_WND - pcb->rcv_wnd)); + len, pcb->rcv_wnd, TCP_WND_MAX(pcb) - pcb->rcv_wnd)); } /** @@ -814,8 +814,9 @@ tcp_connect(struct tcp_pcb *pcb, const ip_addr_t *ipaddr, u16_t port, pcb->snd_nxt = iss; pcb->lastack = iss - 1; pcb->snd_lbb = iss - 1; - pcb->rcv_wnd = TCP_WND; - pcb->rcv_ann_wnd = TCP_WND; + /* Start with a window that does not need scaling. When window scaling is + enabled and used, the window is enlarged when both sides agree on scaling. */ + pcb->rcv_wnd = pcb->rcv_ann_wnd = TCPWND_MIN16(TCP_WND); pcb->rcv_ann_right_edge = pcb->rcv_nxt; pcb->snd_wnd = TCP_WND; /* As initial send MSS, we use TCP_MSS but limit it to 536. @@ -1204,7 +1205,7 @@ tcp_process_refused_data(struct tcp_pcb *pcb) ) { /* correct rcv_wnd as the application won't call tcp_recved() for the FIN's seqno */ - if (pcb->rcv_wnd != TCP_WND) { + if (pcb->rcv_wnd != TCP_WND_MAX(pcb)) { pcb->rcv_wnd++; } TCP_EVENT_CLOSED(pcb, err); @@ -1474,8 +1475,9 @@ tcp_alloc(u8_t prio) pcb->prio = prio; pcb->snd_buf = TCP_SND_BUF; pcb->snd_queuelen = 0; - pcb->rcv_wnd = TCP_WND; - pcb->rcv_ann_wnd = TCP_WND; + /* Start with a window that does not need scaling. When window scaling is + enabled and used, the window is enlarged when both sides agree on scaling. */ + pcb->rcv_wnd = pcb->rcv_ann_wnd = TCPWND_MIN16(TCP_WND); #if LWIP_WND_SCALE /* snd_scale and rcv_scale are zero unless both sides agree to use scaling */ pcb->snd_scale = 0; diff --git a/src/core/tcp_in.c b/src/core/tcp_in.c index a2acdbdd..6de74033 100644 --- a/src/core/tcp_in.c +++ b/src/core/tcp_in.c @@ -460,7 +460,7 @@ tcp_input(struct pbuf *p, struct netif *inp) } else { /* correct rcv_wnd as the application won't call tcp_recved() for the FIN's seqno */ - if (pcb->rcv_wnd != TCP_WND) { + if (pcb->rcv_wnd != TCP_WND_MAX(pcb)) { pcb->rcv_wnd++; } TCP_EVENT_CLOSED(pcb, err); @@ -629,14 +629,14 @@ tcp_timewait_input(struct tcp_pcb *pcb) * - first check sequence number - we skip that one in TIME_WAIT (always * acceptable since we only send ACKs) * - second check the RST bit (... return) */ - if (flags & TCP_RST) { + if (flags & TCP_RST) { return ERR_OK; } /* - fourth, check the SYN bit, */ if (flags & TCP_SYN) { /* If an incoming segment is not acceptable, an acknowledgment should be sent in reply */ - if (TCP_SEQ_BETWEEN(seqno, pcb->rcv_nxt, pcb->rcv_nxt+pcb->rcv_wnd)) { + if (TCP_SEQ_BETWEEN(seqno, pcb->rcv_nxt, pcb->rcv_nxt + pcb->rcv_wnd)) { /* If the SYN is in the window it is an error, send a reset */ tcp_rst(ackno, seqno + tcplen, ip_current_dest_addr(), ip_current_src_addr(), tcphdr->dest, tcphdr->src); @@ -648,7 +648,7 @@ tcp_timewait_input(struct tcp_pcb *pcb) pcb->tmr = tcp_ticks; } - if ((tcplen > 0)) { + if ((tcplen > 0)) { /* Acknowledge data, FIN or out-of-window SYN */ pcb->flags |= TF_ACK_NOW; return tcp_output(pcb); @@ -684,8 +684,8 @@ tcp_process(struct tcp_pcb *pcb) acceptable = 1; } } else { - if (TCP_SEQ_BETWEEN(seqno, pcb->rcv_nxt, - pcb->rcv_nxt+pcb->rcv_wnd)) { + if (TCP_SEQ_BETWEEN(seqno, pcb->rcv_nxt, + pcb->rcv_nxt + pcb->rcv_wnd)) { acceptable = 1; } } @@ -1310,7 +1310,7 @@ tcp_receive(struct tcp_pcb *pcb) inseg.tcphdr->seqno = seqno = pcb->rcv_nxt; } else { - if (TCP_SEQ_LT(seqno, pcb->rcv_nxt)){ + if (TCP_SEQ_LT(seqno, pcb->rcv_nxt)) { /* the whole segment is < rcv_nxt */ /* must be a duplicate of a packet that has already been correctly handled */ @@ -1322,7 +1322,7 @@ tcp_receive(struct tcp_pcb *pcb) /* The sequence number must be within the window (above rcv_nxt and below rcv_nxt + rcv_wnd) in order to be further processed. */ - if (TCP_SEQ_BETWEEN(seqno, pcb->rcv_nxt, + if (TCP_SEQ_BETWEEN(seqno, pcb->rcv_nxt, pcb->rcv_nxt + pcb->rcv_wnd - 1)){ if (pcb->rcv_nxt == seqno) { /* The incoming segment is the next in sequence. We check if @@ -1331,17 +1331,18 @@ tcp_receive(struct tcp_pcb *pcb) tcplen = TCP_TCPLEN(&inseg); if (tcplen > pcb->rcv_wnd) { - LWIP_DEBUGF(TCP_INPUT_DEBUG, + LWIP_DEBUGF(TCP_INPUT_DEBUG, ("tcp_receive: other end overran receive window" "seqno %"U32_F" len %"U16_F" right edge %"U32_F"\n", seqno, tcplen, pcb->rcv_nxt + pcb->rcv_wnd)); if (TCPH_FLAGS(inseg.tcphdr) & TCP_FIN) { - /* Must remove the FIN from the header as we're trimming + /* Must remove the FIN from the header as we're trimming * that byte of sequence-space from the packet */ TCPH_FLAGS_SET(inseg.tcphdr, TCPH_FLAGS(inseg.tcphdr) & ~(unsigned int)TCP_FIN); } /* Adjust length of segment to fit in the window. */ - inseg.len = pcb->rcv_wnd; + TCPWND_CHECK16(pcb->rcv_wnd); + inseg.len = (u16_t)pcb->rcv_wnd; if (TCPH_FLAGS(inseg.tcphdr) & TCP_SYN) { inseg.len -= 1; } @@ -1643,9 +1644,7 @@ tcp_receive(struct tcp_pcb *pcb) } else { /* Segments with length 0 is taken care of here. Segments that fall out of the window are ACKed. */ - /*if (TCP_SEQ_GT(pcb->rcv_nxt, seqno) || - TCP_SEQ_GEQ(seqno, pcb->rcv_nxt + pcb->rcv_wnd)) {*/ - if(!TCP_SEQ_BETWEEN(seqno, pcb->rcv_nxt, pcb->rcv_nxt + pcb->rcv_wnd-1)){ + if(!TCP_SEQ_BETWEEN(seqno, pcb->rcv_nxt, pcb->rcv_nxt + pcb->rcv_wnd - 1)){ tcp_ack_now(pcb); } } @@ -1725,6 +1724,10 @@ tcp_parseopt(struct tcp_pcb *pcb) } pcb->rcv_scale = TCP_RCV_SCALE; pcb->flags |= TF_WND_SCALE; + /* window scaling is enabled, we can use the full receive window */ + LWIP_ASSERT("window not at default value", pcb->rcv_wnd == TCPWND_MIN16(TCP_WND)); + LWIP_ASSERT("window not at default value", pcb->rcv_ann_wnd == TCPWND_MIN16(TCP_WND)); + pcb->rcv_wnd = pcb->rcv_ann_wnd = TCP_WND; } break; #endif diff --git a/src/core/tcp_out.c b/src/core/tcp_out.c index 98bae81f..01fbc6a0 100644 --- a/src/core/tcp_out.c +++ b/src/core/tcp_out.c @@ -121,7 +121,7 @@ tcp_output_alloc_header(struct tcp_pcb *pcb, u16_t optlen, u16_t datalen, tcphdr->seqno = seqno_be; tcphdr->ackno = htonl(pcb->rcv_nxt); TCPH_HDRLEN_FLAGS_SET(tcphdr, (5 + optlen / 4), TCP_ACK); - tcphdr->wnd = htons(RCV_WND_SCALE(pcb, pcb->rcv_ann_wnd)); + tcphdr->wnd = htons(TCPWND_MIN16(RCV_WND_SCALE(pcb, pcb->rcv_ann_wnd))); tcphdr->chksum = 0; tcphdr->urgp = 0; @@ -387,7 +387,7 @@ tcp_write(struct tcp_pcb *pcb, const void *arg, u16_t len, u8_t apiflags) #endif /* TCP_CHECKSUM_ON_COPY */ err_t err; /* don't allocate segments bigger than half the maximum window we ever received */ - u16_t mss_local = LWIP_MIN(pcb->mss, pcb->snd_wnd_max/2); + u16_t mss_local = LWIP_MIN(pcb->mss, TCPWND_MIN16(pcb->snd_wnd_max/2)); mss_local = mss_local ? mss_local : pcb->mss; #if LWIP_NETIF_TX_SINGLE_PBUF @@ -1141,11 +1141,11 @@ tcp_output_segment(struct tcp_seg *seg, struct tcp_pcb *pcb) if (seg->flags & TF_SEG_OPTS_WND_SCALE) { /* The Window field in a SYN segment itself (the only type where we send the window scale option) is never scaled. */ - seg->tcphdr->wnd = htons(pcb->rcv_ann_wnd); + seg->tcphdr->wnd = htons(TCPWND_MIN16(pcb->rcv_ann_wnd)); } else #endif /* LWIP_WND_SCALE */ { - seg->tcphdr->wnd = htons(RCV_WND_SCALE(pcb, pcb->rcv_ann_wnd)); + seg->tcphdr->wnd = htons(TCPWND_MIN16(RCV_WND_SCALE(pcb, pcb->rcv_ann_wnd))); } pcb->rcv_ann_right_edge = pcb->rcv_nxt + pcb->rcv_ann_wnd; diff --git a/src/include/lwip/opt.h b/src/include/lwip/opt.h index e923cad7..52f19b8d 100644 --- a/src/include/lwip/opt.h +++ b/src/include/lwip/opt.h @@ -1245,7 +1245,7 @@ * explicit window update */ #ifndef TCP_WND_UPDATE_THRESHOLD -#define TCP_WND_UPDATE_THRESHOLD (TCP_WND / 4) +#define TCP_WND_UPDATE_THRESHOLD LWIP_MIN((TCP_WND / 4), (TCP_MSS * 4)) #endif /** diff --git a/src/include/lwip/tcp.h b/src/include/lwip/tcp.h index 1311d68d..e0616c2e 100644 --- a/src/include/lwip/tcp.h +++ b/src/include/lwip/tcp.h @@ -128,13 +128,15 @@ typedef err_t (*tcp_connected_fn)(void *arg, struct tcp_pcb *tpcb, err_t err); #if LWIP_WND_SCALE #define RCV_WND_SCALE(pcb, wnd) (((wnd) >> (pcb)->rcv_scale)) #define SND_WND_SCALE(pcb, wnd) (((wnd) << (pcb)->snd_scale)) -#define TCPWND16(x) ((u16_t)LWIP_MIN((x), 0xFFFF)) +#define TCPWND16(x) ((u16_t)LWIP_MIN((x), 0xFFFF)) +#define TCP_WND_MAX(pcb) ((tcpwnd_size_t)(((pcb)->flags & TF_WND_SCALE) ? TCP_WND : TCPWND16(TCP_WND))) typedef u32_t tcpwnd_size_t; typedef u16_t tcpflags_t; #else #define RCV_WND_SCALE(pcb, wnd) (wnd) #define SND_WND_SCALE(pcb, wnd) (wnd) -#define TCPWND16(x) (x) +#define TCPWND16(x) (x) +#define TCP_WND_MAX(pcb) TCP_WND typedef u16_t tcpwnd_size_t; typedef u8_t tcpflags_t; #endif diff --git a/src/include/lwip/tcp_impl.h b/src/include/lwip/tcp_impl.h index b02b054c..51401223 100644 --- a/src/include/lwip/tcp_impl.h +++ b/src/include/lwip/tcp_impl.h @@ -332,11 +332,15 @@ struct tcp_seg { #define TCP_BUILD_MSS_OPTION(mss) htonl(0x02040000 | ((mss) & 0xFFFF)) #if LWIP_WND_SCALE -#define TCPWNDSIZE_F U32_F -#define TCPWND_MAX 0xFFFFFFFFU +#define TCPWNDSIZE_F U32_F +#define TCPWND_MAX 0xFFFFFFFFU +#define TCPWND_CHECK16(x) LWIP_ASSERT("window size > 0xFFFF", (x) <= 0xFFFF) +#define TCPWND_MIN16(x) ((u16_t)LWIP_MIN((x), 0xFFFF)) #else /* LWIP_WND_SCALE */ -#define TCPWNDSIZE_F U16_F -#define TCPWND_MAX 0xFFFFU +#define TCPWNDSIZE_F U16_F +#define TCPWND_MAX 0xFFFFU +#define TCPWND_CHECK16(x) +#define TCPWND_MIN16(x) x #endif /* LWIP_WND_SCALE */ /* Global variables: */