Re: [PATCH bpf 1/9] bpf: Acquire map uref in .init_seq_private for array map iterator

[houtao <houtao@huaweicloud.com>]

Hi,

On 8/8/2022 10:53 PM, Yonghong Song wrote:
>
>
> On 8/6/22 12:40 AM, Hou Tao wrote:
>> From: Hou Tao <houtao1@huawei.com>
>>
>> During bpf(BPF_LINK_CREATE) for BPF_TRACE_ITER, bpf_iter_attach_map()
>> has already acquired a map uref, but the uref may be released by
>> bpf_link_release() during th reading of map iterator.
>
> some wording issue:
> bpf_iter_attach_map() acquires a map uref, and the uref may be released
> before or in the middle of iterating map elements. For example, the uref
> could be released in bpf_iter_detach_map() as part of
> bpf_link_release(), or could be released in bpf_map_put_with_uref()
> as part of bpf_map_release().
Thanks, it is much better than the original commit message. Will update in v2.

Regards
Tao
>
>>
>> Alternative fix is acquiring an extra bpf_link reference just like
>> a pinned map iterator does, but it introduces unnecessary dependency
>> on bpf_link instead of bpf_map.
>>
>> So choose another fix: acquiring an extra map uref in .init_seq_private
>> for array map iterator.
>>
>> Fixes: d3cc2ab546ad ("bpf: Implement bpf iterator for array maps")
>> Signed-off-by: Hou Tao <houtao1@huawei.com>
>
> Acked-by: Yonghong Song <yhs@fb.com>
>
>> ---
>>   kernel/bpf/arraymap.c | 7 +++++++
>>   1 file changed, 7 insertions(+)
>>
>> diff --git a/kernel/bpf/arraymap.c b/kernel/bpf/arraymap.c
>> index d3e734bf8056..bf6898bb7cb8 100644
>> --- a/kernel/bpf/arraymap.c
>> +++ b/kernel/bpf/arraymap.c
>> @@ -649,6 +649,12 @@ static int bpf_iter_init_array_map(void *priv_data,
>>           seq_info->percpu_value_buf = value_buf;
>>       }
>>   +    /*
>> +     * During bpf(BPF_LINK_CREATE), bpf_iter_attach_map() has already
>> +     * acquired a map uref, but the uref may be released by
>> +     * bpf_link_release(), so acquire an extra map uref for iterator.
>> +     */
>> +    bpf_map_inc_with_uref(map);
>>       seq_info->map = map;
>>       return 0;
>>   }
>> @@ -657,6 +663,7 @@ static void bpf_iter_fini_array_map(void *priv_data)
>>   {
>>       struct bpf_iter_seq_array_map_info *seq_info = priv_data;
>>   +    bpf_map_put_with_uref(seq_info->map);
>>       kfree(seq_info->percpu_value_buf);
>>   }
>>