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@xxxxxxxxxx>
>>
>> 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@xxxxxxxxxx>
>
> Acked-by: Yonghong Song <yhs@xxxxxx>
>
>> ---
>> 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);
>> }
>>
