Re: [PATCH bpf-next 2/2] selftests/bpf: Cover nullable RCU pointer use after unlock

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The newly added tests fail to test `MEM_RCU` pointer use-after-unlock behavior because the pointer is not an `__rcu` pointer and is read outside the explicit RCU critical section.
--

commit 78986d2dc799ce86278ca6d6e66b17c43c68dbeb
Author: Yiyang Chen <chenyy23@mails.tsinghua.edu.cn>

selftests/bpf: Cover nullable RCU pointer use after unlock
    
Add coverage for nullable BTF pointers that are read under
bpf_rcu_read_lock() and then used after bpf_rcu_read_unlock().

> diff --git a/tools/testing/selftests/bpf/progs/rcu_read_lock.c b/tools/testing/selftests/bpf/progs/rcu_read_lock.c
> index b4e073168fb15..b78542706fbce 100644
> --- a/tools/testing/selftests/bpf/progs/rcu_read_lock.c
> +++ b/tools/testing/selftests/bpf/progs/rcu_read_lock.c

[ ... ]

> @@ -372,6 +372,26 @@ int cross_rcu_region(void *ctx)
>  	return 0;
>  }
>  
> +SEC("?tp_btf/net_dev_queue")
> +int BPF_PROG(rcu_null_check_after_unlock, struct sk_buff *skb)
> +{
> +	bpf_rcu_read_lock();
> +	bpf_rcu_read_unlock();
> +
> +	if (!skb->sk)
> +		return 0;
> +	return skb->sk->__sk_common.skc_daddr;
> +}

[Severity: Medium]
Since the skb->sk pointer is loaded after the explicit bpf_rcu_read_unlock()
call, does this actually test the MEM_RCU trust behavior?

Because skb->sk is not an __rcu pointer and the load occurs completely outside
the RCU critical section, it appears the lock and unlock calls here are
isolated and have no effect on the pointer's trust state in the verifier.

> +
> +SEC("?tp_btf/net_dev_queue")
> +int BPF_PROG(rcu_null_deref_after_unlock, struct sk_buff *skb)
> +{
> +	bpf_rcu_read_lock();
> +	bpf_rcu_read_unlock();
> +
> +	return skb->sk->__sk_common.skc_daddr;
> +}

[Severity: Medium]
Is this negative test being rejected by the verifier for the wrong reason?

Because skb->sk is a standard PTR_MAYBE_NULL, the verifier will reject the
dereference due to the missing NULL check regardless of the RCU unlock.
Could this mask the absence of the intended RCU reference tracking coverage?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1781968391.git.chenyy23@mails.tsinghua.edu.cn?part=2