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

[Kui-Feng Lee <sinquersw@gmail.com>]

On 2/2/24 16:52, Martin KaFai Lau wrote:
> 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.


Totally agree!


> 
>>
>> 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);
>