Re: [PATCH bpf-next 07/11] bpf: check_map_access() with the knowledge of arrays.

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

On 4/11/24 15:14, Eduard Zingerman wrote:
> On Tue, 2024-04-09 at 17:41 -0700, Kui-Feng Lee wrote:
> [...]
> 
>> Any access to elements other than the first one would be rejected.
> 
> I'm not sure this is true, could you please point me to a specific
> check in the code that enforces access to go to the first element?
> The check added in this patch only enforces correct alignment with
> array element start.

I mean accessing to elements other than the first one would be rejected
if we don't have this patch.

Before the change, it enforces correct alignment with the start of the
whole array.  Once the array feature is enabled, the "size" of struct
btf_field will be the size of entire array. In another word, accessing
to later elements, other than the first one, doesn't align with the
beginning of entire array, and will be rejected.


> 
> Other than this, the patch looks good to me.
> 
> [...]
> 
>> @@ -5448,7 +5448,10 @@ static int check_map_access(struct bpf_verifier_env *env, u32 regno,
>>   					verbose(env, "kptr access cannot have variable offset\n");
>>   					return -EACCES;
>>   				}
>> -				if (p != off + reg->var_off.value) {

Here "p" is the start of the entire array. If access any elements other
than the first one, it should return -EACCES.

>> +				var_p = off + reg->var_off.value;
>> +				elem_size = field->size / field->nelems;
>> +				if (var_p < p || var_p >= p + field->size ||
>> +				    (var_p - p) % elem_size) {
>>   					verbose(env, "kptr access misaligned expected=%u off=%llu\n",
>>   						p, off + reg->var_off.value);
>>   					return -EACCES;
> 
>