From: sashiko-bot@kernel.org
To: "Alexei Starovoitov" <alexei.starovoitov@gmail.com>, bpf@vger.kernel.org
Subject: Re: [PATCH bpf-next 29/38] bpf: Keep values of a dead owner for sleepable programs
Date: Sat, 17 Oct 2026 02:10:44 +0000 [thread overview]
Message-ID: <sashiko-outbox-175195@kernel.org> (raw)
In-Reply-To: <20261017012451.364800-21-alexei.starovoitov@gmail.com>
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Critical] bpf: premature bpf_obj_free_fields() in bpf_selem_unlink_nofail()
- [High] bpf: uncounted sleepable users in bpf_selem_unlink_nofail() via freplace
--
commit 8642076a3bb10a2ade07f6d402a14e8067337799
Author: Josef Bacik <josef@toxicpanda.com>
bpf: Keep values of a dead owner for sleepable programs
This patch defers freeing local storage elements behind an RCU Tasks Trace
grace period when an owner is destroyed if the storage map has sleepable
users or has been used as an inner map.
> diff --git a/kernel/bpf/bpf_local_storage.c b/kernel/bpf/bpf_local_storage.c
> index f234f671e75f8..3d0c455d258c2 100644
> --- a/kernel/bpf/bpf_local_storage.c
> +++ b/kernel/bpf/bpf_local_storage.c
> @@ -352,7 +352,7 @@ int bpf_selem_unlink(struct bpf_local_storage_elem *selem)
> static void bpf_selem_unlink_nofail(struct bpf_local_storage_elem *selem,
> struct bpf_local_storage_map_bucket *b)
> {
> - bool in_map_free = !!b, free_storage = false;
> + bool in_map_free = !!b, free_storage = false, sleepable_users = false;
> struct bpf_local_storage *local_storage;
> struct bpf_local_storage_map *smap;
> unsigned long flags;
> @@ -362,6 +362,10 @@ static void bpf_selem_unlink_nofail(struct bpf_local_storage_elem *selem,
> smap = rcu_dereference_check(SDATA(selem)->smap, bpf_rcu_lock_held());
>
> if (smap) {
> + /* A map that is freed, or NULL here, has no program left. */
> + sleepable_users = !in_map_free &&
> + (atomic64_read(&smap->map.sleepable_refcnt) ||
> + READ_ONCE(smap->map.was_inner_map));
[Severity: High]
Can a BPF_PROG_TYPE_EXT (freplace) program extending a sleepable target
program bypass this sleepable_users check and trigger a slab use-after-free
on SDATA(selem)->data?
In kernel/bpf/verifier.c:can_be_sleepable(), BPF_F_SLEEPABLE is rejected for
BPF_PROG_TYPE_EXT programs, so ext_prog->sleepable is always false:
kernel/bpf/verifier.c:can_be_sleepable() {
if (prog->type == BPF_PROG_TYPE_TRACING) {
...
}
if (prog->type == BPF_PROG_TYPE_LSM)
return prog->expected_attach_type != BPF_LSM_CGROUP;
return prog->type == BPF_PROG_TYPE_KPROBE /* only for uprobes */ ||
prog->type == BPF_PROG_TYPE_STRUCT_OPS ||
prog->type == BPF_PROG_TYPE_RAW_TRACEPOINT ||
prog->type == BPF_PROG_TYPE_TRACEPOINT;
}
When maps are bound to ext_prog in __add_used_map() or bpf_prog_bind_map(),
map->sleepable_refcnt is only incremented if bpf_prog_runs_sleepable(prog)
is true.
However, check_attach_btf_id() and __bpf_trampoline_link_prog() allow a
non-sleepable BPF_PROG_TYPE_EXT program (where ext_prog->aux->might_sleep is
false) to replace the main entry or a global subprogram of a sleepable
target program (where tgt_prog->sleepable is true) without propagating
sleepable status or incrementing map->sleepable_refcnt on ext_prog's
used_maps:
kernel/bpf/verifier.c:check_attach_btf_id() {
if (tgt_prog && prog->type == BPF_PROG_TYPE_EXT) {
env->ops = bpf_verifier_ops[tgt_prog->type];
prog->expected_attach_type = tgt_prog->expected_attach_type;
}
}
At runtime, tgt_prog enters via __bpf_prog_enter_sleepable() or
__bpf_prog_enter_sleepable_recur(), which only acquires
rcu_read_lock_trace() and migrate_disable(), and jumps directly into
ext_prog->bpf_func.
If ext_prog looks up a value from a local storage map bound only to ext_prog
(so sleepable_refcnt is 0 and was_inner_map is false) via
bpf_task_storage_get() and then releases the owner or exits
bpf_rcu_read_unlock(), bpf_local_storage_destroy() ->
bpf_selem_unlink_nofail() will see sleepable_users == false and free selem
via kfree_rcu() while ext_prog still accesses SDATA(selem)->data.
> b = b ? : select_bucket(smap, local_storage);
> err = raw_res_spin_lock_irqsave(&b->lock, flags);
> if (!err) {
> @@ -434,11 +438,12 @@ static void bpf_selem_unlink_nofail(struct bpf_local_storage_elem *selem,
> *
> * The owner that map_free() takes an selem from is alive: sleepable code, which
> * is in no RCU read side critical section, may be walking its list for another
> - * map.
> + * map. And it can still use a value that it got before it let go of an owner
> + * that destroy() is called for.
> */
> if (unlink == 2 ||
> atomic_cmpxchg(&selem->state, SELEM_UNLINKED, SELEM_TOFREE) == SELEM_UNLINKED)
> - bpf_selem_free(selem, !in_map_free);
> + bpf_selem_free(selem, !in_map_free && !sleepable_users);
[Severity: Critical]
When sleepable_users is true, bpf_selem_free() defers freeing selem to
bpf_selem_free_trace_rcu() because a sleepable program may still be reading
or writing SDATA(selem)->data. However, earlier in
bpf_selem_unlink_nofail(), bpf_obj_free_fields() is still called
immediately under b->lock and SDATA(selem)->smap is cleared to NULL:
kernel/bpf/bpf_local_storage.c:bpf_selem_unlink_nofail() {
err = raw_res_spin_lock_irqsave(&b->lock, flags);
if (!err) {
if (likely(selem_linked_to_map(selem))) {
hlist_del_init_rcu(&selem->map_node);
bpf_obj_free_fields(smap->map.record, SDATA(selem)->data);
unlink++;
}
raw_res_spin_unlock_irqrestore(&b->lock, flags);
}
WARN_ON_ONCE(err && in_map_free);
if (!err || in_map_free)
RCU_INIT_POINTER(SDATA(selem)->smap, NULL);
}
Doesn't calling bpf_obj_free_fields() before the RCU Tasks Trace grace
period cause a physical page use-after-free for BPF_UPTR fields and a kptr
leak for BPF_KPTR_REF or BPF_KPTR_PERCPU fields?
For a BPF_MAP_TYPE_TASK_STORAGE map with a BPF_UPTR field:
1. A sleepable program obtains SDATA(selem)->data (or loads the uptr's
PTR_TO_MEM register via mark_uptr_ld_reg()) and releases the task
reference or exits bpf_rcu_read_unlock().
2. When the task exits, free_task() -> bpf_task_storage_free() ->
bpf_local_storage_destroy() -> bpf_selem_unlink_nofail() runs after
exit_mm() has already unmapped the task's user address space.
3. bpf_obj_free_fields() immediately calls unpin_uptr_kaddr() ->
unpin_user_page(virt_to_page(kaddr)) without waiting for the RCU Tasks
Trace grace period and without clearing the pointer in SDATA(selem)->data,
dropping the last reference to the physical page while the sleepable
program can still read and write to it.
Similarly, for BPF_KPTR_REF or BPF_KPTR_PERCPU fields, the running sleepable
program can store a new kptr into SDATA(selem)->data via bpf_kptr_xchg()
after bpf_selem_unlink_nofail() has already called bpf_obj_free_fields() and
set SDATA(selem)->smap to NULL. When bpf_selem_free_trace_rcu() later runs
after the RCU Tasks Trace grace period:
kernel/bpf/bpf_local_storage.c:bpf_selem_free_trace_rcu() {
smap = rcu_dereference_check(SDATA(selem)->smap, 1);
if (smap)
bpf_obj_free_fields(smap->map.record, SDATA(selem)->data);
kfree(selem);
}
SDATA(selem)->smap is already NULL, so bpf_selem_free_trace_rcu() skips
bpf_obj_free_fields() and leaks the newly exchanged kptr.
>
> if (free_storage)
> bpf_local_storage_free(local_storage, !in_map_free);
[ ... ]
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261017012058.355787-1-alexei.starovoitov@gmail.com?part=29
next prev parent reply other threads:[~2026-10-17 2:10 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
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 [this message]
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-175195@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