netdev - Re: [PATCH] tcp: properly update lost_cnt

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <alpine.DEB.2.00.1109281418280.21709@wel-95.cs.helsinki.fi>
Date:	Wed, 28 Sep 2011 14:29:34 +0300 (EEST)
From:	"Ilpo Järvinen" <ilpo.jarvinen@...sinki.fi>
To:	"Yan, Zheng" <zheng.z.yan@...el.com>
cc:	"netdev@...r.kernel.org" <netdev@...r.kernel.org>,
	Nandita Dukkipati <nanditad@...gle.com>
Subject: Re: [PATCH] tcp: properly update lost_cnt_hint during shifting

On Wed, 28 Sep 2011, Yan, Zheng wrote:

> > But is the non-SACKed case really handled right when hint == skb by the 
> > sacktag_one. We move the seqno in between and then before(x->newseq, 
> > x->newseq) check returns false?
> > 
> you are right, thank you.
> 
> really hope my patch is correct this time :)
> ---
> diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
> index 21fab3e..a04622e 100644
> --- a/net/ipv4/tcp_input.c
> +++ b/net/ipv4/tcp_input.c
> @@ -1390,8 +1390,7 @@ static int tcp_shifted_skb(struct sock *sk, struct sk_buff *skb,
>  	BUG_ON(!pcount);
>  
>  	/* Tweak before seqno plays */
> -	if (!tcp_is_fack(tp) && tcp_is_sack(tp) && tp->lost_skb_hint &&
> -	    !before(TCP_SKB_CB(tp->lost_skb_hint)->seq, TCP_SKB_CB(skb)->seq))
> +	if (!tcp_is_fack(tp) && tcp_is_sack(tp) && tp->lost_skb_hint == skb)
>  		tp->lost_cnt_hint += pcount;
>  
>  	TCP_SKB_CB(prev)->end_seq += shifted;

It also looks a lot nicer now and more obvious. According to my current 
understanding, feel free to add this once doing the proper submission with 
Signed-off etc. (please also remove the comment too as seqnos have no 
longer any significance here):

Acked-by: Ilpo Järvinen <ilpo.jarvinen@...sinki.fi>

...but it certainly wouldn't hurt if also somebody else has pair of eyes 
to spare to confirm that we (both) are now in agreement what the code 
really says.


-- 
 i.