Message ID | 20230801203630.3581291-4-davemarchevsky@fb.com (mailing list archive) |
---|---|
State | Changes Requested |
Delegated to: | BPF |
Headers | show |
Series | BPF Refcount followups 3: bpf_mem_free_rcu refcounted nodes | expand |
On 8/1/23 1:36 PM, Dave Marchevsky wrote: > This is the final fix for the use-after-free scenario described in > commit 7793fc3babe9 ("bpf: Make bpf_refcount_acquire fallible for > non-owning refs"). That commit, by virtue of changing > bpf_refcount_acquire's refcount_inc to a refcount_inc_not_zero, fixed > the "refcount incr on 0" splat. The not_zero check in > refcount_inc_not_zero, though, still occurs on memory that could have > been free'd and reused, so the commit didn't properly fix the root > cause. > > This patch actually fixes the issue by free'ing using the recently-added > bpf_mem_free_rcu, which ensures that the memory is not reused until > RCU Tasks Trace grace period has elapsed. If that has happened then > there are no non-owning references alive that point to the > recently-free'd memory, so it can be safely reused. > > Signed-off-by: Dave Marchevsky <davemarchevsky@fb.com> Acked-by: Yonghong Song <yonghong.song@linux.dev>
diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c index 56ce5008aedd..7702f657fa3f 100644 --- a/kernel/bpf/helpers.c +++ b/kernel/bpf/helpers.c @@ -1913,7 +1913,11 @@ void __bpf_obj_drop_impl(void *p, const struct btf_record *rec) if (rec) bpf_obj_free_fields(rec, p); - bpf_mem_free(&bpf_global_ma, p); + + if (rec && rec->refcount_off >= 0) + bpf_mem_free_rcu(&bpf_global_ma, p); + else + bpf_mem_free(&bpf_global_ma, p); } __bpf_kfunc void bpf_obj_drop_impl(void *p__alloc, void *meta__ign)
This is the final fix for the use-after-free scenario described in commit 7793fc3babe9 ("bpf: Make bpf_refcount_acquire fallible for non-owning refs"). That commit, by virtue of changing bpf_refcount_acquire's refcount_inc to a refcount_inc_not_zero, fixed the "refcount incr on 0" splat. The not_zero check in refcount_inc_not_zero, though, still occurs on memory that could have been free'd and reused, so the commit didn't properly fix the root cause. This patch actually fixes the issue by free'ing using the recently-added bpf_mem_free_rcu, which ensures that the memory is not reused until RCU Tasks Trace grace period has elapsed. If that has happened then there are no non-owning references alive that point to the recently-free'd memory, so it can be safely reused. Signed-off-by: Dave Marchevsky <davemarchevsky@fb.com> --- kernel/bpf/helpers.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-)