Re: [PATCH bpf-next v2 10/25] bpf: Introduce local kptrs

Kumar Kartikeya Dwivedi
2022-10-25T18:12:02Z

On Tue, Oct 25, 2022 at 10:02:49PM IST, Dave Marchevsky wrote:
> On 10/13/22 2:22 AM, Kumar Kartikeya Dwivedi wrote:
> > Introduce the idea of local kptrs, i.e. PTR_TO_BTF_ID that point to a
> > type in program BTF. This is indicated by the presence of MEM_TYPE_LOCAL
> > type tag in reg->type to avoid having to check btf_is_kernel when trying
> > to match argument types in helpers.
> >
> > For now, these local kptrs will always be referenced in verifier
> > context, hence ref_obj_id == 0 for them is a bug. It is allowed to write
> > to such objects, as long fields that are special are not touched
> > (support for which will be added in subsequent patches).
> >
> > No PROBE_MEM handling is hence done since they can never be in an
> > undefined state, and their lifetime will always be valid.
> >
> > Signed-off-by: Kumar Kartikeya Dwivedi <memxor@gmail.com>
> > ---
>
> One nit unrelated to the other thread we have going for this patch.
>
> [...]
>
> > diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
> > index 066984d73a8b..65f444405d9c 100644
> > --- a/kernel/bpf/btf.c
> > +++ b/kernel/bpf/btf.c
> > @@ -6019,11 +6019,13 @@ static int btf_struct_walk(struct bpf_verifier_log *log, const struct btf *btf,
> > return -EINVAL;
> > }
> >
> > -int btf_struct_access(struct bpf_verifier_log *log, const struct btf *btf,
> > +int btf_struct_access(struct bpf_verifier_log *log,
> > + const struct bpf_reg_state *reg, const struct btf *btf,
> > const struct btf_type *t, int off, int size,
> > enum bpf_access_type atype __maybe_unused,
> > u32 *next_btf_id, enum bpf_type_flag *flag)
> > {
> > + bool local_type = reg && (type_flag(reg->type) & MEM_TYPE_LOCAL);
>
> Can you add a type_is_local_kptr helper (or similar name) to reduce this
> type_flag(reg->type) & MEM_TYPE_LOCAL repetition here and elsewhere in the patch?
> Some examples of repetition in verifier.c below.
>

Good point, it was there in RFC but for some reason I decided against it. I will
include it in v3.