On Mon Aug 3, 2026 at 1:12 AM CEST, Ning Ding wrote:
> A verifier-tracked spin lock provides implicit RCU protection while held.
> Consequently, a kptr loaded from a lock-protected map value is marked
> MEM_RCU.
>
> process_spin_lock() invalidates non-owning references on unlock, but leaves
> RCU-protected references intact. This is correct while another RCU context
> remains active. However, for a sleepable program, releasing its final lock
> can end the only RCU-protected context. The verifier then allows a MEM_RCU
> pointer to be used after its protection has ended.
>
> Invalidate RCU-protected references when a successful unlock changes
> in_rcu_cs() from true to false. Non-sleepable programs remain in their
> invocation-wide implicit RCU context, and an explicit RCU read-side section
> continues to protect pointers across the unlock.
>
> A task kptr retained across the final unlock can race with removal of the
> map kptr and bpf_task_release(). This was confirmed to result in a
> task_struct use-after-free in __bpf_get_task_stack().
>
> Add a negative sleepable regression and positive non-sleepable and explicit
> RCU controls.
>
> Fixes: 5861d1e8dbc4 ("bpf: Allow bpf_spin_{lock,unlock} in sleepable progs")
> Assisted-by: Codex:gpt-5.6-sol
> Assisted-by: ChatGPT:GPT-5.6-Pro
> Signed-off-by: Ning Ding <[email protected]>
> ---

It makes sense, but split the kernel side fix and selftests into two separate
patches. The list has several examples. Also, I don't think this is as serious,
so please target bpf-next in the respin.

pw-bot: cr

>  kernel/bpf/verifier.c                         |  5 ++
>  .../selftests/bpf/prog_tests/task_kfunc.c     |  2 +
>  .../selftests/bpf/progs/task_kfunc_common.h   | 12 +++++
>  .../selftests/bpf/progs/task_kfunc_failure.c  | 24 ++++++++++
>  .../selftests/bpf/progs/task_kfunc_success.c  | 48 +++++++++++++++++++
>  5 files changed, 91 insertions(+)
>
> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> index fdc5fbb1f78c..aea9fdbbd33a 100644
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c
> @@ -204,6 +204,7 @@ static int acquire_reference(struct bpf_verifier_env 
> *env, int insn_idx, int par
>  static int release_reference_nomark(struct bpf_verifier_state *state, int 
> id);
>  static int release_reference(struct bpf_verifier_env *env, int id);
>  static void invalidate_non_owning_refs(struct bpf_verifier_env *env);
> +static void invalidate_rcu_protected_refs(struct bpf_verifier_env *env);
>  static bool in_rbtree_lock_required_cb(struct bpf_verifier_env *env);
>  static bool is_tracing_prog_type(enum bpf_prog_type type);
>  static int ref_set_non_owning(struct bpf_verifier_env *env,
> @@ -7051,6 +7052,7 @@ static int process_spin_lock(struct bpf_verifier_env 
> *env, struct bpf_reg_state
>                       return err;
>               }
>       } else {
> +             bool was_in_rcu_cs;
>               void *ptr;
>               int type;
>
> @@ -7078,10 +7080,13 @@ static int process_spin_lock(struct bpf_verifier_env 
> *env, struct bpf_reg_state
>                       verbose(env, "%s_unlock cannot be out of order\n", 
> lock_str);
>                       return -EINVAL;
>               }
> +             was_in_rcu_cs = in_rcu_cs(env);
>               if (release_lock_state(cur, type, reg->id, ptr)) {
>                       verbose(env, "%s_unlock of different lock\n", lock_str);
>                       return -EINVAL;
>               }
> +             if (was_in_rcu_cs && !in_rcu_cs(env))
> +                     invalidate_rcu_protected_refs(env);
>
>               invalidate_non_owning_refs(env);
>       }
> diff --git a/tools/testing/selftests/bpf/prog_tests/task_kfunc.c 
> b/tools/testing/selftests/bpf/prog_tests/task_kfunc.c
> index e6e95c1416e6..fbd7855712c1 100644
> --- a/tools/testing/selftests/bpf/prog_tests/task_kfunc.c
> +++ b/tools/testing/selftests/bpf/prog_tests/task_kfunc.c
> @@ -176,6 +176,8 @@ static const char * const success_tests[] = {
>       "test_task_from_pid_current",
>       "test_task_from_pid_invalid",
>       "task_kfunc_acquire_trusted_walked",
> +     "task_kfunc_acquire_after_spin_unlock_non_sleepable",
> +     "task_kfunc_acquire_after_spin_unlock_explicit_rcu",
>       "test_task_kfunc_flavor_relo",
>       "test_task_kfunc_flavor_relo_not_found",
>  };
> diff --git a/tools/testing/selftests/bpf/progs/task_kfunc_common.h 
> b/tools/testing/selftests/bpf/progs/task_kfunc_common.h
> index e9c4fea7a4bb..052c9d0e3e2a 100644
> --- a/tools/testing/selftests/bpf/progs/task_kfunc_common.h
> +++ b/tools/testing/selftests/bpf/progs/task_kfunc_common.h
> @@ -20,6 +20,18 @@ struct {
>       __uint(max_entries, 1);
>  } __tasks_kfunc_map SEC(".maps");
>
> +struct task_kptr_lock_value {
> +     struct bpf_spin_lock lock;
> +     struct task_struct __kptr * task;
> +};
> +
> +struct {
> +     __uint(type, BPF_MAP_TYPE_ARRAY);
> +     __type(key, int);
> +     __type(value, struct task_kptr_lock_value);
> +     __uint(max_entries, 1);
> +} task_kptr_lock_map SEC(".maps");
> +
>  struct task_struct *bpf_task_acquire(struct task_struct *p) __ksym;
>  void bpf_task_release(struct task_struct *p) __ksym;
>  struct task_struct *bpf_task_from_pid(s32 pid) __ksym;
> diff --git a/tools/testing/selftests/bpf/progs/task_kfunc_failure.c 
> b/tools/testing/selftests/bpf/progs/task_kfunc_failure.c
> index 8942b5478129..7a0c7ee95511 100644
> --- a/tools/testing/selftests/bpf/progs/task_kfunc_failure.c
> +++ b/tools/testing/selftests/bpf/progs/task_kfunc_failure.c
> @@ -378,3 +378,27 @@ int BPF_PROG(task_kfunc_release_in_map, struct 
> task_struct *task, u64 clone_flag
>
>       return 0;
>  }
> +
> +SEC("?fentry.s/" SYS_PREFIX "sys_getpgid")
> +__failure __msg("R1 must be a rcu pointer")
> +int BPF_PROG(task_kfunc_acquire_after_final_spin_unlock)
> +{
> +     struct task_kptr_lock_value *v;
> +     struct task_struct *task, *acquired;
> +     int key = 0;
> +
> +     v = bpf_map_lookup_elem(&task_kptr_lock_map, &key);
> +     if (!v)
> +             return 0;
> +
> +     bpf_spin_lock(&v->lock);
> +     task = v->task;
> +     bpf_spin_unlock(&v->lock);
> +     if (!task)
> +             return 0;
> +
> +     acquired = bpf_task_acquire(task);
> +     if (acquired)
> +             bpf_task_release(acquired);
> +     return 0;
> +}
> diff --git a/tools/testing/selftests/bpf/progs/task_kfunc_success.c 
> b/tools/testing/selftests/bpf/progs/task_kfunc_success.c
> index d63a79ee33dc..2bab7634c9df 100644
> --- a/tools/testing/selftests/bpf/progs/task_kfunc_success.c
> +++ b/tools/testing/selftests/bpf/progs/task_kfunc_success.c
> @@ -6,6 +6,7 @@
>  #include <bpf/bpf_helpers.h>
>
>  #include "../bpf_experimental.h"
> +#include "bpf_misc.h"
>  #include "task_kfunc_common.h"
>
>  char _license[] SEC("license") = "GPL";
> @@ -366,6 +367,53 @@ int BPF_PROG(task_kfunc_acquire_trusted_walked, struct 
> task_struct *task, u64 cl
>       return 0;
>  }
>
> +SEC("fentry/" SYS_PREFIX "sys_getpgid")
> +int BPF_PROG(task_kfunc_acquire_after_spin_unlock_non_sleepable)
> +{
> +     struct task_kptr_lock_value *v;
> +     struct task_struct *task, *acquired;
> +     int key = 0;
> +
> +     v = bpf_map_lookup_elem(&task_kptr_lock_map, &key);
> +     if (!v)
> +             return 0;
> +
> +     bpf_spin_lock(&v->lock);
> +     task = v->task;
> +     bpf_spin_unlock(&v->lock);
> +     if (!task)
> +             return 0;
> +
> +     acquired = bpf_task_acquire(task);
> +     if (acquired)
> +             bpf_task_release(acquired);
> +     return 0;
> +}
> +
> +SEC("fentry.s/" SYS_PREFIX "sys_getpgid")
> +int BPF_PROG(task_kfunc_acquire_after_spin_unlock_explicit_rcu)
> +{
> +     struct task_kptr_lock_value *v;
> +     struct task_struct *task, *acquired;
> +     int key = 0;
> +
> +     v = bpf_map_lookup_elem(&task_kptr_lock_map, &key);
> +     if (!v)
> +             return 0;
> +
> +     bpf_rcu_read_lock();
> +     bpf_spin_lock(&v->lock);
> +     task = v->task;
> +     bpf_spin_unlock(&v->lock);
> +     if (task) {
> +             acquired = bpf_task_acquire(task);
> +             if (acquired)
> +                     bpf_task_release(acquired);
> +     }
> +     bpf_rcu_read_unlock();
> +     return 0;
> +}
> +
>  SEC("syscall")
>  int test_task_from_vpid_current(const void *ctx)
>  {


Reply via email to