netdev - Re: [PATCH v2 bpf 2/2] bpf: net: Avoid incorrect bpf_sk_reuseport

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: <20200709184729.jg2y2g6crxlazlqo@kafai-mbp>
Date:   Thu, 9 Jul 2020 11:47:29 -0700
From:   Martin KaFai Lau <kafai@...com>
To:     James Chapman <jchapman@...alix.com>
CC:     <bpf@...r.kernel.org>, Alexei Starovoitov <ast@...nel.org>,
        Daniel Borkmann <daniel@...earbox.net>, <kernel-team@...com>,
        <netdev@...r.kernel.org>
Subject: Re: [PATCH v2 bpf 2/2] bpf: net: Avoid incorrect
 bpf_sk_reuseport_detach call

On Thu, Jul 09, 2020 at 11:58:33AM +0100, James Chapman wrote:
> On  Wed, Jul 08, 2020 at 23:11:10 -0700, Martin KaFai Lau wrote:
> > bpf_sk_reuseport_detach is currently called when sk->sk_user_data
> > is not NULL.  It is incorrect because sk->sk_user_data may not be
> > managed by the bpf's reuseport_array.  It has been reported in [1] that,
> > the bpf_sk_reuseport_detach() which is called from udp_lib_unhash() has
> > corrupted the sk_user_data managed by l2tp.
> > 
> > This patch solves it by using another bit (defined as SK_USER_DATA_BPF)
> > of the sk_user_data pointer value.  It marks that a sk_user_data is
> > managed/owned by BPF.
> 
> I have reservations about using a bit in sk_user_data to indicate
> ownership of that pointer. But putting that aside, I confirm that the
> patch fixes the problem.
> 
> Acked-by: James Chapman <jchapman@...alix.com>
> Tested-by: James Chapman <jchapman@...alix.com>
> Reported-by: syzbot+9f092552ba9a5efca5df@...kaller.appspotmail.com
Thanks for the test!

One bit	of sk_user_data	has already been used to indicate SK_USER_DATA_NOCOPY.
I think using another bit is the cleanest fix for the bpf/net branch instead
of tracking this somewhere else.