Re: [PATCH v4 bpf-next 2/6] bpf: prevent deadlock from recursive bpf_task_storage_[get|delete]

On 2/22/21 11:19 PM, Andrii Nakryiko wrote:

quoted

quoted

quoted

+       bpf_task_storage_lock();
        sdata = bpf_local_storage_update(
                task, (struct bpf_local_storage_map *)map, value, map_flags);

this should probably be container_of() instead of casting

bpf_task_storage.c uses casting in multiple places. How about we fix it in a
separate patch?

Sure, let's fix all uses in a separate patch. My point is that this
makes an assumption (that struct bpf_map map field is always the very
first one) which is not enforced and not documented in struct
bpf_local_storage_map.

I'd rather document it in separate patch.
Just doing container_of here and there will lead to wrong assumption
that it can be in a different place, but it's much more involved.
Consider what happens with all map_alloc callbacks... how that pointer
is hard coded into bpf prog.. then passed back into helpers...
then the logic that can read inner fields of bpf_map from the prog...