On 10/19/22 8:48 PM, Kumar Kartikeya Dwivedi wrote:
> On Wed, Oct 19, 2022 at 10:45:22PM IST, Dave Marchevsky wrote:
>> On 10/13/22 2:22 AM, Kumar Kartikeya Dwivedi wrote:
>>> Introduce the idea of local kptrs, i.e. PTR_TO_BTF_ID that point to a
>>> type in program BTF. This is indicated by the presence of MEM_TYPE_LOCAL
>>> type tag in reg->type to avoid having to check btf_is_kernel when trying
>>> to match argument types in helpers.
>>>
>>> For now, these local kptrs will always be referenced in verifier
>>> context, hence ref_obj_id == 0 for them is a bug. It is allowed to write
>>> to such objects, as long fields that are special are not touched
>>> (support for which will be added in subsequent patches).
>>>
>>> No PROBE_MEM handling is hence done since they can never be in an
>>> undefined state, and their lifetime will always be valid.
>>>
>>> Signed-off-by: Kumar Kartikeya Dwivedi <memxor@xxxxxxxxx>
>>> ---
[...]
>>
>>
>> more re: passing entire reg state to btf_struct access:
>>
>> In the next patch in the series ("bpf: Recognize bpf_{spin_lock,list_head,list_node} in local kptrs")
>> you do btf_find_struct_meta(btf, reg->btf_id). I see why you couldn't use 't'
>> that's passed in here / elsewhere since you need the btf_id for meta lookup.
>> Perhaps 'btf_type *t' param can be changed to btf_id, eliminating the need
>> to pass 'reg'.
>>
>> Alternatively, since we're already passing reg->btf and result of
>> btf_type_by_id(reg->btf, reg->btf_id), seems like btf_struct_access
>> maybe is tied closely enough to reg state that passing reg state
>> directly and getting rid of extraneous args is cleaner.
>>
>
> So Alexei actually suggested dropping both btf and type arguments and simply
> pass in the register and get it from there.
>
> But one call site threw a wrench in the plan:
>
> check_ptr_to_map_access -> btf_struct_access
>
> Here, it passes it's own btf and type to simulate access to a map. Maybe I
> should be creating a dummy register on stack and make it work like that for this
> particular case? Otherwise all other callers pass in what they have from reg.
Ah, sorry for missing that. Personally I'm not a fan of dummy register on the
stack. Then if btf_struct_access starts using some reg state that wasn't
populated in the dummy reg it will be confusing.
