On 5/3/23 11:48 AM, Alexei Starovoitov wrote:
What it means that sleepable progs using hashmap will be able to avoid uaf with bpf_rcu_read_lock(). Without explicit bpf_rcu_read_lock() it's still safe and equivalent to existing behavior of bpf_mem_alloc. (while your proposed BPF_MA_FREE_AFTER_RCU_GP flavor is not safe to use in hashtab with sleepable progs) After that we can unconditionally remove rcu_head/call_rcu from bpf_cpumask and improve usability of bpf_obj_drop. Probably usage of bpf_mem_alloc in local storage can be simplified as well. Martin wdyt?
If the bpf prog always does a bpf_rcu_read_lock() before accessing the (e.g.) task local storage, it can remove the reuse_now conditions in the bpf_local_storage and directly call the bpf_mem_cache_free().
The only corner use case is when the bpf_prog or syscall does bpf_task_storage_delete() instead of having the task storage stays with the whole lifetime of the task_struct. Using REUSE_AFTER_RCU_GP will be a change of this uaf guarantee to the sleepable program but it is still safe because it is freed after tasks_trace gp. We could take this chance to align this behavior of the local storage map to the other bpf maps.
For BPF_MA_FREE_AFTER_RCU_GP, there are cases that the bpf local storage knows it can be freed without waiting tasks_trace gp. However, only task/cgroup storages are in bpf ma and I don't believe this optimization matter much for them. I would rather focus on the REUSE_AFTER_RCU_GP first.
