lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Fri, 4 Oct 2013 21:03:53 +0300 (EEST) From: "Ilpo Järvinen" <ilpo.jarvinen@...sinki.fi> To: Eric Dumazet <eric.dumazet@...il.com> cc: David Miller <davem@...emloft.net>, netdev <netdev@...r.kernel.org>, Neal Cardwell <ncardwell@...gle.com>, Yuchung Cheng <ycheng@...gle.com> Subject: Re: [PATCH] tcp: do not forget FIN in tcp_shifted_skb() On Fri, 4 Oct 2013, Eric Dumazet wrote: > From: Eric Dumazet <edumazet@...gle.com> > > Yuchung found following problem : > > There are bugs in the SACK processing code, merging part in > tcp_shift_skb_data(), that incorrectly resets or ignores the sacked > skbs FIN flag. When a receiver first SACK the FIN sequence, and later > throw away ofo queue (e.g., sack-reneging), the sender will stop > retransmitting the FIN flag, and hangs forever. > > Following packetdrill test can be used to reproduce the bug. > > [...snip...] > > First, a typo inverted left/right of one OR operation, then > code forgot to advance end_seq if the merged skb carried FIN. > > Bug was added in 2.6.29 by commit 832d11c5cd076ab > ("tcp: Try to restore large SKBs while SACK processing") > > Signed-off-by: Eric Dumazet <edumazet@...gle.com> > Signed-off-by: Yuchung Cheng <ycheng@...gle.com> > Acked-by: Neal Cardwell <ncardwell@...gle.com> > Cc: Ilpo Järvinen <ilpo.jarvinen@...sinki.fi> > --- > net/ipv4/tcp_input.c | 5 ++++- > 1 file changed, 4 insertions(+), 1 deletion(-) > > diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c > index 25a89ea..113dc5f 100644 > --- a/net/ipv4/tcp_input.c > +++ b/net/ipv4/tcp_input.c > @@ -1284,7 +1284,10 @@ static bool tcp_shifted_skb(struct sock *sk, struct sk_buff *skb, > tp->lost_cnt_hint -= tcp_skb_pcount(prev); > } > > - TCP_SKB_CB(skb)->tcp_flags |= TCP_SKB_CB(prev)->tcp_flags; > + TCP_SKB_CB(prev)->tcp_flags |= TCP_SKB_CB(skb)->tcp_flags; > + if (TCP_SKB_CB(skb)->tcp_flags & TCPHDR_FIN) > + TCP_SKB_CB(prev)->end_seq++; > + > if (skb == tcp_highest_sack(sk)) > tcp_advance_highest_sack(sk, skb); Nice that it was finally found. For some reason my memory tries to say that it wouldn't have not even tried to merge skbs with FIN but either I changed that at some point while deving or I just remember wrong. Acked-by: Ilpo Järvinen <ilpo.jarvinen@...sinki.fi> -- i.
Powered by blists - more mailing lists