netdev - Re: [Patch net 2/2] l2tp: close all race conditions in l2tp_tunnel

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 for Android: free password hash cracker in your pocket

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

Message-ID: <Y7nNdx1yDoEEPrwY@pop-os.localdomain>
Date:   Sat, 7 Jan 2023 11:52:23 -0800
From:   Cong Wang <xiyou.wangcong@...il.com>
To:     Saeed Mahameed <saeed@...nel.org>
Cc:     netdev@...r.kernel.org, g.nault@...halink.fr,
        Cong Wang <cong.wang@...edance.com>,
        syzbot+52866e24647f9a23403f@...kaller.appspotmail.com,
        syzbot+94cc2a66fc228b23f360@...kaller.appspotmail.com,
        Tetsuo Handa <penguin-kernel@...ove.sakura.ne.jp>,
        Jakub Sitnicki <jakub@...udflare.com>,
        Eric Dumazet <edumazet@...gle.com>
Subject: Re: [Patch net 2/2] l2tp: close all race conditions in
 l2tp_tunnel_register()

On Sat, Jan 07, 2023 at 10:41:41AM -0800, Saeed Mahameed wrote:
> On 05 Jan 11:13, Cong Wang wrote:
> > From: Cong Wang <cong.wang@...edance.com>
> > 
> > The code in l2tp_tunnel_register() is racy in several ways:
> > 
> > 1. It modifies the tunnel socket _after_ publishing it.
> > 
> > 2. It calls setup_udp_tunnel_sock() on an existing socket without
> >   locking.
> > 
> > 3. It changes sock lock class on fly, which triggers many syzbot
> >   reports.
> > 
> > This patch amends all of them by moving socket initialization code
> > before publishing and under sock lock. As suggested by Jakub, the
> > l2tp lockdep class is not necessary as we can just switch to
> > bh_lock_sock_nested().
> > 
> > Fixes: 37159ef2c1ae ("l2tp: fix a lockdep splat")
> > Fixes: 6b9f34239b00 ("l2tp: fix races in tunnel creation")
> 
> This patch relies on the previous one which doesn't include any tags.
> If you are interested in this making it to -stable then maybe you need to
> add those tags to the previous commit ?
> 

But technically patch 1/2 does not fix anything alone, this is why I
heisitate to add any Fixes tag to it.

Since this is a patchset, I think maintainers can easily figure out this
is a whole set.

Thanks.