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

So it turns out that syzkaller hit another crash. It has to do with switching

Browse Source 
stacks with a SENT_FIN outstanding. Both rack and bbr will only send a
FIN if all data is ack'd so this must be enforced. Also if the previous stack
sent the FIN we need to make sure in rack that when we manufacture the
"unknown" sends that we include the proper HAS_FIN bits.

Note for BBR we take a simpler approach and just refuse to switch.

Sponsored by:	Netflix Inc.
Differential Revision:	https://reviews.freebsd.org/D26269
This commit is contained in:
rrs 2020-09-09 11:11:50 +00:00
parent 4305fae23e
commit da7c5efcab
2 changed files with 23 additions and 3 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

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

@ -10281,6 +10281,8 @@ bbr_handoff_ok(struct tcpcb *tp)
*/
return (EAGAIN);
}
if (tp->t_flags & TF_SENTFIN)
return (EINVAL);
if ((tp->t_flags & TF_SACK_PERMIT) || bbr_sack_not_required) {
return (0);
}

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

@ -10451,7 +10451,12 @@ rack_init(struct tcpcb *tp)
rsm->r_rtr_cnt = 1;
rsm->r_rtr_bytes = 0;
rsm->r_start = tp->snd_una;
rsm->r_end = tp->snd_max;
if (tp->t_flags & TF_SENTFIN) {
rsm->r_end = tp->snd_max - 1;
rsm->r_flags |= RACK_HAS_FIN;
} else {
rsm->r_end = tp->snd_max;
}
rsm->usec_orig_send = us_cts;
rsm->r_dupack = 0;
insret = RB_INSERT(rack_rb_tree_head, &rack->r_ctl.rc_mtree, rsm);
@ -10518,8 +10523,21 @@ rack_handoff_ok(struct tcpcb *tp)
if ((tp->t_state == TCPS_SYN_SENT) ||
(tp->t_state == TCPS_SYN_RECEIVED)) {
/*
* We really don't know you have to get to ESTAB or beyond
* to tell.
* We really don't know if you support sack,
* you have to get to ESTAB or beyond to tell.
*/
return (EAGAIN);
}
if ((tp->t_flags & TF_SENTFIN) && ((tp->snd_max - tp->snd_una) > 1)) {
/*
* Rack will only send a FIN after all data is acknowledged.
* So in this case we have more data outstanding. We can't
* switch stacks until either all data and only the FIN
* is left (in which case rack_init() now knows how
* to deal with that) <or> all is acknowledged and we
* are only left with incoming data, though why you
* would want to switch to rack after all data is acknowledged
* I have no idea (rrs)!
*/
return (EAGAIN);
}