linux-kernel - Re: [RFC PATCH net] net: fix data-races around sk->sk_forward

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: <CANn89iKWbcjavVB-7Lwqou8n2v6oGnaE3-jzDz7n9Nj3+5yJTw@mail.gmail.com>
Date: Mon, 4 Nov 2024 16:45:11 +0100
From: Eric Dumazet <edumazet@...gle.com>
To: Wang Liang <wangliang74@...wei.com>
Cc: davem@...emloft.net, kuba@...nel.org, pabeni@...hat.com, horms@...nel.org, 
	dsahern@...nel.org, yuehaibing@...wei.com, zhangchangzhong@...wei.com, 
	netdev@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [RFC PATCH net] net: fix data-races around sk->sk_forward_alloc

On Fri, Nov 1, 2024 at 7:24 AM Wang Liang <wangliang74@...wei.com> wrote:
>
>
> 在 2024/10/31 22:08, Eric Dumazet 写道:
> > On Thu, Oct 31, 2024 at 1:06 PM Wang Liang <wangliang74@...wei.com> wrote:
> >> Syzkaller reported this warning:
> > Was this a public report ?
> Yes，I find the report here (the C repo in the url is useful):
>
> https://syzkaller.appspot.com/bug?id=3e9b62ff331dcc3a6c28c41207f3b9911828a46b
> >> [   65.568203][    C0] ------------[ cut here ]------------
> >> [   65.569339][    C0] WARNING: CPU: 0 PID: 16 at net/ipv4/af_inet.c:156 inet_sock_destruct+0x1c5/0x1e0
> >> [   65.575017][    C0] Modules linked in:
> >> [   65.575699][    C0] CPU: 0 UID: 0 PID: 16 Comm: ksoftirqd/0 Not tainted 6.12.0-rc5 #26
> >> [   ...]
> > Oh the horror, this is completely wrong and unsafe anyway.
> >
> > TCP listen path MUST be lockless, and stay lockless.
> >
> > Ask yourself : Why would a listener even hold a pktoptions in the first place ?
> >
> > Normally, each request socket can hold an ireq->pktopts (see in
> > tcp_v6_init_req())
> >
> > The skb_clone_and_charge_r() happen later in tcp_v6_syn_recv_sock()
> >
> > The correct fix is to _not_ call skb_clone_and_charge_r() for a
> > listener socket, of course, this never made _any_ sense.
> >
> > The following patch should fix both TCP  and DCCP, and as a bonus make
> > TCP SYN processing faster
> > for listeners requesting these IPV6_PKTOPTIONS things.
> Thank you very much for your suggestion and patch!
>
> However, the problem remains unsolved when I use the following patch to
> test.
>
> Because skb_clone_and_charge_r() is still called when sk_state is
> TCP_LISTEN in discard tag.
>
> So I modify the patch like this (it works after local test):

SGTM, please send a V2 then.