netdev - Re: [PATCH net] tcp: prevent concurrent execution of tcp_sk_exit

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: <20240812235259.GA6030@breakpoint.cc>
Date: Tue, 13 Aug 2024 01:52:59 +0200
From: Florian Westphal <fw@...len.de>
To: Kuniyuki Iwashima <kuniyu@...zon.com>
Cc: fw@...len.de, davem@...emloft.net, edumazet@...gle.com,
	kerneljasonxing@...il.com, kuba@...nel.org, netdev@...r.kernel.org,
	pabeni@...hat.com,
	syzbot+8ea26396ff85d23a8929@...kaller.appspotmail.com
Subject: Re: [PATCH net] tcp: prevent concurrent execution of
 tcp_sk_exit_batch

Kuniyuki Iwashima <kuniyu@...zon.com> wrote:
> > ... because refcount_dec() of tw_refcount unexpectedly dropped to 0.
> > 
> > This doesn't seem like an actual bug (no tw sockets got lost and I don't
> > see a use-after-free) but as erroneous trigger of debug check.
> 
> I guess the reason you don't move the check back to tcp_sk_exit() is

No, it would not work. .exit runs before .exit_batch, we'd splat.

Before e9bd0cca09d1 ordering doesn't matter because
refcount_dec_and_test is used consistently, so it was not relevant
if the 0-transition occured from .exit or later via inet_twsk_kill.

> to catch a potential issue explained in solution 4 in the link, right ?

Yes, it would help to catch such issue wrt. twsk lifetime.
Having the WARN ensures no twsk can escape .exit_batch completion.

E.g. if twsk destructors in the future refer to some other
object that has to be released before netns pointers become invalid
or something like that.

Does that make sense?  Otherwise I'm open to alternative approaches.
Or we can wait until syzbot finds a reproducer, I don't think its
a real/urgent bug.