Re: [PATCH net v3] net/smc: protect clcsock lifetime in smc_getname

[Mahanta Jambigi <mjambigi@linux.ibm.com>]

On 08/10/26 2:14 pm, Chengfeng Ye wrote:
> smc_getname() dereferences smc->clcsock without holding
> clcsock_release_lock. Link-group termination can release the CLC socket
> through smc_close_active_abort() while the SMC socket is still open,
> for example after shutdown(SHUT_WR).
> 
> A getsockname() caller can load smc->clcsock, then the termination worker
> can clear the pointer and call sock_release() before the caller accesses
> clcsock->ops or invokes getname(). This causes a use-after-free; if the
> worker clears the pointer before the load, it causes a NULL dereference.
> The syscall's file reference keeps the SMC socket alive but does not
> prevent asynchronous release of its CLC socket.
> 
> KASAN reported the following with test-only timing instrumentation:
> 
>   BUG: KASAN: slab-use-after-free in smc_getname+0x19e/0x1b0
>   Read of size 8 at addr ffff888109abb4e0 by task poc/103
>   Call Trace:
>    smc_getname+0x19e/0x1b0
>    do_getsockname+0xe5/0x170
>    __sys_getsockname+0x8c/0x100
> 
>   Allocated by task 95:
>    sock_alloc_inode+0x1e/0x280
>    sock_alloc+0x3d/0x240
>    __sock_create+0x7e/0x430
>    smc_create+0x121/0x240
> 
>   Freed by task 0:
>    kmem_cache_free+0xcc/0x340
>    rcu_core+0x50a/0x1850
> 
>   Last potentially related work creation:
>    evict+0x446/0x6c0
>    smc_clcsock_release+0xa8/0xd0
>    smc_close_active_abort+0x26a/0x3a0
>    __smc_lgr_terminate.part.0+0x137/0x2e0
> 
> Hold clcsock_release_lock across the pointer check and the getname()
> callback to serialize with smc_clcsock_release(). Return -EBADF if the
> CLC socket has already been released, preserving the existing peer
> state check and the callback's return value otherwise.
> 
> Fixes: b03faa1fafc8 ("net/smc: postpone release of clcsock")
> Cc: stable@vger.kernel.org
> Assisted-by: GPT-6.1-Sol
> Signed-off-by: Chengfeng Ye <nicoyip.dev@gmail.com>
> ---
> Changes in v3:
> - Limit the fix to smc_getname(), as requested by Mahanta. The code
>   change is identical to v1 and uses the existing release mutex.
> - Drop the new clcsock_lock and the other v2 changes. Mahanta will
>   address the other readers through the broader CLC lifetime changes
>   discussed with Dust Li.
> - Rebase onto current net/main; keep the stable Cc.
> 
> Review:
> https://lore.kernel.org/r/4bb7f15e-2cd8-4598-9bab-1ea5f5d2bb59@linux.ibm.com/
> v2:
> https://lore.kernel.org/r/20261003183325.2289707-1-nicoyip.dev@gmail.com/
> v1:
> https://lore.kernel.org/r/20260926180404.2721010-1-nicoyip.dev@gmail.com/
> 
> Validation: W=1 builds of net/smc/ passed with IPv6 enabled and disabled.
> The KASAN excerpt is the original report; no runtime test was run for v3.
> 
>  net/smc/af_smc.c | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/net/smc/af_smc.c b/net/smc/af_smc.c
> index e9f93b3ab435b..8b3ee70f8433f 100644
> --- a/net/smc/af_smc.c
> +++ b/net/smc/af_smc.c
> @@ -2784,6 +2784,7 @@ int smc_getname(struct socket *sock, struct sockaddr *addr,
>  		int peer)
>  {
>  	struct smc_sock *smc;
> +	int rc = -EBADF;
>  
>  	if (peer && (sock->sk->sk_state != SMC_ACTIVE) &&
>  	    (sock->sk->sk_state != SMC_APPCLOSEWAIT1))
> @@ -2791,7 +2792,11 @@ int smc_getname(struct socket *sock, struct sockaddr *addr,
>  
>  	smc = smc_sk(sock->sk);
>  
> -	return smc->clcsock->ops->getname(smc->clcsock, addr, peer);
> +	mutex_lock(&smc->clcsock_release_lock);
> +	if (smc->clcsock)
> +		rc = smc->clcsock->ops->getname(smc->clcsock, addr, peer);
> +	mutex_unlock(&smc->clcsock_release_lock);
> +	return rc;
>  }
>  
>  int smc_sendmsg(struct socket *sock, struct msghdr *msg, size_t len)
> 
> base-commit: 6d25ffca055a77787c21a36b66c253f76239411b

Reviewed-by: Mahanta Jambigi <mjambigi@linux.ibm.com>