Netdev List
 help / color / mirror / Atom feed
From: "D. Wythe" <alibuda@linux.alibaba.com    >
To: Sechang Lim <rhkrqnwk98@gmail.com>
Cc: "D . Wythe" <alibuda@linux.alibaba.com>,
	Dust Li <dust.li@linux.alibaba.com>,
	Sidraya Jayagond <sidraya@linux.ibm.com>,
	Wenjia Zhang <wenjia@linux.ibm.com>,
	"David S . Miller" <davem@davemloft.net>,
	Eric Dumazet <edumazet@google.com>,
	Jakub Kicinski <kuba@kernel.org>, Paolo Abeni <pabeni@redhat.com>,
	Mahanta Jambigi <mjambigi@linux.ibm.com>,
	Tony Lu <tonylu@linux.alibaba.com>,
	Wen Gu <guwen@linux.alibaba.com>, Simon Horman <horms@kernel.org>,
	Ursula Braun <ubraun@linux.ibm.com>,
	Karsten Graul <kgraul@linux.ibm.com>,
	Guvenc Gulce <guvenc@linux.ibm.com>,
	linux-rdma@vger.kernel.org, linux-s390@vger.kernel.org,
	netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
	bpf@vger.kernel.org
Subject: Re: [PATCH net v2] net/smc: fix out-of-bounds read when sk_user_data holds a sk_psock
Date: Tue, 23 Jun 2026 14:02:18 +0800	[thread overview]
Message-ID: <20260623060218.GA29925@j66a10360.sqa.eu95> (raw)
In-Reply-To: <20260619150342.3626224-1-rhkrqnwk98@gmail.com>

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@gmail.com>
> ---
>  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

      parent reply	other threads:[~2026-06-23  6:02 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-06-19 15:03 [PATCH net v2] net/smc: fix out-of-bounds read when sk_user_data holds a sk_psock Sechang Lim
2026-06-22 12:11 ` Jiayuan Chen
2026-06-23  6:02 ` D. Wythe [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260623060218.GA29925@j66a10360.sqa.eu95 \
    --to=alibuda@linux.alibaba.com \
    --cc=bpf@vger.kernel.org \
    --cc=davem@davemloft.net \
    --cc=dust.li@linux.alibaba.com \
    --cc=edumazet@google.com \
    --cc=guvenc@linux.ibm.com \
    --cc=guwen@linux.alibaba.com \
    --cc=horms@kernel.org \
    --cc=kgraul@linux.ibm.com \
    --cc=kuba@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-rdma@vger.kernel.org \
    --cc=linux-s390@vger.kernel.org \
    --cc=mjambigi@linux.ibm.com \
    --cc=netdev@vger.kernel.org \
    --cc=pabeni@redhat.com \
    --cc=rhkrqnwk98@gmail.com \
    --cc=sidraya@linux.ibm.com \
    --cc=tonylu@linux.alibaba.com \
    --cc=ubraun@linux.ibm.com \
    --cc=wenjia@linux.ibm.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox