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

[Martin KaFai Lau <martin.lau@linux.dev>]

On Tue, May 19, 2026 at 10:54:20PM +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> - [Critical] The patch incorrectly skips the Tasks RCU grace period (`synchronize_rcu_tasks()`) before freeing executable trampoline memory for all `struct_ops` except `tcp_congestion_ops`, leading to Use-After-Free of executable memory.
> - [High] RCU-001 violation: `bpf_struct_ops_map_dissoc_progs()` removes the map from `prog->aux->st_ops_assoc` after the regular RCU grace period has already completed, leading to a Use-After-Free.
> --
> 
> commit 85a451ad8aefbb364a1840344e08d2f34596b96c
> 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 during struct_ops map
> freeing. It introduces a map_free_pre_rcu callback to remove ksyms before the
> regular RCU grace period. The tasks RCU grace period is now only explicitly
> requested by tcp_congestion_ops.
> 
> > diff --git a/kernel/bpf/bpf_struct_ops.c b/kernel/bpf/bpf_struct_ops.c
> > index 08791180d71d9..28eab24ef0ed0 100644
> > --- a/kernel/bpf/bpf_struct_ops.c
> > +++ b/kernel/bpf/bpf_struct_ops.c
> > @@ -996,24 +1005,8 @@ static void bpf_struct_ops_map_free(struct bpf_map *map)
> >  
> >  	bpf_struct_ops_map_dissoc_progs(st_map);
> 
> [Severity: High]
> Does this violate the RCU remove-before-reclaim rule?
> 
> Because map->free_after_rcu_gp is set to true in bpf_struct_ops_map_alloc(),
> the regular RCU grace period occurs before bpf_struct_ops_map_free() is called.
> Leaving bpf_struct_ops_map_dissoc_progs() here means the map is removed
> from prog->aux->st_ops_assoc after the grace period has already finished.

After the grace period finished, all struct_ops's bpf prog has
finished running

> 
> Could a concurrent execution of the BPF program acquire the RCU read lock, read
> the still-valid aux->st_ops_assoc pointer, and then access the map after it is
> freed?

so this cannot happen.

afaik, for non struct_ops's prog, the st_ops_assoc has a refcnt.
[I could be wrong here. CC Amery.]

Overall, moving bpf_struct_ops_map_dissoc_progs to map_free_pre_rcu
could be a simpler read but don't think it is needed.

> 
> Should bpf_struct_ops_map_dissoc_progs() be moved to the new
> bpf_struct_ops_map_free_pre_rcu() callback instead?
> 
> > -	/* 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);
> >  }
> 
> [Severity: Critical]
> Does skipping the Tasks RCU grace period for other struct_ops cause a
> use-after-free of executable trampoline memory?

I don't think so. Other subsystems using struct_ops wait for
the bpf prog finished running in unreg().

Note that in general this should only matter to the bpf-tcp-cc's struct_ops prog
that does a bpf_setsockopt(TCP_CONGESTION) which triggers a bpf_map_put() on
its own running struct_ops's map refcnt.

> 
> All struct_ops programs are executed via dynamically generated trampolines. When
> the program returns, the RCU read lock is dropped and preemption is re-enabled.
> If a task is preempted while still executing the trampoline epilogue, the
> regular RCU grace period will not wait for it.
> 
> Without the Tasks RCU wait, could the executable trampoline image be freed
> prematurely, causing preempted tasks to execute freed memory when they resume?
> 
> -- 
> Sashiko AI review · https://sashiko.dev/#/patchset/20260519215841.2984970-1-martin.lau@linux.dev?part=2