Hi,
On 10/9/2024 2:42 PM, Eduard Zingerman wrote:
> On Tue, 2024-10-08 at 15:11 +0800, Hou Tao wrote:
>> From: Hou Tao <houtao1@xxxxxxxxxx>
>>
>> When trying to repeat the btf fields for array of nested struct, it
>> doesn't check the remaining info_cnt. The following splat will be
>> reported when the value of ret * nelems is greater than BTF_FIELDS_MAX:
> [...]
>
>> Fix it by checking the remaining info_cnt in btf_repeat_fields() before
>> repeating the btf fields.
>>
>> Fixes: 64e8ee814819 ("bpf: look into the types of the fields of a struct type recursively.")
>> Signed-off-by: Hou Tao <houtao1@xxxxxxxxxx>
>> ---
> Acked-by: Eduard Zingerman <eddyz87@xxxxxxxxx>
Thanks for the ack.
>
>> @@ -3681,10 +3687,10 @@ static int btf_find_field_one(const struct btf *btf,
>>
>> if (ret == BTF_FIELD_IGNORE)
>> return 0;
>> - if (nelems > info_cnt)
>> + if (!info_cnt)
>> return -E2BIG;
>> if (nelems > 1) {
>> - ret = btf_repeat_fields(info, 1, nelems - 1, sz);
>> + ret = btf_repeat_fields(info, info_cnt, 1, nelems - 1, sz);
>> if (ret < 0)
>> return ret;
>> }
>
> I think the change like below (on top of yours) would work the same
> (because nelems is >= 1 at this point):
>
> - if (!info_cnt)
> - return -E2BIG;
> - if (nelems > 1) {
> - ret = btf_repeat_fields(info, info_cnt, 1, nelems - 1, sz);
> - if (ret < 0)
> - return ret;
> - }
> +
> + ret = btf_repeat_fields(info, info_cnt, 1, nelems - 1, sz);
> + if (ret < 0)
> + return ret;
>
> wdyt?
I don't think they are the same. The main reason is due to the check in
the beginning of btf_repeat_field():
/* Ensure not repeating fields that should not be repeated. */
for (i = 0; i < field_cnt; i++) {
switch (info[i].type) {
There are two cases here:
1) info_cnt == 0
Because info_cnt is 0, the found record isn't saved in info[0], the
check will be incorrect
2) nelements ==1 && info_cnt > 0
If the found record is bpf_timer or similar, btf_repeat_fields() will
return -EINVAL instead of 0.
