Re: [PATCH bpf-next v3 02/15] bpf: Make struct_ops tasks_rcu grace period optional

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Use-After-Free due to RCU-001 violation in bpf_struct_ops_map_free()
--

commit 1218e0fc116da5657ca3ee1c0ca920d127c6c75d
Author: Martin KaFai Lau <martin.lau@kernel.org>

bpf: Make struct_ops tasks_rcu grace period optional

This commit makes the tasks RCU grace period optional for struct_ops maps.
It adds a map_free_pre_rcu callback so ksyms can be removed before
bpf_map_put waits for a regular RCU grace period, and defers tasks RCU
synchronization to map free time only when required by the ops.

> diff --git a/kernel/bpf/bpf_struct_ops.c b/kernel/bpf/bpf_struct_ops.c
> index d06b3d9bcc13c..c422ce41873ee 100644
> --- a/kernel/bpf/bpf_struct_ops.c
> +++ b/kernel/bpf/bpf_struct_ops.c
[ ... ]
> @@ -997,24 +1006,8 @@ static void bpf_struct_ops_map_free(struct bpf_map *map)
>  
>  	bpf_struct_ops_map_dissoc_progs(st_map);
>  
> -	bpf_struct_ops_map_del_ksyms(st_map);
> -
> -	/* The struct_ops's function may switch to another struct_ops.
> -	 *
> -	 * For example, bpf_tcp_cc_x->init() may switch to
> -	 * another tcp_cc_y by calling
> -	 * setsockopt(TCP_CONGESTION, "tcp_cc_y").
> -	 * During the switch,  bpf_struct_ops_put(tcp_cc_x) is called
> -	 * and its refcount may reach 0 which then free its
> -	 * trampoline image while tcp_cc_x is still running.
> -	 *
> -	 * A vanilla rcu gp is to wait for all bpf-tcp-cc prog
> -	 * to finish. bpf-tcp-cc prog is non sleepable.
> -	 * A rcu_tasks gp is to wait for the last few insn
> -	 * in the tramopline image to finish before releasing
> -	 * the trampoline image.
> -	 */
> -	synchronize_rcu_mult(call_rcu, call_rcu_tasks);
> +	if (tasks_rcu && IS_ENABLED(CONFIG_TASKS_RCU))
> +		synchronize_rcu_tasks();
>  
>  	__bpf_struct_ops_map_free(map);
>  }
[ ... ]
> @@ -1123,6 +1116,7 @@ static struct bpf_map *bpf_struct_ops_map_alloc(union bpf_attr *attr)
>  
>  	mutex_init(&st_map->lock);
>  	bpf_map_init_from_attr(map, attr);
> +	map->free_after_rcu_gp = true;

[Severity: High]
Does this sequence introduce a use-after-free by violating the RCU "remove
before reclaim" ordering?

By setting map->free_after_rcu_gp = true, bpf_map_put() will now wait for a
regular RCU grace period before invoking bpf_struct_ops_map_free():

bpf_map_put()
  call_rcu(&map->rcu, bpf_map_free_rcu_gp)
  ...
  // later in workqueue
  bpf_struct_ops_map_free()

However, the programs are unpublished from the map during
bpf_struct_ops_map_free():

static void bpf_struct_ops_map_free(struct bpf_map *map)
{
        ...
        bpf_struct_ops_map_dissoc_progs(st_map);
        ...
        __bpf_struct_ops_map_free(map);
}

Because the unpublish operation (bpf_struct_ops_map_dissoc_progs) happens
after the RCU grace period has already elapsed, could a concurrent BPF program
execution (e.g., via bpf_prog_test_run()) start after the grace period begins,
find the still-associated struct_ops pointer, and access it while it is being
synchronously freed by __bpf_struct_ops_map_free()?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260706171918.317102-1-ameryhung@gmail.com?part=2