netdev - Re: [PATCH bpf] bpf: Use hlist_add_head_rcu when linking to sk

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: <20200917174739.wbwiayb66aemydc5@ast-mbp.dhcp.thefacebook.com>
Date:   Thu, 17 Sep 2020 10:47:39 -0700
From:   Alexei Starovoitov <alexei.starovoitov@...il.com>
To:     Martin KaFai Lau <kafai@...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 bpf] bpf: Use hlist_add_head_rcu when linking to
 sk_storage

On Wed, Sep 16, 2020 at 01:09:25PM -0700, Martin KaFai Lau wrote:
> The sk_storage->list will be traversed by rcu reader in parallel.
> Thus, hlist_add_head_rcu() is needed in __selem_link_sk().  This
> patch fixes it.
> 
> This part of the code has recently been refactored in bpf-next.
> A separate fix will be provided for the bpf-next tree.
> 
> Fixes: 6ac99e8f23d4 ("bpf: Introduce bpf sk local storage")
> Signed-off-by: Martin KaFai Lau <kafai@...com>
> ---
>  net/core/bpf_sk_storage.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/net/core/bpf_sk_storage.c b/net/core/bpf_sk_storage.c
> index b988f48153a4..d4d2a56e9d4a 100644
> --- a/net/core/bpf_sk_storage.c
> +++ b/net/core/bpf_sk_storage.c
> @@ -219,7 +219,7 @@ static void __selem_link_sk(struct bpf_sk_storage *sk_storage,
>  			    struct bpf_sk_storage_elem *selem)
>  {
>  	RCU_INIT_POINTER(selem->sk_storage, sk_storage);
> -	hlist_add_head(&selem->snode, &sk_storage->list);
> +	hlist_add_head_rcu(&selem->snode, &sk_storage->list);
>  }

Applying the same, yet very different from git point of view, patch to
bpf and bpf-next trees will create a ton of confusion for everyone.
I prefer to take this fix (in bpf-next form) into bpf-next only and apply
this fix (in bpf form) to 5.9 and stable after the merge window.
The code has been around since April 2019 and it wasn't hit in prod,
so I don't think there is urgency.
Agree?