Re: [PATCH bpf-next v4 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:
- [Medium] bpf: Use-After-Free of struct_ops map due to RCU ordering violation
--

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

bpf: Make struct_ops tasks_rcu grace period optional

bpf_struct_ops_map_free() currently waits for both a regular RCU grace
period and a tasks RCU grace period for every struct_ops map through
synchronize_rcu_mult(call_rcu, call_rcu_tasks).
Add a map_free_pre_rcu() callback so the struct_ops map can remove
ksyms before bpf_map_put() wait for the regular rcu grace period.

> diff --git a/kernel/bpf/bpf_struct_ops.c b/kernel/bpf/bpf_struct_ops.c
> index 3935bf35a4237..7802859eac1ea 100644
> --- a/kernel/bpf/bpf_struct_ops.c
> +++ b/kernel/bpf/bpf_struct_ops.c
> @@ -1037,24 +1046,8 @@ static void bpf_struct_ops_map_free(struct bpf_map *map)
>  
>  	bpf_struct_ops_map_dissoc_progs(st_map);

[Severity: Medium]
Since map->free_after_rcu_gp = true is now set in bpf_struct_ops_map_alloc(),
this function executes after the regular RCU grace period. This means
prog->aux->st_ops_assoc is only cleared after the grace period ends.

Does this violate the RCU 'remove before reclaim' invariant?

A BPF program executing after the RCU grace period (for example, via
bpf_prog_test_run()) can read the stale pointer in
bpf_prog_get_assoc_struct_ops():

kernel/bpf/bpf_struct_ops.c:bpf_prog_get_assoc_struct_ops() {
    ...
    st_ops_assoc = rcu_dereference_check(aux->st_ops_assoc, bpf_rcu_lock_held());
    ...
}

Because the pointer hasn't been cleared yet, the program will read the stale
map pointer, leading to a Use-After-Free when this workqueue subsequently
frees the map.

Can this be exploited using bpf_dummy_ops by closing the map to
trigger the RCU grace period and calling bpf_prog_test_run(prog_fd) exactly
after the grace period ends?

> -	bpf_struct_ops_map_del_ksyms(st_map);
> -
> -	/* The struct_ops's function may switch to another struct_ops.
[ ... ]
> @@ -1163,6 +1156,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: Medium]
Setting this flag shifts the RCU grace period to occur asynchronously before
the map free workqueue executes, which causes the aforementioned Use-After-Free
issue.

Should bpf_struct_ops_map_dissoc_progs() be called in
bpf_struct_ops_map_free_pre_rcu() instead to ensure the RCU pointer is
cleared before the grace period begins?

>  
>  	return map;

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