Re: [PATCH bpf-next 1/2] bpf: Retire task work before reclaiming hash elements

["Alexei Starovoitov" <alexei.starovoitov@gmail.com>]

On Wed, Oct 28, 2026 at 12:35 AM Yun Lu <luyun_611@163.com> wrote:
> Leave ERR_PTR(-ENOENT) in task-work slots before returning non-preallocated
> HASH and RHASH elements to the allocator. The existing fetch/acquire error
> path rejects the retired slot. Reopen it only when the allocator acquires
> the storage again. Also retire a reinitialized RHASH candidate abandoned
> after insertion finds an existing element or fails: a reader of a previous
> incarnation can still hold its value pointer and install a context there.

This is overkill.
It adds a 3rd value of twk->ctx and two new functions that hashtab.c
calls in 6 places.
The kfuncs start to work differently for prealloc and non-prealloc maps.

The map knows whether the element is deleted. Look it up.
In bpf_task_work_acquire_ctx() the prog owns the ctx in PENDING state.
The irq_work is not queued yet.

  prog                                   delete
  cmpxchg(&twk->ctx, NULL, ctx)          unlink the element
  cmpxchg(&ctx->state, .., PENDING)      xchg(&twk->ctx, NULL)
  lookup

Either the lookup doesn't find the element or the xchg() finds the ctx.
It's the same scheme as the usercnt check in __bpf_async_init().

hashtab.c doesn't change. A deleted element gets -ENOENT in all map types.
The cost is one lookup per bpf_task_work_schedule_*().

The diff doesn't have the reorder of FREED and detach in
bpf_task_work_cancel_and_free().
That's a fix for a different race. It doesn't need a new ctx in a deleted
element. Pls send it as a separate patch with its own steps.

Something like this (compile tested only):

diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c
index 648abe1..acd62ef 100644
--- a/kernel/bpf/helpers.c
+++ b/kernel/bpf/helpers.c
@@ -1148,6 +1148,14 @@ static void *map_key_from_value(struct bpf_map *map, void *value, u32 *arr_idx)
 	return (void *)value - round_up(map->key_size, 8);
 }

+/* Is the element that holds this value still in the map? */
+static bool bpf_map_value_is_linked(struct bpf_map *map, void *value)
+{
+	u32 idx;
+
+	return map->ops->map_lookup_elem(map, map_key_from_value(map, value, &idx)) == value;
+}
+
 enum bpf_async_type {
 	BPF_ASYNC_TYPE_TIMER = 0,
 	BPF_ASYNC_TYPE_WQ,
@@ -4741,6 +4749,19 @@ static struct bpf_task_work_ctx *bpf_task_work_acquire_ctx(struct bpf_task_work
 		return ERR_PTR(-EBUSY);
 	}

+	/*
+	 * The prog can still hold a value of an element that was deleted from the map.
+	 * bpf_task_work_cancel_and_free() is done with it and nothing stops a new callback
+	 * when the element is freed. The ctx is published and PENDING at this point: either
+	 * the lookup doesn't find the element or bpf_task_work_cancel_and_free() finds the ctx.
+	 * An idle ctx stays in the element until the element is reused or freed.
+	 */
+	if (!bpf_map_value_is_linked(map, (void *)tw - map->record->task_work_off)) {
+		(void)cmpxchg(&ctx->state, BPF_TW_PENDING, BPF_TW_STANDBY);
+		bpf_task_work_ctx_put(ctx);
+		return ERR_PTR(-ENOENT);
+	}
+
 	/*
 	 * If no process or bpffs is holding a reference to the map, no new callbacks should be
 	 * scheduled. This does not address any race or correctness issue, but rather is a policy

[...]

> +	bpf_task_work_free(&rhtab->map, rhtab_elem_value(elem, rhtab->map.key_size));
>  	bpf_obj_cancel_fields(&rhtab->map,
>  			      rhtab_elem_value(elem, rhtab->map.key_size));
>  	bpf_mem_cache_free_rcu(&rhtab->ma, elem);

bpf_wq and bpf_timer have the same problem.
bpf_wq_init() on a deleted element installs a new cb. cb->value points to
the element. htab_mem_dtor() ends in cancel_work(). It doesn't wait for
bpf_wq_work(). The callback still runs when the element is freed.
Pls fix all three.

Also trim the commit log. The 5 steps and the splat are enough.

pw-bot: cr