netdev - Re: [PATCH bpf-next v4 1/4] bpf: remove __rcu annotations from bpf_prog

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: <1ee0ed53-dcbd-ca24-94c9-54fb303f1b4c@iogearbox.net>
Date:   Wed, 29 May 2019 16:08:32 +0200
From:   Daniel Borkmann <daniel@...earbox.net>
To:     Stanislav Fomichev <sdf@...gle.com>, netdev@...r.kernel.org,
        bpf@...r.kernel.org
Cc:     davem@...emloft.net, ast@...nel.org, Roman Gushchin <guro@...com>
Subject: Re: [PATCH bpf-next v4 1/4] bpf: remove __rcu annotations from
 bpf_prog_array

On 05/28/2019 11:14 PM, Stanislav Fomichev wrote:
> Drop __rcu annotations and rcu read sections from bpf_prog_array
> helper functions. They are not needed since all existing callers
> call those helpers from the rcu update side while holding a mutex.
> This guarantees that use-after-free could not happen.
> 
> In the next patches I'll fix the callers with missing
> rcu_dereference_protected to make sparse/lockdep happy, the proper
> way to use these helpers is:
> 
> 	struct bpf_prog_array __rcu *progs = ...;
> 	struct bpf_prog_array *p;
> 
> 	mutex_lock(&mtx);
> 	p = rcu_dereference_protected(progs, lockdep_is_held(&mtx));
> 	bpf_prog_array_length(p);
> 	bpf_prog_array_copy_to_user(p, ...);
> 	bpf_prog_array_delete_safe(p, ...);
> 	bpf_prog_array_copy_info(p, ...);
> 	bpf_prog_array_copy(p, ...);
> 	bpf_prog_array_free(p);
> 	mutex_unlock(&mtx);
> 
> No functional changes! rcu_dereference_protected with lockdep_is_held
> should catch any cases where we update prog array without a mutex
> (I've looked at existing call sites and I think we hold a mutex
> everywhere).
> 
> Motivation is to fix sparse warnings:
> kernel/bpf/core.c:1803:9: warning: incorrect type in argument 1 (different address spaces)
> kernel/bpf/core.c:1803:9:    expected struct callback_head *head
> kernel/bpf/core.c:1803:9:    got struct callback_head [noderef] <asn:4> *
> kernel/bpf/core.c:1877:44: warning: incorrect type in initializer (different address spaces)
> kernel/bpf/core.c:1877:44:    expected struct bpf_prog_array_item *item
> kernel/bpf/core.c:1877:44:    got struct bpf_prog_array_item [noderef] <asn:4> *
> kernel/bpf/core.c:1901:26: warning: incorrect type in assignment (different address spaces)
> kernel/bpf/core.c:1901:26:    expected struct bpf_prog_array_item *existing
> kernel/bpf/core.c:1901:26:    got struct bpf_prog_array_item [noderef] <asn:4> *
> kernel/bpf/core.c:1935:26: warning: incorrect type in assignment (different address spaces)
> kernel/bpf/core.c:1935:26:    expected struct bpf_prog_array_item *[assigned] existing
> kernel/bpf/core.c:1935:26:    got struct bpf_prog_array_item [noderef] <asn:4> *
> 
> v2:
> * remove comment about potential race; that can't happen
>   because all callers are in rcu-update section
> 
> Cc: Roman Gushchin <guro@...com>
> Acked-by: Roman Gushchin <guro@...com>
> Signed-off-by: Stanislav Fomichev <sdf@...gle.com>

Series applied, thanks!