On Fri, Dec 23, 2022 at 1:55 PM Paul Moore <p...@paul-moore.com> wrote: > > When changing the ebpf program put() routines to support being called > from within IRQ context the program ID was reset to zero prior to > calling the perf event and audit UNLOAD record generators, which > resulted in problems as the ebpf program ID was bogus (always zero). > This patch resolves this by adding a new flag, bpf_prog::valid_id, to > indicate when the bpf_prog_aux ID field is valid; it is set to true/1 > in bpf_prog_alloc_id() and set to false/0 in bpf_prog_free_id(). In > order to help ensure that access to the bpf_prog_aux ID field takes > into account the new valid_id flag, the bpf_prog_aux ID field is > renamed to bpf_prog_aux::__id and a getter function, > bpf_prog_get_id(), was created and all users of bpf_prog_aux::id were > converted to the new caller. Exceptions to this include some of the > internal ebpf functions and the xdp trace points, although the latter > still take into account the valid_id flag. > > I also modified the bpf_audit_prog() logic used to associate the > AUDIT_BPF record with other associated records, e.g. @ctx != NULL. > Instead of keying off the operation, it now keys off the execution > context, e.g. '!in_irg && !irqs_disabled()', which is much more > appropriate and should help better connect the UNLOAD operations with > the associated audit state (other audit records). > > Fixes: d809e134be7a ("bpf: Prepare bpf_prog_put() to be called from irq > context.") > Reported-by: Burn Alting <burn.alt...@iinet.net.au> > Reported-by: Jiri Olsa <olsaj...@gmail.com> > Signed-off-by: Paul Moore <p...@paul-moore.com> > > -- > * v2 > - change subj > - add mention of the perf regression > - drop the dedicated program audit ID > - add the bpf_prog::valid_id flag, bpf_prog_get_id() getter > - convert prog ID users to new ID getter > * v1 > - subj was: "bpf: restore the ebpf audit UNLOAD id field" > - initial draft > --- > drivers/net/netdevsim/bpf.c | 6 ++++-- > include/linux/bpf.h | 11 +++++++++-- > include/linux/bpf_verifier.h | 2 +- > include/trace/events/xdp.h | 4 ++-- > kernel/bpf/arraymap.c | 2 +- > kernel/bpf/bpf_struct_ops.c | 2 +- > kernel/bpf/cgroup.c | 2 +- > kernel/bpf/core.c | 2 +- > kernel/bpf/cpumap.c | 2 +- > kernel/bpf/devmap.c | 2 +- > kernel/bpf/syscall.c | 27 +++++++++++++++------------ > kernel/events/core.c | 6 +++++- > kernel/trace/bpf_trace.c | 2 +- > net/core/dev.c | 2 +- > net/core/filter.c | 3 ++- > net/core/rtnetlink.c | 2 +- > net/core/sock_map.c | 2 +- > net/ipv6/seg6_local.c | 3 ++- > net/sched/act_bpf.c | 2 +- > net/sched/cls_bpf.c | 2 +- > 20 files changed, 52 insertions(+), 34 deletions(-)
... > diff --git a/include/linux/bpf.h b/include/linux/bpf.h > index 9e7d46d16032..18e965bd7db9 100644 > --- a/include/linux/bpf.h > +++ b/include/linux/bpf.h > @@ -1688,6 +1689,12 @@ void bpf_prog_inc(struct bpf_prog *prog); > struct bpf_prog * __must_check bpf_prog_inc_not_zero(struct bpf_prog *prog); > void bpf_prog_put(struct bpf_prog *prog); > > +static inline u32 bpf_prog_get_id(const struct bpf_prog *prog) > +{ > + if (WARN(!prog->valid_id, "Attempting to use an invalid eBPF > program")) > + return 0; > + return prog->aux->__id; > +} > void bpf_prog_free_id(struct bpf_prog *prog, bool do_idr_lock); > void bpf_map_free_id(struct bpf_map *map, bool do_idr_lock); The bpf_prog_get_id() either needs to be moved outside the `#ifdef CONFIG_BPF_SYSCALL` block, or a dummy function needs to be added when CONFIG_BPF_SYSCALL is undefined. I can fix that up easily enough, but given the time of year I'll wait a bit to see if there are any other comments before posting another revision. -- paul-moore.com -- Linux-audit mailing list Linux-audit@redhat.com https://listman.redhat.com/mailman/listinfo/linux-audit