netdev - Re: [PATCH net] l2tp: add sk_reuseport checks to l2tp_validate

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]

Date:   Wed, 8 Jul 2020 11:23:06 +0100
From:   James Chapman <jchapman@...alix.com>
To:     Martin KaFai Lau <kafai@...com>
Cc:     David Miller <davem@...emloft.net>, netdev@...r.kernel.org,
        gnault@...hat.com
Subject: Re: [PATCH net] l2tp: add sk_reuseport checks to l2tp_validate_socket

On  Tue, Jul 07, 2020 at 11:31:28 -0700, Martin KaFai Lau wrote:
> On Mon, Jul 06, 2020 at 12:45:36PM -0700, David Miller wrote:
> > From: James Chapman <jchapman@...alix.com>
> > Date: Mon, 6 Jul 2020 13:12:59 +0100
> > 
> > > The crash occurs in the socket destroy path. bpf_sk_reuseport_detach
> > > assumes ownership of sk_user_data if sk_reuseport is set and writes a
> > > NULL pointer to the memory pointed to by
> > > sk_user_data. bpf_sk_reuseport_detach is called via
> > > udp_lib_unhash. l2tp does its socket cleanup through sk_destruct,
> > > which fetches private data through sk_user_data. The BUG_ON fires
> > > because this data has been corrupted.
> > 
> > The ownership of sk_user_data has to be handled more cleanly.
> > 
> > BPF really has no business taking over this as it is for the protocols
> > to use and what L2TP is doing is quite natural and normal.  Exactly
> > what sk_user_data was designed to be used for.
> > 
> > I'm not applying this, please take this up with the BPF folks.  They
> > need to store their metadata elsewhere.
> Thanks for the report.
> 
> The sk_user_data is used when a sk is added to the bpf's reuseport_sockarray.
> Before it can be added, the bpf side does check if the sk_user_data has already been
> used or not.  It is the similar check like other usages on sk_user_data.
> 
> The missing part is the reuseport_detach_sock() should check if a
> sk is currently in a reuseport_sockarray before calling bpf_sk_reuseport_detach().
> It can be solved by remembering if a sk is added to the reuseport_sockarray.
> I will work on a fix by doing this.

Thanks Martin.