[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-Id: <6099162df8322a2198497a8a27e1b0e1e5c017aeb74b20fc1eecde1e67826900@mail.kernel.org>
Date: Tue, 4 Nov 2025 14:52:29 +0000 (UTC)
From: bot+bpf-ci@...nel.org
To: leon.hwang@...ux.dev,bpf@...r.kernel.org
Cc: ast@...nel.org,andrii@...nel.org,daniel@...earbox.net,martin.lau@...ux.dev,eddyz87@...il.com,song@...nel.org,yonghong.song@...ux.dev,john.fastabend@...il.com,kpsingh@...nel.org,sdf@...ichev.me,haoluo@...gle.com,jolsa@...nel.org,memxor@...il.com,ameryhung@...il.com,linux-kernel@...r.kernel.org,kernel-patches-bot@...com,leon.hwang@...ux.dev,ast@...nel.org,andrii@...nel.org,daniel@...earbox.net,martin.lau@...nel.org,eddyz87@...il.com,yonghong.song@...ux.dev,clm@...a.com,ihor.solodrai@...ux.dev
Subject: Re: [PATCH bpf-next v5 2/2] selftests/bpf: Add test to verify freeing the special fields when update [lru_,]percpu_hash maps
> diff --git a/tools/testing/selftests/bpf/progs/refcounted_kptr.c b/tools/testing/selftests/bpf/progs/refcounted_kptr.c
> index 893a4fdb4..87b0cc018 100644
> --- a/tools/testing/selftests/bpf/progs/refcounted_kptr.c
> +++ b/tools/testing/selftests/bpf/progs/refcounted_kptr.c
[ ... ]
> @@ -568,4 +568,64 @@ int BPF_PROG(rbtree_sleepable_rcu_no_explicit_rcu_lock,
> return 0;
> }
>
> +private(kptr_ref) u64 ref;
> +
> +static int probe_read_refcount(void)
> +{
> + u32 refcount;
> +
> + bpf_probe_read_kernel(&refcount, sizeof(refcount), (void *) ref);
> + return refcount;
> +}
> +
> +static int __insert_in_list(struct bpf_list_head *head, struct bpf_spin_lock *lock,
> + struct node_data __kptr **node)
> +{
> + struct node_data *n, *m;
> +
> + n = bpf_obj_new(typeof(*n));
> + if (!n)
> + return -1;
> +
> + m = bpf_refcount_acquire(n);
> + n = bpf_kptr_xchg(node, n);
> + if (n) {
> + bpf_obj_drop(n);
> + bpf_obj_drop(m);
> + return -2;
> + }
In __insert_in_list(), after bpf_kptr_xchg() stores the new object in
the map and returns the old value in n, can the error path drop both
n and m? At this point, the new object (pointed to by m) is already
referenced by the map. Dropping m here would free an object that the
map still points to, leaving a dangling pointer.
The test itself never triggers this path since it initializes the map
with zeros first, so the kptr field starts as NULL. However, if this
test were extended or this pattern copied elsewhere, the incorrect
error handling could lead to a use-after-free.
> +
> + bpf_spin_lock(lock);
> + bpf_list_push_front(head, &m->l);
> + ref = (u64)(void *) &m->ref;
> + bpf_spin_unlock(lock);
> + return probe_read_refcount();
> +}
[ ... ]
---
AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md
CI run summary: https://github.com/kernel-patches/bpf/actions/runs/19072261328
Powered by blists - more mailing lists