netdev - Re: [net-next] tcp: Avoid sending an erroneous RST due to RCU 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] [thread-next>] [day] [month] [year] [list]

Message-ID: <CANn89iLn0VK2kfq2m56kcaLGE6U-=p5eOr8=EFUqTr5bkONDiA@mail.gmail.com>
Date: Tue, 9 Jan 2024 11:36:13 +0100
From: Eric Dumazet <edumazet@...gle.com>
To: Paolo Abeni <pabeni@...hat.com>
Cc: Jon Maxwell <jmaxwell37@...il.com>, davem@...emloft.net, kuba@...nel.org, 
	dsahern@...nel.org, netdev@...r.kernel.org, 
	Davide Caratti <dcaratti@...hat.com>
Subject: Re: [net-next] tcp: Avoid sending an erroneous RST due to RCU race

On Tue, Jan 9, 2024 at 8:24 AM Paolo Abeni <pabeni@...hat.com> wrote:
>
> On Tue, 2024-01-09 at 10:13 +1100, Jon Maxwell wrote:
> > There are 2 cases where a socket lookup races due to RCU and finds a
> > LISTEN socket instead of an ESTABLISHED or a TIME-WAIT socket. As the ACK flag
> > is set this will generate an erroneous RST.
> >
> > There are 2 scenarios, one where 2 ACKs (one for the 3 way handshake and
> > another with the same sequence number carrying data) are sent with a very
> > small time interval between them. In this case the 2 ACKs can race while being
> > processed on different CPUs and the latter may find the LISTEN socket instead
> > of the ESTABLISHED socket. That will make the one end of the TCP connection
> > out of sync with the other and cause a break in communications. The other
> > scenario is a "FIN ACK" racing with an ACK which may also find the LISTEN
> > socket instead of the TIME_WAIT socket. Instead of getting ignored that
> > generates an invalid RST.
> >
> > Instead of the next connection attempt succeeding. The client then gets an
> > ECONNREFUSED error on the next connection attempt when it finds a socket in
> > the FIN_WAIT_2 state as discussed here:
> >
> > https://lore.kernel.org/netdev/20230606064306.9192-1-duanmuquan@baidu.com/
> >
> > Modeled on Erics idea, introduce __inet_lookup_skb_locked() and
> > __inet6_lookup_skb_locked()  to fix this by doing a locked lookup only for
> > these rare cases to avoid finding the LISTEN socket.
>
> I think Eric's idea was to keep the bucket lock held after such lookup,
> to avoid possibly re-acquiring it for time-wait sockets.

Yes, I think a real fix needs more work/refactoring, I can work on
this in the next cycle.