d/freebsd-skq
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

This fixes a couple of skyzaller crashes. Most

Browse Source 
of them have to do with TFO. Even the default stack
had one of the issues:

1) We need to make sure for rack that we don't advance
   snd_nxt beyond iss when we are not doing fast open. We
   otherwise can get a bunch of SYN's sent out incorrectly
   with the seq number advancing.
2) When we complete the 3-way handshake we should not ever
   append to reassembly if the tlen is 0, if TFO is enabled
   prior to this fix we could still call the reasemmbly. Note
   this effects all three stacks.
3) Rack like its cousin BBR should track if a SYN is on a
   send map entry.
4) Both bbr and rack need to only consider len incremented on a SYN
   if the starting seq is iss, otherwise we don't increment len which
   may mean we return without adding a sendmap entry.

This work was done in collaberation with Michael Tuexen, thanks for
all the testing!
Sponsored by:	Netflix Inc
Differential Revision:	https://reviews.freebsd.org/D25000
This commit is contained in:
Randall Stewart 2020-06-03 14:07:31 +00:00
parent d442a65733
commit f1ea4e4120
4 changed files with 10 additions and 6 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
sys/netinet/tcp_input.c
View File

@ -2989,7 +2989,7 @@ tcp_do_segment(struct mbuf *m, struct tcphdr *th, struct socket *so,
*/
tfo_syn = ((tp->t_state == TCPS_SYN_RECEIVED) &&
IS_FASTOPEN(tp->t_flags));
if ((tlen || (thflags & TH_FIN) || tfo_syn) &&
if ((tlen || (thflags & TH_FIN) || (tfo_syn && tlen > 0)) &&
TCPS_HAVERCVDFIN(tp->t_state) == 0) {
tcp_seq save_start = th->th_seq;
tcp_seq save_rnxt = tp->rcv_nxt;

4
sys/netinet/tcp_stacks/bbr.c
View File

@ -6028,7 +6028,7 @@ bbr_log_output(struct tcp_bbr *bbr, struct tcpcb *tp, struct tcpopt *to, int32_t
* or FIN if seq_out is adding more on and a FIN is present
* (and we are not resending).
*/
if (th_flags & TH_SYN)
if ((th_flags & TH_SYN) && (tp->iss == seq_out))
len++;
if (th_flags & TH_FIN)
len++;
@ -8369,7 +8369,7 @@ bbr_process_data(struct mbuf *m, struct tcphdr *th, struct socket *so,
*/
tfo_syn = ((tp->t_state == TCPS_SYN_RECEIVED) &&
IS_FASTOPEN(tp->t_flags));
if ((tlen || (thflags & TH_FIN) || tfo_syn) &&
if ((tlen || (thflags & TH_FIN) || (tfo_syn && tlen > 0)) &&
TCPS_HAVERCVDFIN(tp->t_state) == 0) {
tcp_seq save_start = th->th_seq;
tcp_seq save_rnxt = tp->rcv_nxt;

9
sys/netinet/tcp_stacks/rack.c
View File

@ -6237,7 +6237,7 @@ rack_log_output(struct tcpcb *tp, struct tcpopt *to, int32_t len,
* or FIN if seq_out is adding more on and a FIN is present
* (and we are not resending).
*/
if (th_flags & TH_SYN)
if ((th_flags & TH_SYN) && (seq_out == tp->iss))
len++;
if (th_flags & TH_FIN)
len++;
@ -6280,6 +6280,7 @@ rack_log_output(struct tcpcb *tp, struct tcpopt *to, int32_t len,
rsm->usec_orig_send = us_cts;
if (th_flags & TH_SYN) {
/* The data space is one beyond snd_una */
rsm->r_flags |= RACK_HAS_SIN;
rsm->r_start = seq_out + 1;
rsm->r_end = rsm->r_start + (len - 1);
} else {
@ -8724,7 +8725,7 @@ rack_process_data(struct mbuf *m, struct tcphdr *th, struct socket *so,
*/
tfo_syn = ((tp->t_state == TCPS_SYN_RECEIVED) &&
IS_FASTOPEN(tp->t_flags));
if ((tlen || (thflags & TH_FIN) || tfo_syn) &&
if ((tlen || (thflags & TH_FIN) || (tfo_syn && tlen > 0)) &&
TCPS_HAVERCVDFIN(tp->t_state) == 0) {
tcp_seq save_start = th->th_seq;
tcp_seq save_rnxt = tp->rcv_nxt;
@ -12563,8 +12564,10 @@ rack_output(struct tcpcb *tp)
len = 0;
}
/* Without fast-open there should never be data sent on a SYN */
if ((flags & TH_SYN) && (!IS_FASTOPEN(tp->t_flags)))
if ((flags & TH_SYN) && (!IS_FASTOPEN(tp->t_flags))) {
tp->snd_nxt = tp->iss;
len = 0;
}
orig_len = len;
if (len <= 0) {
/*

1
sys/netinet/tcp_stacks/tcp_rack.h
View File

@ -39,6 +39,7 @@
#define RACK_RWND_COLLAPSED 0x0100/* The peer collapsed the rwnd on the segment */
#define RACK_APP_LIMITED 0x0200/* We went app limited after this send */
#define RACK_WAS_ACKED 0x0400/* a RTO undid the ack, but it already had a rtt calc done */
#define RACK_HAS_SIN 0x0800/* SIN is on this guy */
#define RACK_NUM_OF_RETRANS 3
#define RACK_INITIAL_RTO 1000 /* 1 second in milli seconds */