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. And as Kumar noticed __check_ptr_off_reg() part of it is necessary for MEM_ALLOC too.
