netdev - Re: [PATCH v2] tcp: fix connection reset due to tw hashdance race.

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] [day] [month] [year] [list]

Message-ID: <D984749A-EACD-4408-9C8F-0B3222281C39@baidu.com>
Date: Tue, 20 Jun 2023 10:37:27 +0000
From: "Duan,Muquan" <duanmuquan@...du.com>
To: Eric Dumazet <edumazet@...gle.com>
CC: "davem@...emloft.net" <davem@...emloft.net>, "dsahern@...nel.org"
	<dsahern@...nel.org>, "kuba@...nel.org" <kuba@...nel.org>,
	"pabeni@...hat.com" <pabeni@...hat.com>, "netdev@...r.kernel.org"
	<netdev@...r.kernel.org>, "linux-kernel@...r.kernel.org"
	<linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v2] tcp: fix connection reset due to tw hashdance race.

Hi, Eric,

Thanks for your reply!


>> Why not speak of the FIN:
>> For current implementation, hashdance can be done on state FIN_WAIT2,  it may race with the ehash lookup process of  passive closer’s FIN. My new patch 3 does the tw hashdance until receiving passive closer's FIN(real TIME_WAIT),  so this race does not exist and  the 'connection refused' issue will not occur, so I did not speak of the FIN again with the new patch.
>> 
> shdance begins, the FIN may be dropped in further process if the sock
> is destroyed on another CPU after hashdance.
   
With my patch 3, passive closer’s FIN will find original sock because hashdance will not be done before receiving it,
After hashdance, the tw sock’s state is set to TIME_WAIT already, it can accept new SYN with the sampe 4-tuples. 
If the original sock is destroyed on another CPU or the FIN is droped after hashdance,  it will not affect the tw sock.
I don’t know whether I get your point correctly?


>> 
>> I took a look at FreeBSD, it uses hash table lock and per sock level lock.It also needs some tricks to retry for some cases, for example, sock dropped by another thread when waiting for per sock lock during the lookup:
>>   /*
>>     * While waiting for inp lock during the lookup, another thread
>>     * can have dropped the inpcb, in which case we need to loop back
>>     * and try to find a new inpcb to deliver to.
>>     */
>>    if (inp->inp_flags & INP_DROPPED) {
>>        INP_WUNLOCK(inp);
>>        inp = NULL;
>>        goto findpcb;
>> }
>> 
> 
> This is the last time you bring FreeBSD code here.
> 
> We do not copy FreeBSD code for obvious reasons.
> I never looked at FreeBSD code and never will.
> 
> Stop this, please, or I will ignore your future emails.


I am very sorry, I will not do this again.






Regards!
Duanmuquan

> 2023年6月20日 下午4:44，Eric Dumazet <edumazet@...gle.com> 写道：
> 
> On Tue, Jun 20, 2023 at 5:30 AM Duan,Muquan <duanmuquan@...du.com> wrote:
>> 
>> Hi, Eric,
>> 
>> Thanks for your comments!
>> 
>> Why not speak of the FIN:
>> For current implementation, hashdance can be done on state FIN_WAIT2,  it may race with the ehash lookup process of  passive closer’s FIN. My new patch 3 does the tw hashdance until receiving passive closer's FIN(real TIME_WAIT),  so this race does not exist and  the 'connection refused' issue will not occur, so I did not speak of the FIN again with the new patch.
>> 
> shdance begins, the FIN may be dropped in further process if the sock
> is destroyed on another CPU after hashdance.
>> 
>> I took a look at FreeBSD, it uses hash table lock and per sock level lock.It also needs some tricks to retry for some cases, for example, sock dropped by another thread when waiting for per sock lock during the lookup:
>>   /*
>>     * While waiting for inp lock during the lookup, another thread
>>     * can have dropped the inpcb, in which case we need to loop back
>>     * and try to find a new inpcb to deliver to.
>>     */
>>    if (inp->inp_flags & INP_DROPPED) {
>>        INP_WUNLOCK(inp);
>>        inp = NULL;
>>        goto findpcb;
>> }
>> 
> 
> This is the last time you bring FreeBSD code here.
> 
> We do not copy FreeBSD code for obvious reasons.
> I never looked at FreeBSD code and never will.
> 
> Stop this, please, or I will ignore your future emails.