On 8/21/23 12:44 PM, Alexei Starovoitov wrote:
On Mon, Aug 21, 2023 at 9:03 AM Kumar Kartikeya Dwivedi
<memxor@xxxxxxxxx> 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@xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx/#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.
