From: sashiko-bot@kernel.org
To: "Alexei Starovoitov" <alexei.starovoitov@gmail.com>, bpf@vger.kernel.org
Subject: Re: [PATCH bpf-next 04/38] bpf: don't hand out a linked element as extra element
Date: Sat, 17 Oct 2026 01:45:09 +0000 [thread overview]
Message-ID: <sashiko-outbox-175145@kernel.org> (raw)
In-Reply-To: <20261017012058.355787-5-alexei.starovoitov@gmail.com>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] bpf: l_old stored in extra_elems before check_and_cancel_fields()
--
commit 2a85541dd93d84eaa60a85ebbff00bfad598417f
Author: Josef Bacik <josef@toxicpanda.com>
bpf: don't hand out a linked element as extra element
This commit atomically clears the per-CPU extra_elems slot when claiming a
spare element during preallocated hash map updates, and delays storing the
replaced element into the slot until after it has been unlinked from its
bucket.
> diff --git a/kernel/bpf/hashtab.c b/kernel/bpf/hashtab.c
> index 9d4791ee17411..923fec694747b 100644
> --- a/kernel/bpf/hashtab.c
> +++ b/kernel/bpf/hashtab.c
> @@ -1279,11 +1279,17 @@ static struct htab_elem *alloc_htab_elem(struct bpf_htab *htab, void *key,
> if (prealloc) {
> if (old_elem) {
> /* if we're updating the existing element,
> - * use per-cpu extra elems to avoid freelist_pop/push
> + * use per-cpu extra elems to avoid freelist_pop/push.
> + * The caller puts old_elem into the slot once it is
> + * unlinked. Until then the slot is empty for an NMI
> + * or a kprobe that updates this map on this cpu.
> */
> pl_new = this_cpu_ptr(htab->extra_elems);
> - l_new = *pl_new;
> - *pl_new = old_elem;
> + l_new = READ_ONCE(*pl_new);
> + do {
> + if (!l_new)
> + return ERR_PTR(-EBUSY);
> + } while (!try_cmpxchg_local(pl_new, &l_new, NULL));
> } else {
> struct pcpu_freelist_node *l;
>
> @@ -1459,11 +1465,14 @@ static long htab_map_update_elem(struct bpf_map *map, void *key, void *value,
> if (l_old) {
> hlist_nulls_del_rcu(&l_old->hash_node);
>
> - /* l_old has already been stashed in htab->extra_elems, cancel
> - * its reusable special fields before it is available for reuse.
> + /* l_old replaces the extra element that alloc_htab_elem() took,
> + * cancel its reusable special fields before the next update on
> + * this cpu reuses it.
> */
> - if (htab_is_prealloc(htab))
> + if (htab_is_prealloc(htab)) {
> + WRITE_ONCE(*this_cpu_ptr(htab->extra_elems), l_old);
> check_and_cancel_fields(htab, l_old);
> + }
> }
> htab_unlock_bucket(b, flags);
> if (l_old && !htab_is_prealloc(htab))
[Severity: High]
In kernel/bpf/hashtab.c:htab_map_update_elem(), l_old is written to
htab->extra_elems before check_and_cancel_fields(htab, l_old) is called:
if (htab_is_prealloc(htab)) {
WRITE_ONCE(*this_cpu_ptr(htab->extra_elems), l_old);
check_and_cancel_fields(htab, l_old);
}
Once WRITE_ONCE(*this_cpu_ptr(htab->extra_elems), l_old) executes, the
per-CPU slot is no longer NULL. If an NMI or a tracing program (for example,
fentry on bpf_obj_cancel_fields()) triggers on the same CPU and updates an
existing key in another bucket of the same preallocated map,
alloc_htab_elem() will claim l_old instead of returning -EBUSY.
When the map contains BTF special fields (BPF_TIMER, BPF_WORKQUEUE, or
BPF_TASK_WORK), the nested update can link l_old into the other bucket while
its previous timer, workqueue, or task_work fields are still active, and then
the outer check_and_cancel_fields(htab, l_old) will cancel and free the
special fields of the newly inserted live element.
Should check_and_cancel_fields(htab, l_old) be called before
WRITE_ONCE(*this_cpu_ptr(htab->extra_elems), l_old) so the slot stays NULL
until the special fields have been canceled?
титься
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261017012058.355787-1-alexei.starovoitov@gmail.com?part=4
next prev parent reply other threads:[~2026-10-17 1:45 UTC|newest]
Thread overview: 51+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-10-17 1:20 [PATCH bpf-next 00/38] bpf: Memory safety, lifetime and locking fixes Alexei Starovoitov
2026-10-17 1:20 ` [PATCH bpf-next 01/38] bpf: use the frame mem type in devmap egress XDP Alexei Starovoitov
2026-10-17 1:20 ` [PATCH bpf-next 02/38] bpf: forbid sleeping in hash map for_each callbacks Alexei Starovoitov
2026-10-17 1:20 ` [PATCH bpf-next 03/38] bpf: don't leak the inode when token creation fails Alexei Starovoitov
2026-10-17 1:20 ` [PATCH bpf-next 04/38] bpf: don't hand out a linked element as extra element Alexei Starovoitov
2026-10-17 1:45 ` sashiko-bot [this message]
2026-10-17 1:52 ` bot+bpf-ci
2026-10-17 1:20 ` [PATCH bpf-next 05/38] bpf: reject sleepable programs for task_vma iterator Alexei Starovoitov
2026-10-17 1:20 ` [PATCH bpf-next 06/38] bpf: forbid sleeping in bpf_find_vma() callbacks Alexei Starovoitov
2026-10-17 1:20 ` [PATCH bpf-next 07/38] bpf: look at all 64 bits of a BPF_JSET mask on 32-bit Alexei Starovoitov
2026-10-17 1:20 ` [PATCH bpf-next 08/38] bpf: don't let bpf_skb_adjust_room() use up the MSS Alexei Starovoitov
2026-10-17 1:51 ` sashiko-bot
2026-10-17 1:24 ` [PATCH bpf-next 09/38] bpf: Read the probe of a perf event under event_mutex Alexei Starovoitov
2026-10-17 1:24 ` [PATCH bpf-next 10/38] bpf: Don't saturate owner_refcnt on the last unpin Alexei Starovoitov
2026-10-17 1:24 ` [PATCH bpf-next 11/38] bpf: Validate the SRH which End.DT6 uncovers in End.BPF Alexei Starovoitov
2026-10-17 1:24 ` [PATCH bpf-next 12/38] bpf: Pull the inner header for End.DT6 " Alexei Starovoitov
2026-10-17 1:24 ` [PATCH bpf-next 13/38] bpf: Update the SRH state if End.B6 fails late Alexei Starovoitov
2026-10-17 1:24 ` [PATCH bpf-next 14/38] bpf: Clear sk_refcnt of a clone before its storage is linked Alexei Starovoitov
2026-10-17 1:24 ` [PATCH bpf-next 15/38] bpf: Fix NULL deref in the sk_storage map iterator Alexei Starovoitov
2026-10-17 1:24 ` [PATCH bpf-next 16/38] bpf: Hold the socket " Alexei Starovoitov
2026-10-17 2:02 ` bot+bpf-ci
2026-10-17 1:24 ` [PATCH bpf-next 17/38] bpf: Bound the nesting that the BTF show code follows Alexei Starovoitov
2026-10-17 1:46 ` sashiko-bot
2026-10-17 1:24 ` [PATCH bpf-next 18/38] bpf: Check the new program of an iterator link update Alexei Starovoitov
2026-10-17 1:24 ` [PATCH bpf-next 19/38] bpf: Fix stale offload->prog pointer after realloc Alexei Starovoitov
2026-10-17 1:45 ` sashiko-bot
2026-10-17 1:52 ` bot+bpf-ci
2026-10-17 1:24 ` [PATCH bpf-next 20/38] bpf: Don't bind a program to a netdev that goes away Alexei Starovoitov
2026-10-17 1:24 ` [PATCH bpf-next 21/38] bpf: Keep a program until its bpf_wq callback has ended Alexei Starovoitov
2026-10-17 1:24 ` [PATCH bpf-next 22/38] bpf: Count sleepable callbacks as sleepable map users Alexei Starovoitov
2026-10-17 1:24 ` [PATCH bpf-next 23/38] bpf: Check the maps that sleepable callbacks use Alexei Starovoitov
2026-10-17 1:58 ` sashiko-bot
2026-10-17 1:24 ` [PATCH bpf-next 24/38] bpf: Assume no bounds for buffers behind a tail call Alexei Starovoitov
2026-10-17 1:24 ` [PATCH bpf-next 25/38] bpf: Don't take an extension for an iterator program Alexei Starovoitov
2026-10-17 1:24 ` [PATCH bpf-next 26/38] bpf: Assume no bounds for a writable tp with tail calls Alexei Starovoitov
2026-10-17 2:05 ` sashiko-bot
2026-10-17 1:24 ` [PATCH bpf-next 27/38] bpf: Keep iterator programs off special map fields Alexei Starovoitov
2026-10-17 1:24 ` [PATCH bpf-next 28/38] bpf: Free what map_free() unlinks for sleepable readers Alexei Starovoitov
2026-10-17 1:24 ` [PATCH bpf-next 29/38] bpf: Keep values of a dead owner for sleepable programs Alexei Starovoitov
2026-10-17 2:10 ` sashiko-bot
2026-10-17 1:24 ` [PATCH bpf-next 30/38] bpf: Keep the symbols of a program until it has ended Alexei Starovoitov
2026-10-17 1:24 ` [PATCH bpf-next 31/38] bpf: Use the socket of a sk_lookup test run under RCU Alexei Starovoitov
2026-10-17 1:24 ` [PATCH bpf-next 32/38] bpf, cpumap: Hold the device of frames in the queue Alexei Starovoitov
2026-10-17 1:24 ` [PATCH bpf-next 33/38] bpf: Drop packet pointers behind kfuncs writing an skb Alexei Starovoitov
2026-10-17 2:10 ` sashiko-bot
2026-10-17 1:24 ` [PATCH bpf-next 34/38] bpf: Let bpf_dynptr_write() mark a global function Alexei Starovoitov
2026-10-17 1:24 ` [PATCH bpf-next 35/38] bpf: Don't take a dynptr argument for a local dynptr Alexei Starovoitov
2026-10-17 1:24 ` [PATCH bpf-next 36/38] bpf: Follow the head if an skb is written from a slice Alexei Starovoitov
2026-10-17 2:05 ` sashiko-bot
2026-10-17 1:24 ` [PATCH bpf-next 37/38] bpf: Use memmove() to store bytes in an skb Alexei Starovoitov
2026-10-17 1:24 ` [PATCH bpf-next 38/38] bpf: Refuse packet slices for packet-changing helpers Alexei Starovoitov
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=sashiko-outbox-175145@kernel.org \
--to=sashiko-bot@kernel.org \
--cc=alexei.starovoitov@gmail.com \
--cc=bpf@vger.kernel.org \
--cc=sashiko-reviews@lists.linux.dev \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox