Re: [PATCH v2] bpf: restore the ebpf program ID for BPF_AUDIT_UNLOAD and PERF_BPF_EVENT_PROG_UNLOAD

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.alting(a)iinet.net.au&gt; Reported-by: Jiri Olsa <olsajiri(a)gmail.com&gt; Signed-off-by: Paul Moore <paul(a)paul-moore.com&gt; -- * 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);