Re: [PATCH net v2] net/smc: fix out-of-bounds read when sk_user_data holds a sk_psock

[D. Wythe]

On Fri, Jun 19, 2026 at 03:03:41PM +0000, Sechang Lim wrote:
> SMC stores its smc_sock in the clcsock's sk_user_data tagged
> SK_USER_DATA_NOCOPY and reads it back with smc_clcsock_user_data(), which
> only strips that flag. sockmap stores a sk_psock in the same field tagged
> SK_USER_DATA_NOCOPY | SK_USER_DATA_PSOCK. Nothing keeps both off one
> socket, and SMC then casts the sk_psock to an smc_sock.
> 
> A passive-open child hits this. It inherits the listener's
> smc_clcsock_data_ready(), but sk_clone_lock() clears its NOCOPY
> sk_user_data, and a BPF sock_ops program then adds the child to a sockmap,
> installing a sk_psock in that field. The inherited callback reads it as an
> smc_sock and dereferences a clcsk_* pointer past the end of the sk_psock:
> 
>   BUG: KASAN: slab-out-of-bounds in smc_clcsock_data_ready+0x84/0x200 net/smc/af_smc.c:2637
>   Read of size 8 at addr ffff8880013b8674 by task syz.6.12484/67930
>    <IRQ>
>    smc_clcsock_data_ready+0x84/0x200 net/smc/af_smc.c:2637
>    tcp_urg+0x24d/0x360 net/ipv4/tcp_input.c:6264
>    tcp_rcv_state_process+0x280d/0x4940 net/ipv4/tcp_input.c:7336
>    tcp_child_process+0x371/0xa50 net/ipv4/tcp_minisocks.c:1002
>    tcp_v4_rcv+0x1eaa/0x2a00 net/ipv4/tcp_ipv4.c:2186
>    [...]
>    </IRQ>
> 
>   Allocated by task 67930:
>    sk_psock_init+0x142/0x740 net/core/skmsg.c:766
>    sock_hash_update_common+0xd3/0x990 net/core/sock_map.c:1010
>    bpf_sock_hash_update+0x114/0x170 net/core/sock_map.c:1229
>    __cgroup_bpf_run_filter_sock_ops+0x74/0xa0 kernel/bpf/cgroup.c:1727
>    tcp_init_transfer+0x1085/0x1100 net/ipv4/tcp_input.c:6693
>    [...]
> 
> sk_psock() already guards the other side, returning NULL unless
> SK_USER_DATA_PSOCK is set. Make smc_clcsock_user_data() and its RCU
> variant return the smc_sock only when sk_user_data carries SMC's tag
> alone. A sk_psock then reads back as NULL, which the data_ready and
> fallback callbacks already handle.
> 
> Fixes: a60a2b1e0af1 ("net/smc: reduce active tcp_listen workers")
> Signed-off-by: Sechang Lim <rhkrqnwk98@xxxxxxxxx>
> ---
>  net/smc/smc.h | 18 +++++++++++++++---
>  1 file changed, 15 insertions(+), 3 deletions(-)
> 
> diff --git a/net/smc/smc.h b/net/smc/smc.h
> index 52145df83f6e..88dfb459b7cc 100644
> --- a/net/smc/smc.h
> +++ b/net/smc/smc.h
> @@ -342,13 +342,25 @@ static inline void smc_init_saved_callbacks(struct smc_sock *smc)
>  
>  static inline struct smc_sock *smc_clcsock_user_data(const struct sock *clcsk)
>  {
> -	return (struct smc_sock *)
> -	       ((uintptr_t)clcsk->sk_user_data & ~SK_USER_DATA_NOCOPY);
> +	uintptr_t data = (uintptr_t)clcsk->sk_user_data;
> +
> +	/*
> +	 * Return the smc_sock only if the slot carries SMC's tag alone.
> +	 * sockmap stores a sk_psock here tagged SK_USER_DATA_PSOCK; it is
> +	 * not an smc_sock and must not be dereferenced as one.
> +	 */
> +	if ((data & ~SK_USER_DATA_PTRMASK) != SK_USER_DATA_NOCOPY)
> +		return NULL;
> +	return (struct smc_sock *)(data & SK_USER_DATA_PTRMASK);
>  }
>  
>  static inline struct smc_sock *smc_clcsock_user_data_rcu(const struct sock *clcsk)
>  {
> -	return (struct smc_sock *)rcu_dereference_sk_user_data(clcsk);
> +	uintptr_t data = (uintptr_t)rcu_dereference(__sk_user_data(clcsk));
> +
> +	if ((data & ~SK_USER_DATA_PTRMASK) != SK_USER_DATA_NOCOPY)
> +		return NULL;
> +	return (struct smc_sock *)(data & SK_USER_DATA_PTRMASK);
>  }
>  
>  /* save target_cb in saved_cb, and replace target_cb with new_cb */

No. The core issue is how to resolve the ownership conflict between
sockmap and SMC over sk_user_data, which can by no means be solved by
adding runtime checks on the read path.

Following sk_psock_init(), the simplest approach would be to always
explicitly set sk_user_data or ulp_ops during the active/passive
creation of smc->clcsock, thereby avoiding the conflict at its root.

Additionally, compatibility with sockmap in the fallback path needs to
be considered, though that can be addressed later.

> -- 
> 2.43.0