Re: [RFC bpf-next v4 2/6] bpf: Extend PTR_TO_BTF_ID to handle pointers to scalar and array types.

[Martin KaFai Lau <martin.lau@linux.dev>]

On 2/2/24 2:05 PM, thinker.li@gmail.com wrote:
> From: Kui-Feng Lee <thinker.li@gmail.com>
> 
> The verifier calls btf_struct_access() to check the access for
> PTR_TO_BTF_ID. btf_struct_access() supported only pointer to struct types
> (including union). We add the support of scalar types and array types.
> 
> btf_reloc_array_access() is responsible for relocating the access from the
> whole array to an element in the array. That means to adjust the offset
> relatively to the start of an element and change the type to the type of
> the element. With this relocation, we can check the access against the
> element type instead of the array type itself.
> 
> After relocation, the struct types, including union types, will continue
> the loop of btf_struct_walk(). Other types are treated as scalar types,
> including pointers, and return from btf_struct_access().

Unless there is an immediate use case to support PTR_MAYBE_NULL on a non-struct 
pointer, I would suggest to separate the other pointer type support from the 
current PTR_MAYBE_NULL feature patchset. afaik, they are orthogonal.

> 
> Signed-off-by: Kui-Feng Lee <thinker.li@gmail.com>
> ---
>   kernel/bpf/btf.c | 61 ++++++++++++++++++++++++++++++++++++++++++++++++
>   1 file changed, 61 insertions(+)
> 
> diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
> index 0847035bba99..d3f94d04c69d 100644
> --- a/kernel/bpf/btf.c
> +++ b/kernel/bpf/btf.c
> @@ -6590,6 +6590,61 @@ static int btf_struct_walk(struct bpf_verifier_log *log, const struct btf *btf,
>   	return -EINVAL;
>   }
>   
> +/* Relocate the access relatively to the beginning of an element in an
> + * array.
> + *
> + * The offset is adjusted relatively to the beginning of the element and the
> + * type is adjusted to the type of the element.
> + *
> + * Return NULL for scalar, enum, and pointer type.
> + * Return a btf_type pointer for struct and union.
> + */
> +static const struct btf_type *
> +btf_reloc_array_access(struct bpf_verifier_log *log, const struct btf *btf,
> +		       const struct btf_type *t, int *off, int size)
> +{
> +	const struct btf_type *rt, *elem_type;
> +	u32 rt_size, elem_id, total_nelems, rt_id, elem_size;
> +	u32 elem_idx;
> +
> +	rt = __btf_resolve_size(btf, t, &rt_size, &elem_type, &elem_id,
> +				&total_nelems, &rt_id);
> +	if (IS_ERR(rt))
> +		return rt;
> +	if (btf_type_is_array(rt)) {
> +		if (*off >= rt_size) {
> +			bpf_log(log, "access out of range of type %s with offset %d and size %u\n",
> +				__btf_name_by_offset(btf, t->name_off), *off, rt_size);
> +			return ERR_PTR(-EACCES);
> +		}
> +
> +		/* Multi-dimensional arrays are flattened by
> +		 * __btf_resolve_size(). Check the comment in
> +		 * btf_struct_walk().
> +		 */
> +		elem_size = rt_size / total_nelems;
> +		elem_idx = *off / elem_size;
> +		/* Relocate the offset relatively to the start of the
> +		 * element at elem_idx.
> +		 */
> +		*off -= elem_idx * elem_size;
> +		rt = elem_type;
> +		rt_size = elem_size;
> +	}
> +
> +	if (btf_type_is_struct(rt))
> +		return rt;
> +
> +	if (*off + size > rt_size) {
> +		bpf_log(log, "access beyond the range of type %s with offset %d and size %d\n",
> +			__btf_name_by_offset(btf, rt->name_off), *off, size);
> +		return ERR_PTR(-EACCES);
> +	}
> +
> +	/* The access is accepted as a scalar. */
> +	return NULL;
> +}
> +
>   int btf_struct_access(struct bpf_verifier_log *log,
>   		      const struct bpf_reg_state *reg,
>   		      int off, int size, enum bpf_access_type atype __maybe_unused,
> @@ -6625,6 +6680,12 @@ int btf_struct_access(struct bpf_verifier_log *log,
>   	}
>   
>   	t = btf_type_by_id(btf, id);
> +	t = btf_reloc_array_access(log, btf, t, &off, size);
> +	if (IS_ERR(t))
> +		return PTR_ERR(t);
> +	if (!t)
> +		return SCALAR_VALUE;
> +
>   	do {
>   		err = btf_struct_walk(log, btf, t, off, size, &id, &tmp_flag, field_name);
>