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

[Paul Moore <paul@xxxxxxxxxxxxxx>]

Apologies for the top post, but as I mentioned in my last message in this 
thread, next week I'll post a version without the getter/checks so this 
should not be an issue.

--
paul-moore.com

On December 25, 2022 9:13:40 AM Jiri Olsa <olsajiri@xxxxxxxxx> wrote:

> On Fri, Dec 23, 2022 at 01:55:31PM -0500, Paul Moore wrote:
>
> SNIP
>
> > diff --git a/drivers/net/netdevsim/bpf.c b/drivers/net/netdevsim/bpf.c
> > index 50854265864d..2795f03f5f34 100644
> > --- a/drivers/net/netdevsim/bpf.c
> > +++ b/drivers/net/netdevsim/bpf.c
> > @@ -109,7 +109,7 @@ nsim_bpf_offload(struct netdevsim *ns, struct bpf_prog 
> > *prog, bool oldprog)
> > "bad offload state, expected offload %sto be active",
> > oldprog ? "" : "not ");
> > ns->bpf_offloaded = prog;
> > - ns->bpf_offloaded_id = prog ? prog->aux->id : 0;
> > + ns->bpf_offloaded_id = prog ? bpf_prog_get_id(prog) : 0;
> > nsim_prog_set_loaded(prog, true);
> >
> > return 0;
> > @@ -221,6 +221,7 @@ static int nsim_bpf_create_prog(struct nsim_dev *nsim_dev,
> > struct nsim_bpf_bound_prog *state;
> > char name[16];
> > int ret;
> > + u32 id;
> >
> > state = kzalloc(sizeof(*state), GFP_KERNEL);
> > if (!state)
> > @@ -239,7 +240,8 @@ static int nsim_bpf_create_prog(struct nsim_dev *nsim_dev,
> > return ret;
> > }
> >
> > - debugfs_create_u32("id", 0400, state->ddir, &prog->aux->id);
> > + id = bpf_prog_get_id(prog);
> > + debugfs_create_u32("id", 0400, state->ddir, &id);
> > debugfs_create_file("state", 0400, state->ddir,
> > &state->state, &nsim_bpf_string_fops);
> > debugfs_create_bool("loaded", 0400, state->ddir, &state->is_loaded);
> > 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
> > @@ -1102,7 +1102,7 @@ struct bpf_prog_aux {
> > u32 max_pkt_offset;
> > u32 max_tp_access;
> > u32 stack_depth;
> > - u32 id;
> > + u32 __id; /* access via bpf_prog_get_id() to check bpf_prog::valid_id */
>
> it breaks bpftool that uses
>
>  BPF_CORE_READ((struct bpf_prog *)ent, aux, id);
>
> and bpffs selftest because of preload iter object uses aux->id
>
>  kernel/bpf/preload/iterators/iterators.bpf.c
>
> it'd be great to have a solution that keep 'id' field,
> because it's probably used in many bpf programs already
>
> jirka
>
> > u32 func_cnt; /* used by non-func prog as the number of func progs */
> > u32 func_idx; /* 0 for non-func prog, the index in func array for func prog */
> > u32 attach_btf_id; /* in-kernel BTF type id to attach to */
> > @@ -1197,7 +1197,8 @@ struct bpf_prog {
> > enforce_expected_attach_type:1, /* Enforce expected_attach_type checking at 
> > attach time */
> > call_get_stack:1, /* Do we call bpf_get_stack() or bpf_get_stackid() */
> > call_get_func_ip:1, /* Do we call get_func_ip() */
> > - tstamp_type_access:1; /* Accessed __sk_buff->tstamp_type */
> > + tstamp_type_access:1, /* Accessed __sk_buff->tstamp_type */
> > + valid_id:1; /* Is bpf_prog::aux::__id valid? */
> > enum bpf_prog_type type; /* Type of BPF program */
> > enum bpf_attach_type expected_attach_type; /* For some prog types */
> > u32 len; /* Number of filter blocks */
> > @@ -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);
>
> SNIP