Re: [PATCH bpf v3 4/4] selftests/bpf: Test untrusted allocated-object pointers

[Kumar Kartikeya Dwivedi]

On Mon Aug 3, 2026 at 1:22 PM CEST, Ning Ding wrote:
> The verifier previously allowed pointers used after RCU protection ended
> to reach bpf_refcount_acquire() and, for one object layout, a direct write.
> If the object was freed and reused, these operations could access stale
> memory.
>
> Add tests that keep BPF_PROBE_MEM reads accepted but reject reference
> acquisition and direct writes after RCU protection ends. Cover both tested
> object layouts.
>
> Reported-by: sashiko-bot@xxxxxxxxxx
> Link: https://lore.kernel.org/r/20260726021304.97ED91F000E9@xxxxxxxxxxxxxxx
> Assisted-by: Codex:gpt-5
> Signed-off-by: Ning Ding <dingning04@xxxxxxxxx>
> ---
>  .../selftests/bpf/progs/refcounted_kptr.c     | 100 ++++++++++++++++++
>  .../bpf/progs/refcounted_kptr_fail.c          |  27 +++++
>  2 files changed, 127 insertions(+)
>
> diff --git a/tools/testing/selftests/bpf/progs/refcounted_kptr.c b/tools/testing/selftests/bpf/progs/refcounted_kptr.c
> index fd35093285c0d..b70be8b52ff80 100644
> --- a/tools/testing/selftests/bpf/progs/refcounted_kptr.c
> +++ b/tools/testing/selftests/bpf/progs/refcounted_kptr.c
> @@ -893,6 +893,106 @@ long refcount_acquire_rcu_map_kptr_null_checked(void *ctx)
>  	return 0;
>  }
>
> +SEC("?tc")
> +__success
> +long map_kptr_read_after_rcu_unlock(void *ctx)
> +{
> +	struct map_value_refcount_only *mapval;
> +	struct node_refcount_only *n;
> +	int idx = 0;
> +
> +	mapval = bpf_map_lookup_elem(&stashed_refcount_only, &idx);
> +	if (!mapval)
> +		return 0;
> +
> +	bpf_rcu_read_lock();
> +	n = mapval->node;
> +	if (!n) {
> +		bpf_rcu_read_unlock();
> +		return 0;
> +	}
> +	bpf_rcu_read_unlock();
> +
> +	return n->key;

Here, n->key won't read untrusted pointer, since tc progs have RCU protection,
so I don't think the pointer becomes untrusted.

> +}
> +
> +SEC("?tc")
> +__failure __msg("is neither owning or non-owning ref")
> +long refcount_acquire_graph_after_rcu_unlock(void *ctx)
> +{
> +	struct map_value *mapval;
> +	struct node_data *n, *m;
> +	int idx = 0;
> +
> +	mapval = bpf_map_lookup_elem(&stashed_nodes, &idx);
> +	if (!mapval)
> +		return 0;
> +
> +	bpf_rcu_read_lock();
> +	n = mapval->node;
> +	if (!n) {
> +		bpf_rcu_read_unlock();
> +		return 0;
> +	}
> +	bpf_rcu_read_unlock();
> +
> +	m = bpf_refcount_acquire(n);
> +	if (m)
> +		bpf_obj_drop(m);
> +
> +	return 0;
> +}

I don't think you ran these tests, they have unexpected success, mostly because
tc has RCU read protection already, so n passed to refcount acquire is not
untrusted.

> +
> +SEC("?tc")
> +__failure __msg("only read is supported")
> +long graph_map_kptr_write_after_rcu_unlock(void *ctx)
> +{
> +	struct map_value *mapval;
> +	struct node_data *n;
> +	int idx = 0;
> +
> +	mapval = bpf_map_lookup_elem(&stashed_nodes, &idx);
> +	if (!mapval)
> +		return 1;
> +
> +	bpf_rcu_read_lock();
> +	n = mapval->node;
> +	if (!n) {
> +		bpf_rcu_read_unlock();
> +		return 2;
> +	}
> +	bpf_rcu_read_unlock();
> +
> +	n->key = 1;
> +	return 0;
> +}
> +

Same for write here, it succeeds...

> +SEC("?tc")
> +__success
> +long graph_map_kptr_read_after_spin_unlock(void *ctx)
> +{
> +	struct map_value *mapval;
> +	struct node_data *n;
> +	int idx = 0;
> +
> +	mapval = bpf_map_lookup_elem(&stashed_nodes, &idx);
> +	if (!mapval)
> +		return 0;
> +
> +	bpf_rcu_read_lock();
> +	n = mapval->node;
> +	if (!n) {
> +		bpf_rcu_read_unlock();
> +		return 0;
> +	}
> +	bpf_rcu_read_unlock();
> +
> +	bpf_spin_lock(&lock);
> +	bpf_spin_unlock(&lock);
> +
> +	return n->key;
> +}
> +

Similar case.

>  static long __stash_map_empty_xchg(struct node_data *n, int idx)
>  {
>  	struct map_value *mapval = bpf_map_lookup_elem(&stashed_nodes, &idx);
> diff --git a/tools/testing/selftests/bpf/progs/refcounted_kptr_fail.c b/tools/testing/selftests/bpf/progs/refcounted_kptr_fail.c
> index acd3e81a39168..3408f68ad444d 100644
> --- a/tools/testing/selftests/bpf/progs/refcounted_kptr_fail.c
> +++ b/tools/testing/selftests/bpf/progs/refcounted_kptr_fail.c
> @@ -127,6 +127,33 @@ long refcount_acquire_rcu_map_kptr_unchecked_drop(void *ctx)
>  	return 0;
>  }
>
> +SEC("?tc")
> +__failure __msg("is neither owning or non-owning ref")
> +long refcount_acquire_after_rcu_unlock(void *ctx)
> +{
> +	struct map_value_refcount_only *mapval;
> +	struct node_refcount_only *n, *m;
> +	int idx = 0;
> +
> +	mapval = bpf_map_lookup_elem(&stashed_refcount_only, &idx);
> +	if (!mapval)
> +		return 1;
> +
> +	bpf_rcu_read_lock();
> +	n = mapval->node;
> +	if (!n) {
> +		bpf_rcu_read_unlock();
> +		return 2;
> +	}
> +	bpf_rcu_read_unlock();
> +
> +	m = bpf_refcount_acquire(n);
> +	if (m)
> +		bpf_obj_drop(m);
> +
> +	return 0;
> +}

This also passes.

> +
>  SEC("?tc")
>  __failure __msg("Unreleased reference id=3 alloc_insn={{[0-9]+}}")
>  long rbtree_refcounted_node_ref_escapes_owning_input(void *ctx)

Please respin and only send patches after you have tested them properly.

The right fix might be to use fentry.s for these tests to trigger untrusted
marking of pointers. The fixes themselves do make sense.

For the next version, target bpf-next and drop Cc: stable.

pw-bot: cr