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 PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Wed, 13 Apr 2022 12:23:16 -0700 From: Andrii Nakryiko <andrii.nakryiko@...il.com> To: Stanislav Fomichev <sdf@...gle.com> Cc: Networking <netdev@...r.kernel.org>, bpf <bpf@...r.kernel.org>, Alexei Starovoitov <ast@...nel.org>, Daniel Borkmann <daniel@...earbox.net>, Andrii Nakryiko <andrii@...nel.org>, Martin KaFai Lau <kafai@...com> Subject: Re: [PATCH bpf-next] bpf: move rcu lock management out of BPF_PROG_RUN routines On Wed, Apr 13, 2022 at 11:33 AM Stanislav Fomichev <sdf@...gle.com> wrote: > > Commit 7d08c2c91171 ("bpf: Refactor BPF_PROG_RUN_ARRAY family of macros > into functions") switched a bunch of BPF_PROG_RUN macros to inline > routines. This changed the semantic a bit. Due to arguments expansion > of macros, it used to be: > > rcu_read_lock(); > array = rcu_dereference(cgrp->bpf.effective[atype]); > ... > > Now, with with inline routines, we have: > array_rcu = rcu_dereference(cgrp->bpf.effective[atype]); > /* array_rcu can be kfree'd here */ > rcu_read_lock(); > array = rcu_dereference(array_rcu); > So subtle difference, wow... But this open-coding of rcu_read_lock() seems very unfortunate as well. Would making BPF_PROG_RUN_ARRAY back to a macro which only does rcu lock/unlock and grabs effective array and then calls static inline function be a viable solution? #define BPF_PROG_RUN_ARRAY_CG_FLAGS(array_rcu, ctx, run_prog, ret_flags) \ ({ int ret; rcu_read_lock(); ret = __BPF_PROG_RUN_ARRAY_CG_FLAGS(rcu_dereference(array_rcu), ....); rcu_read_unlock(); ret; }) where __BPF_PROG_RUN_ARRAY_CG_FLAGS is what BPF_PROG_RUN_ARRAY_CG_FLAGS is today but with __rcu annotation dropped (and no internal rcu stuff)? > I'm assuming in practice rcu subsystem isn't fast enough to trigger > this but let's use rcu API properly: ask callers of BPF_PROG_RUN > to manage rcu themselves. > > Also, rename to lower caps to not confuse with macros. Additionally, > drop and expand BPF_PROG_CGROUP_INET_EGRESS_RUN_ARRAY. > > See [1] for more context. > > [1] https://lore.kernel.org/bpf/CAKH8qBs60fOinFdxiiQikK_q0EcVxGvNTQoWvHLEUGbgcj1UYg@mail.gmail.com/T/#u > > Cc: Martin KaFai Lau <kafai@...com> > Fixes: 7d08c2c91171 ("bpf: Refactor BPF_PROG_RUN_ARRAY family of macros into functions") > Signed-off-by: Stanislav Fomichev <sdf@...gle.com> > --- > drivers/media/rc/bpf-lirc.c | 8 +++- > include/linux/bpf.h | 70 ++++++------------------------- > kernel/bpf/cgroup.c | 84 +++++++++++++++++++++++++++++-------- > kernel/trace/bpf_trace.c | 5 ++- > 4 files changed, 90 insertions(+), 77 deletions(-) > [...]
Powered by blists - more mailing lists