Re: [PATCH v2 2/2] NFS: Fix RCU dereference of cl_xprt in nfs_compare_super_address

[Benjamin Coddington]

On 19 Apr 2026, at 3:01, Sean Chang wrote:

> The cl_xprt pointer in struct rpc_clnt is marked as __rcu. Accessing
> it directly in nfs_compare_super_address() is unsafe and triggers
> Sparse warnings.
>
> Fix this by wrapping the access with rcu_read_lock() and using
> rcu_dereference() to safely retrieve the transport pointer. This
> ensures the xprt structure remains memory-safe during the comparison
> of network namespaces and addresses.
>
> Additionally, add a check for the XPRT_CONNECTED state bit. While RCU
> guarantees the memory remains valid, checking XPRT_CONNECTED ensures
> the transport is still logically active, preventing operations on a
> transport that is already undergoing teardown.
>
> Fixes: 7e3fcf61abde ("nfs: don't share mounts between network namespaces")
> Signed-off-by: Sean Chang <seanwascoding@xxxxxxxxx>
> ---
>  fs/nfs/super.c | 21 ++++++++++++++++++---
>  1 file changed, 18 insertions(+), 3 deletions(-)
>
> diff --git a/fs/nfs/super.c b/fs/nfs/super.c
> index 7a318581f85b..c9044d9d64cc 100644
> --- a/fs/nfs/super.c
> +++ b/fs/nfs/super.c
> @@ -1166,12 +1166,23 @@ static int nfs_set_super(struct super_block *s, struct fs_context *fc)
>  static int nfs_compare_super_address(struct nfs_server *server1,
>  				     struct nfs_server *server2)
>  {
> +	struct rpc_xprt *xprt1, *xprt2;
>  	struct sockaddr *sap1, *sap2;
> -	struct rpc_xprt *xprt1 = server1->client->cl_xprt;
> -	struct rpc_xprt *xprt2 = server2->client->cl_xprt;
> +
> +	rcu_read_lock();
> +
> +	xprt1 = rcu_dereference(server1->client->cl_xprt);
> +	xprt2 = rcu_dereference(server2->client->cl_xprt);

This is a good fix for the sparse warning, but..

> +
> +	if (!xprt1 || !xprt2 ||
> +	    !test_bit(XPRT_CONNECTED, &xprt1->state) ||
> +	    !test_bit(XPRT_CONNECTED, &xprt2->state))
> +		goto out_unlock;

^^ I really don't think this check is necessary.  Aren't we only ever
comparing with one freshly created, and the other looked up holding sb_lock?

I'm doubtful this hunk is fixing a real problem.

Ben