From: Paul Moore <paul@paul-moore.com> To: linux-audit@redhat.com, bpf@vger.kernel.org Cc: Alexei Starovoitov <ast@kernel.org>, Burn Alting <burn.alting@iinet.net.au>, Stanislav Fomichev <sdf@google.com> Subject: Re: [PATCH v2] bpf: restore the ebpf program ID for BPF_AUDIT_UNLOAD and PERF_BPF_EVENT_PROG_UNLOAD Date: Fri, 23 Dec 2022 16:26:43 -0500 [thread overview] Message-ID: <CAHC9VhQmLcLYxxmDgo9ygmcanuyGVY_A=y2z6rtGdMcwwA4rDw@mail.gmail.com> (raw) In-Reply-To: <20221223185531.222689-1-paul@paul-moore.com> On Fri, Dec 23, 2022 at 1:55 PM Paul Moore <paul@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.alting@iinet.net.au> > Reported-by: Jiri Olsa <olsajiri@gmail.com> > Signed-off-by: Paul Moore <paul@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
WARNING: multiple messages have this Message-ID (diff)
From: Paul Moore <paul@paul-moore.com> To: linux-audit@redhat.com, bpf@vger.kernel.org Cc: Burn Alting <burn.alting@iinet.net.au>, Alexei Starovoitov <ast@kernel.org>, Stanislav Fomichev <sdf@google.com> Subject: Re: [PATCH v2] bpf: restore the ebpf program ID for BPF_AUDIT_UNLOAD and PERF_BPF_EVENT_PROG_UNLOAD Date: Fri, 23 Dec 2022 16:26:43 -0500 [thread overview] Message-ID: <CAHC9VhQmLcLYxxmDgo9ygmcanuyGVY_A=y2z6rtGdMcwwA4rDw@mail.gmail.com> (raw) In-Reply-To: <20221223185531.222689-1-paul@paul-moore.com> On Fri, Dec 23, 2022 at 1:55 PM Paul Moore <paul@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.alting@iinet.net.au> > Reported-by: Jiri Olsa <olsajiri@gmail.com> > Signed-off-by: Paul Moore <paul@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
next prev parent reply other threads:[~2022-12-23 21:27 UTC|newest] Thread overview: 37+ messages / expand[flat|nested] mbox.gz Atom feed top 2022-12-23 18:55 [PATCH v2] bpf: restore the ebpf program ID for BPF_AUDIT_UNLOAD and PERF_BPF_EVENT_PROG_UNLOAD Paul Moore 2022-12-23 18:55 ` Paul Moore 2022-12-23 21:14 ` kernel test robot 2022-12-23 21:14 ` kernel test robot 2022-12-23 21:26 ` Paul Moore [this message] 2022-12-23 21:26 ` Paul Moore 2022-12-23 21:44 ` kernel test robot 2022-12-23 21:44 ` kernel test robot 2022-12-24 1:49 ` Stanislav Fomichev 2022-12-24 1:49 ` Stanislav Fomichev 2022-12-24 15:31 ` Paul Moore 2022-12-24 15:31 ` Paul Moore 2022-12-25 22:16 ` Alexei Starovoitov 2022-12-25 22:16 ` Alexei Starovoitov 2022-12-27 3:35 ` Stanislav Fomichev 2022-12-27 3:35 ` Stanislav Fomichev 2022-12-27 16:40 ` Paul Moore 2022-12-27 16:40 ` Paul Moore 2022-12-30 2:13 ` Stanislav Fomichev 2022-12-30 2:13 ` Stanislav Fomichev 2022-12-30 3:10 ` Alexei Starovoitov 2022-12-30 3:10 ` Alexei Starovoitov 2022-12-30 3:38 ` Stanislav Fomichev 2022-12-30 3:38 ` Stanislav Fomichev 2022-12-30 4:18 ` Alexei Starovoitov 2022-12-30 4:18 ` Alexei Starovoitov 2022-12-27 17:49 ` Alexei Starovoitov 2022-12-27 17:49 ` Alexei Starovoitov 2022-12-28 0:25 ` Stanislav Fomichev 2022-12-28 0:25 ` Stanislav Fomichev 2022-12-25 14:13 ` Jiri Olsa 2022-12-25 14:13 ` Jiri Olsa 2022-12-25 19:14 ` Paul Moore 2022-12-25 19:14 ` Paul Moore 2023-01-03 0:33 ` kernel test robot 2023-01-03 0:33 ` kernel test robot 2023-01-03 18:12 Bryce
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to='CAHC9VhQmLcLYxxmDgo9ygmcanuyGVY_A=y2z6rtGdMcwwA4rDw@mail.gmail.com' \ --to=paul@paul-moore.com \ --cc=ast@kernel.org \ --cc=bpf@vger.kernel.org \ --cc=burn.alting@iinet.net.au \ --cc=linux-audit@redhat.com \ --cc=sdf@google.com \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.