Re: [PATCH bpf-next v7 08/26] bpf: Introduce allocated objects support

[Kumar Kartikeya Dwivedi <memxor@gmail.com>]

On Tue, Nov 15, 2022 at 11:18:18AM IST, Alexei Starovoitov wrote:
> On Tue, Nov 15, 2022 at 12:45:29AM +0530, Kumar Kartikeya Dwivedi wrote:
> > Introduce support for representing pointers to objects allocated by the
> > BPF program, i.e. PTR_TO_BTF_ID that point to a type in program BTF.
> > This is indicated by the presence of MEM_ALLOC type flag in reg->type to
> > avoid having to check btf_is_kernel when trying to match argument types
> > in helpers.
> >
> > Whenever walking such types, any pointers being walked will always yield
> > a SCALAR instead of pointer. In the future we might permit kptr inside
> > such allocated objects (either kernel or local), and it will then form a
>
> (either kernel or program allocated) ?
>
> > PTR_TO_BTF_ID of the respective type.
> >
> > For now, such allocated objects 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). Note that once
> > such a pointer is marked PTR_UNTRUSTED, it is no longer allowed to write
> > to it.
> >
> > No PROBE_MEM handling is therefore done for loads into this type unless
> > PTR_UNTRUSTED is part of the register type, 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>
> > ---
> >  include/linux/bpf.h   | 11 +++++++++++
> >  kernel/bpf/btf.c      |  5 +++++
> >  kernel/bpf/verifier.c | 25 +++++++++++++++++++++++--
> >  3 files changed, 39 insertions(+), 2 deletions(-)
> >
> > diff --git a/include/linux/bpf.h b/include/linux/bpf.h
> > index 49f9d2bec401..3cab113b149e 100644
> > --- a/include/linux/bpf.h
> > +++ b/include/linux/bpf.h
> > @@ -524,6 +524,11 @@ enum bpf_type_flag {
> >  	/* Size is known at compile time. */
> >  	MEM_FIXED_SIZE		= BIT(10 + BPF_BASE_TYPE_BITS),
> >
> > +	/* MEM is of a an allocated object of type from program BTF. This is
> > +	 * used to tag PTR_TO_BTF_ID allocated using bpf_obj_new.
> > +	 */
> > +	MEM_ALLOC		= BIT(11 + BPF_BASE_TYPE_BITS),
> > +
> >  	__BPF_TYPE_FLAG_MAX,
> >  	__BPF_TYPE_LAST_FLAG	= __BPF_TYPE_FLAG_MAX - 1,
> >  };
> > @@ -2791,4 +2796,10 @@ struct bpf_key {
> >  	bool has_ref;
> >  };
> >  #endif /* CONFIG_KEYS */
> > +
> > +static inline bool type_is_alloc(u32 type)
> > +{
> > +	return type & MEM_ALLOC;
> > +}
> > +
> >  #endif /* _LINUX_BPF_H */
> > diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
> > index 875355ff3718..9a596f430558 100644
> > --- a/kernel/bpf/btf.c
> > +++ b/kernel/bpf/btf.c
> > @@ -6034,6 +6034,11 @@ int btf_struct_access(struct bpf_verifier_log *log,
> >
> >  		switch (err) {
> >  		case WALK_PTR:
> > +			/* For local types, the destination register cannot
> > +			 * become a pointer again.
> > +			 */
> > +			if (type_is_alloc(reg->type))
> > +				return SCALAR_VALUE;
> >  			/* If we found the pointer or scalar on t+off,
> >  			 * we're done.
> >  			 */
> > diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> > index 5e74f460dfd0..d726d55622c9 100644
> > --- a/kernel/bpf/verifier.c
> > +++ b/kernel/bpf/verifier.c
> > @@ -4687,14 +4687,27 @@ static int check_ptr_to_btf_access(struct bpf_verifier_env *env,
> >  		return -EACCES;
> >  	}
> >
> > -	if (env->ops->btf_struct_access) {
> > +	if (env->ops->btf_struct_access && !type_is_alloc(reg->type)) {
> > +		if (!btf_is_kernel(reg->btf)) {
> > +			verbose(env, "verifier internal error: reg->btf must be kernel btf\n");
> > +			return -EFAULT;
> > +		}
> >  		ret = env->ops->btf_struct_access(&env->log, reg, off, size, atype, &btf_id, &flag);
> >  	} else {
> > -		if (atype != BPF_READ) {
> > +		/* Writes are permitted with default btf_struct_access for local
> > +		 * kptrs (which always have ref_obj_id > 0), but not for
>
> for program allocated objects (which always have ref_obj_id > 0) ?
>

Ack for both.