mirror/lwip
1
0
Fork 0
mirror of https://github.com/lwip-tcpip/lwip.git synced 2024-12-25 18:14:53 +00:00
Code Issues Packages Projects Releases Wiki Activity

tcp: handle segmentation oversize during segment split (bug #52676)

Browse Source 
This fixes a bug in tcp_split_unsent_seg where oversized segments were not
handled during the split, leading to pcb->unsent_oversized and
useg->oversize_left getting out of sync with the split segment

This would result in over-writing the pbuf if another call to tcp_write()
happened after the split, but before the remainder of the split was sent in
tcp_output

Now pcb->unsent_oversized is explicitly cleared (because the remainder at
the tail is never oversized) and useg->oversized_left is cleared after it is
trimmed

This also updates the test_tcp_persist_split unit test to explicitly check for
this case
This commit is contained in:
Joel Cunningham 2017-12-15 12:15:35 -06:00
parent 31c60775b6
commit 50a5d85f45
2 changed files with 50 additions and 26 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

12
src/core/tcp_out.c
View File

@ -1902,6 +1902,10 @@ tcp_split_unsent_seg(struct tcp_pcb *pcb, u16_t split)
pbuf_realloc(useg->p, useg->p->tot_len - remainder); pbuf_realloc(useg->p, useg->p->tot_len - remainder);
useg->len -= remainder; useg->len -= remainder;
TCPH_SET_FLAG(useg->tcphdr, split_flags); TCPH_SET_FLAG(useg->tcphdr, split_flags);
#if TCP_OVERSIZE_DBGCHECK
/* By trimming, realloc may have actually shrunk the pbuf, so clear oversize_left */
useg->oversize_left = 0;
#endif /* TCP_OVERSIZE_DBGCHECK */
#if TCP_CHECKSUM_ON_COPY #if TCP_CHECKSUM_ON_COPY
/* The checksum on the split segment is now incorrect. We need to re-run it over the split */ /* The checksum on the split segment is now incorrect. We need to re-run it over the split */
@ -1932,6 +1936,14 @@ tcp_split_unsent_seg(struct tcp_pcb *pcb, u16_t split)
seg->next = useg->next; seg->next = useg->next;
useg->next = seg; useg->next = seg;
#if TCP_OVERSIZE
/* If remainder is last segment on the unsent, ensure we clear the oversize amount
* because the remainder is always sized to the exact remaining amount */
if (seg->next == NULL) {
pcb->unsent_oversize = 0;
}
#endif /* TCP_OVERSIZE */
return ERR_OK; return ERR_OK;
memerr: memerr:
TCP_STATS_INC(tcp.memerr); TCP_STATS_INC(tcp.memerr);

64
test/unit/tcp/test_tcp.c
View File

@ -1225,40 +1225,41 @@ START_TEST(test_tcp_persist_split)
pcb->snd_wnd = 3 * TCP_MSS; pcb->snd_wnd = 3 * TCP_MSS;
pcb->snd_wnd_max = 3 * TCP_MSS; pcb->snd_wnd_max = 3 * TCP_MSS;
/* send three segments */ /* send four segments. Fourth should stay buffered and is a 3/4 MSS segment to
err = tcp_write(pcb, &tx_data[0], 3 * TCP_MSS, TCP_WRITE_FLAG_COPY); get coverage on the oversized segment case */
err = tcp_write(pcb, &tx_data[0], (3 * TCP_MSS) + (TCP_MSS - (TCP_MSS / 4)), TCP_WRITE_FLAG_COPY);
EXPECT(err == ERR_OK); EXPECT(err == ERR_OK);
err = tcp_output(pcb); err = tcp_output(pcb);
EXPECT(err == ERR_OK); EXPECT(err == ERR_OK);
/* verify segments are in-flight */ /* verify 3 segments are in-flight */
EXPECT(pcb->unsent == NULL);
EXPECT(pcb->unacked != NULL); EXPECT(pcb->unacked != NULL);
check_seqnos(pcb->unacked, 3, seqnos); check_seqnos(pcb->unacked, 3, seqnos);
EXPECT(txcounters.num_tx_calls == 3); EXPECT(txcounters.num_tx_calls == 3);
EXPECT(txcounters.num_tx_bytes == 3 * (TCP_MSS + 40U)); EXPECT(txcounters.num_tx_bytes == 3 * (TCP_MSS + 40U));
memset(&txcounters, 0, sizeof(txcounters)); memset(&txcounters, 0, sizeof(txcounters));
/* verify 4th segment is on unsent */
EXPECT(pcb->unsent != NULL);
EXPECT(pcb->unsent->len == TCP_MSS - (TCP_MSS / 4));
check_seqnos(pcb->unsent, 1, &seqnos[3]);
#if TCP_OVERSIZE
EXPECT(pcb->unsent_oversize == TCP_MSS / 4);
#if TCP_OVERSIZE_DBGCHECK
EXPECT(pcb->unsent->oversize_left == pcb->unsent_oversize);
#endif /* TCP_OVERSIZE_DBGCHECK */
#endif /* TCP_OVERSIZE */
/* ACK the segments and update the window to only 1/2 TCP_MSS */ /* ACK the 3 segments and update the window to only 1/2 TCP_MSS.
4th segment should stay on unsent because it's bigger than 1/2 MSS */
p = tcp_create_rx_segment_wnd(pcb, NULL, 0, 0, 3 * TCP_MSS, TCP_ACK, TCP_MSS / 2); p = tcp_create_rx_segment_wnd(pcb, NULL, 0, 0, 3 * TCP_MSS, TCP_ACK, TCP_MSS / 2);
test_tcp_input(p, &netif); test_tcp_input(p, &netif);
EXPECT(pcb->unacked == NULL); EXPECT(pcb->unacked == NULL);
EXPECT(pcb->unsent == NULL);
EXPECT(pcb->persist_backoff == 0);
EXPECT(pcb->snd_wnd == TCP_MSS / 2); EXPECT(pcb->snd_wnd == TCP_MSS / 2);
/* send fourth segment, which is larger than snd_wnd */
err = tcp_write(pcb, &tx_data[3 * TCP_MSS], TCP_MSS, TCP_WRITE_FLAG_COPY);
EXPECT(err == ERR_OK);
err = tcp_output(pcb);
EXPECT(err == ERR_OK);
/* ensure it is buffered and persist timer started */
EXPECT(pcb->unacked == NULL);
EXPECT(pcb->unsent != NULL); EXPECT(pcb->unsent != NULL);
check_seqnos(pcb->unsent, 1, &seqnos[3]); check_seqnos(pcb->unsent, 1, &seqnos[3]);
EXPECT(txcounters.num_tx_calls == 0); EXPECT(txcounters.num_tx_calls == 0);
EXPECT(txcounters.num_tx_bytes == 0); EXPECT(txcounters.num_tx_bytes == 0);
/* persist timer should be started since 4th segment is stuck waiting on snd_wnd */
EXPECT(pcb->persist_backoff == 1); EXPECT(pcb->persist_backoff == 1);
/* ensure no errors have been recorded */ /* ensure no errors have been recorded */
@ -1281,11 +1282,22 @@ START_TEST(test_tcp_persist_split)
EXPECT(pcb->persist_backoff == 0); EXPECT(pcb->persist_backoff == 0);
EXPECT(txcounters.num_tx_calls == 1); EXPECT(txcounters.num_tx_calls == 1);
EXPECT(txcounters.num_tx_bytes == ((TCP_MSS /2) + 40U)); EXPECT(txcounters.num_tx_bytes == ((TCP_MSS /2) + 40U));
/* verify half segment sent, half still buffered */ /* verify 1/2 MSS segment sent, 1/4 MSS still buffered */
EXPECT(pcb->unsent != NULL); EXPECT(pcb->unsent != NULL);
EXPECT(pcb->unsent->len == TCP_MSS / 2); EXPECT(pcb->unsent->len == TCP_MSS / 4);
EXPECT(pcb->unacked != NULL); EXPECT(pcb->unacked != NULL);
EXPECT(pcb->unacked->len == TCP_MSS / 2); EXPECT(pcb->unacked->len == TCP_MSS / 2);
#if TCP_OVERSIZE
/* verify there is no oversized remaining since during the
segment split, the remainder pbuf is always the exact length */
EXPECT(pcb->unsent_oversize == 0);
#if TCP_OVERSIZE_DBGCHECK
/* Split segment already transmitted, should be at 0 */
EXPECT(pcb->unacked->oversize_left == 0);
/* Remainder segement should match pcb value (which is 0) */
EXPECT(pcb->unsent->oversize_left == pcb->unsent_oversize);
#endif /* TCP_OVERSIZE_DBGCHECK */
#endif /* TCP_OVERSIZE */
/* verify first half segment */ /* verify first half segment */
EXPECT(txcounters.tx_packets != NULL); EXPECT(txcounters.tx_packets != NULL);
@ -1307,22 +1319,22 @@ START_TEST(test_tcp_persist_split)
txcounters.copy_tx_packets = 1; txcounters.copy_tx_packets = 1;
test_tcp_input(p, &netif); test_tcp_input(p, &netif);
txcounters.copy_tx_packets = 0; txcounters.copy_tx_packets = 0;
/* ensure remaining half segment was sent */ /* ensure remaining segment was sent */
EXPECT(txcounters.num_tx_calls == 1); EXPECT(txcounters.num_tx_calls == 1);
EXPECT(txcounters.num_tx_bytes == ((TCP_MSS /2 ) + 40U)); EXPECT(txcounters.num_tx_bytes == ((TCP_MSS / 4) + 40U));
EXPECT(pcb->unsent == NULL); EXPECT(pcb->unsent == NULL);
EXPECT(pcb->unacked != NULL); EXPECT(pcb->unacked != NULL);
EXPECT(pcb->unacked->len == TCP_MSS / 2); EXPECT(pcb->unacked->len == TCP_MSS / 4);
EXPECT(pcb->snd_wnd == TCP_MSS / 2); EXPECT(pcb->snd_wnd == TCP_MSS / 2);
/* verify second half segment */ /* verify remainder segment */
EXPECT(txcounters.tx_packets != NULL); EXPECT(txcounters.tx_packets != NULL);
if (txcounters.tx_packets != NULL) { if (txcounters.tx_packets != NULL) {
u8_t sent[TCP_MSS / 2]; u8_t sent[TCP_MSS / 4];
u16_t ret; u16_t ret;
ret = pbuf_copy_partial(txcounters.tx_packets, &sent, TCP_MSS / 2, 40U); ret = pbuf_copy_partial(txcounters.tx_packets, &sent, TCP_MSS / 4, 40U);
EXPECT(ret == TCP_MSS / 2); EXPECT(ret == TCP_MSS / 4);
EXPECT(memcmp(sent, &tx_data[(3 * TCP_MSS) + TCP_MSS / 2], TCP_MSS / 2) == 0); EXPECT(memcmp(sent, &tx_data[(3 * TCP_MSS) + TCP_MSS / 2], TCP_MSS / 4) == 0);
} }
if (txcounters.tx_packets != NULL) { if (txcounters.tx_packets != NULL) {
pbuf_free(txcounters.tx_packets); pbuf_free(txcounters.tx_packets);