Re: [RFC bpf-next v4 4/6] bpf: pin, translate, and unpin __uptr from syscalls.

[Martin KaFai Lau <martin.lau@linux.dev>]

On 8/28/24 4:24 PM, Alexei Starovoitov wrote:
>> @@ -714,6 +869,11 @@ void bpf_obj_free_fields(const struct btf_record *rec, void *obj)
>>                                  field->kptr.dtor(xchgd_field);
>>                          }
>>                          break;
>> +               case BPF_UPTR:
>> +                       if (*(void **)field_ptr)
>> +                               bpf_obj_unpin_uptr(field, *(void **)field_ptr);
>> +                       *(void **)field_ptr = NULL;
> This one will be called from
>   task_storage_delete->bpf_selem_free->bpf_obj_free_fields
> 
> and even if upin was safe to do from that context
> we cannot just do:
> *(void **)field_ptr = NULL;
> 
> since bpf prog might be running in parallel,
> it could have just read that addr and now is using it.
> 
> The first thought of a way to fix this was to split
> bpf_obj_free_fields() into the current one plus
> bpf_obj_free_fields_after_gp()
> that will do the above unpin bit.
> and call the later one from bpf_selem_free_rcu()
> while bpf_obj_free_fields() from bpf_selem_free()
> will not touch uptr.
> 
> But after digging further I realized that task_storage
> already switched to use bpf_ma, so the above won't work.
> 
> So we need something similar to BPF_KPTR_REF logic:
> xchgd_field = (void *)xchg((unsigned long *)field_ptr, 0);
> and then delay of uptr unpin for that address into call_rcu.
> 
> Any better ideas?

Many thanks to Kui-Feng starting this useful work on task storage. I will think 
about it and respin the set.