Re: [PATCH bpf 1/2] bpf: Fix a bpf_kptr_xchg() issue with local kptr

[Yonghong Song <yonghong.song@linux.dev>]

On 8/21/23 12:44 PM, Alexei Starovoitov wrote:
> On Mon, Aug 21, 2023 at 9:03 AM Kumar Kartikeya Dwivedi
> <memxor@gmail.com> wrote:
>>>>
>>>> The fix on its own looks ok to me, but any reason you'd not like to
>>>> delegate to map_kptr_match_type?
>>>> Just to collect kptr related type matching logic in its own place.  It
>>>> doesn't matter too much though.
>>>
>>>   From comments from Alexei in
>>>
>>> https://lore.kernel.org/bpf/20230819002907.io3iphmnuk43xblu@macbook-pro-8.dhcp.thefacebook.com/#t
>>>
>>> =====
>>> The map_kptr_match_type() should have been used for kptrs pointing to
>>> kernel objects only.
>>> But you're calling it for MEM_ALLOC object with prog's BTF...
>>> =====
>>>
>>> So looks like map_kptr_match_type() is for kptrs pointing to
>>> kernel objects only. So that is why I didn't use it.
>>>
>>
>> That function was added by me. Back then I added this check as we were
>> discussing possibly supporting such local kptr and more thought would
>> be needed about the design before just doing type matching. Also it
>> was using kernel_type_name which was later renamed as btf_type_name,
>> so as a precaution I added the btf_is_kernel check. Apart from that I
>> remember no other reason, so I think it should be ok to drop it now
>> and use it.
> 
> Agree with Kumar.
> When I said map_kptr_match_type() is only used with kernel BTF I meant
> that as code stands it was the intent of that helper.
> With MEM_ALLOC being kptr_xchg-ed it's better to share the code and
> map_kptr_match_type() should probably be adopted to work with both kernel
> and prog's BTFs.

Sounds good to me. Will use map_kptr_match_type() in v2.

> 
> And as Kumar noticed __check_ptr_off_reg() part of it is necessary for
> MEM_ALLOC too.