[PATCH net] net/smc: prevent TCP from retaining freed address-family ops

[PATCH net] net/smc: prevent TCP from retaining freed address-family ops

[Kyle Zeng]

Yue Sun reported a KASAN use-after-free in tcp_sync_mss() after an
AF_SMC socket entered TCP fallback and listen failed. smc_listen()
installs an address-family operations table embedded in smc_sock, but
its error path leaves the table installed. TCP can outlive the SMC
socket and dereference the freed table. Retrying listen also saves the
SMC wrapper as the original operations and can recurse indefinitely.

Restore the original operations after a failed listen and before
releasing the internal TCP socket. The latter also covers a fallback
listener reused as an active TCP socket. Serialize installation and
restoration with the TCP socket lock, and restore only if the embedded
table is still installed, preserving a concurrent address-family
conversion.

Enable the existing RCU-delayed SMC socket destruction before publishing
the listener callbacks, including on failed listen. Compare a child's
operations with the embedded table itself so that concurrent restoration
of the parent's operations cannot leave the child holding that table.

SMC fallback exposes the kernel TCP socket through the socket file.
Reject attaching the MPTCP ULP to such a socket: otherwise userspace
can bypass smc_setsockopt() and MPTCP can retain the embedded table in
its subflow context. MPTCP's own creation path still attaches its ULP
before the new kernel socket is associated with a file.

A standalone userspace reproducer forces listen to fail with
EADDRINUSE, enters TCP fallback, and closes the SMC owner with data
queued behind a zero receive window. A TCP probe then reports a KASAN
use-after-free in __tcp_transmit_skb() when reading net_header_len.
The same binary completes without a KASAN report after this change.
The complete x86_64 SMC and MPTCP code was compiled.

Fixes: 8270d9c21041 ("net/smc: Limit backlog connections")
Fixes: 2303f994b3e1 ("mptcp: Associate MPTCP context with TCP socket")
Reported-by: Yue Sun <samsun1006219@gmail.com>
Closes: https://lore.kernel.org/netdev/20260713085238.16780-1-samsun1006219@gmail.com/
Cc: stable@vger.kernel.org
Assisted-by: Codex:gpt-6-astra
Signed-off-by: Kyle Zeng <kylebot@openai.com>
---
 net/mptcp/subflow.c |  7 ++++---
 net/smc/af_smc.c    | 14 +++++++++++---
 net/smc/smc_close.c |  6 ++++++
 3 files changed, 21 insertions(+), 6 deletions(-)

diff --git a/net/mptcp/subflow.c b/net/mptcp/subflow.c
index f0a6725d2c37..da7ac71344a9 100644
--- a/net/mptcp/subflow.c
+++ b/net/mptcp/subflow.c
@@ -1984,10 +1984,11 @@ static int subflow_ulp_init(struct sock *sk)
 	struct tcp_sock *tp = tcp_sk(sk);
 	int err = 0;
 
-	/* disallow attaching ULP to a socket unless it has been
-	 * created with sock_create_kern()
+	/* Only attach to a kernel-created socket that has not been
+	 * exposed through a file.
 	 */
-	if (!sk->sk_kern_sock) {
+	if (!sk->sk_kern_sock ||
+	    (sk->sk_socket && READ_ONCE(sk->sk_socket->file))) {
 		err = -EOPNOTSUPP;
 		goto out;
 	}
diff --git a/net/smc/af_smc.c b/net/smc/af_smc.c
index e9f93b3ab435..dbbe7d6574e4 100644
--- a/net/smc/af_smc.c
+++ b/net/smc/af_smc.c
@@ -157,7 +157,7 @@ static struct sock *smc_tcp_syn_recv_sock(const struct sock *sk,
 		rcu_assign_sk_user_data(child, NULL);
 
 		/* v4-mapped sockets don't inherit parent ops. Don't restore. */
-		if (inet_csk(child)->icsk_af_ops == inet_csk(sk)->icsk_af_ops)
+		if (inet_csk(child)->icsk_af_ops == &smc->af_ops)
 			inet_csk(child)->icsk_af_ops = smc->ori_af_ops;
 	}
 	sock_put(&smc->sk);
@@ -2671,6 +2671,8 @@ int smc_listen(struct socket *sock, int backlog)
 	if (!smc->use_fallback)
 		tcp_sk(smc->clcsock->sk)->syn_smc = 1;
 
+	sock_set_flag(sk, SOCK_RCU_FREE);
+
 	/* save original sk_data_ready function and establish
 	 * smc-specific sk_data_ready function
 	 */
@@ -2682,18 +2684,25 @@ int smc_listen(struct socket *sock, int backlog)
 	write_unlock_bh(&smc->clcsock->sk->sk_callback_lock);
 
 	/* save original ops */
+	lock_sock(smc->clcsock->sk);
 	smc->ori_af_ops = inet_csk(smc->clcsock->sk)->icsk_af_ops;
 
 	smc->af_ops = *smc->ori_af_ops;
 	smc->af_ops.syn_recv_sock = smc_tcp_syn_recv_sock;
 
-	inet_csk(smc->clcsock->sk)->icsk_af_ops = &smc->af_ops;
+	WRITE_ONCE(inet_csk(smc->clcsock->sk)->icsk_af_ops, &smc->af_ops);
+	release_sock(smc->clcsock->sk);
 
 	if (smc->limit_smc_hs)
 		tcp_sk(smc->clcsock->sk)->smc_hs_congested = smc_hs_congested;
 
 	rc = kernel_listen(smc->clcsock, backlog);
 	if (rc) {
+		lock_sock(smc->clcsock->sk);
+		if (inet_csk(smc->clcsock->sk)->icsk_af_ops == &smc->af_ops)
+			WRITE_ONCE(inet_csk(smc->clcsock->sk)->icsk_af_ops,
+				   smc->ori_af_ops);
+		release_sock(smc->clcsock->sk);
 		write_lock_bh(&smc->clcsock->sk->sk_callback_lock);
 		smc_clcsock_restore_cb(&smc->clcsock->sk->sk_data_ready,
 				       &smc->clcsk_data_ready);
@@ -2701,7 +2710,6 @@ int smc_listen(struct socket *sock, int backlog)
 		write_unlock_bh(&smc->clcsock->sk->sk_callback_lock);
 		goto out;
 	}
-	sock_set_flag(sk, SOCK_RCU_FREE);
 	sk->sk_max_ack_backlog = backlog;
 	sk->sk_ack_backlog = 0;
 	sk->sk_state = SMC_LISTEN;
diff --git a/net/smc/smc_close.c b/net/smc/smc_close.c
index bb0313ef5f7c..c59e578f3e52 100644
--- a/net/smc/smc_close.c
+++ b/net/smc/smc_close.c
@@ -24,12 +24,18 @@
 void smc_clcsock_release(struct smc_sock *smc)
 {
 	struct socket *tcp;
+	struct sock *sk;
 
 	if (smc->listen_smc && current_work() != &smc->smc_listen_work)
 		cancel_work_sync(&smc->smc_listen_work);
 	mutex_lock(&smc->clcsock_release_lock);
 	if (smc->clcsock) {
 		tcp = smc->clcsock;
+		sk = tcp->sk;
+		lock_sock(sk);
+		if (inet_csk(sk)->icsk_af_ops == &smc->af_ops)
+			WRITE_ONCE(inet_csk(sk)->icsk_af_ops, smc->ori_af_ops);
+		release_sock(sk);
 		smc->clcsock = NULL;
 		sock_release(tcp);
 	}
-- 
2.55.0.openai.867.ga7d5542d7eda

Re: [PATCH net] net/smc: prevent TCP from retaining freed address-family ops

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 2 potential
issue(s) to consider.

Critical: 0 · High: 2 (1) · Medium: 0 · Low: 0

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [High] smc_listen() still installs &smc->af_ops on a clcsock that is
  already a live TCP listener, so an accepted child can keep the…
- [High] The new MPTCP gate in subflow_ulp_init() rejects exposed sockets
  only if it sees `sk->sk_socket->file != NULL`.

Pre-existing issues:
- [High] The problem wasn't introduced by this patch, but smc_listen()
  publishes &smc->af_ops with WRITE_ONCE() after plain stores to…

--

net/smc: prevent TCP from retaining freed address-family ops

This patch restores the original icsk_af_ops when smc_listen() fails and
in smc_clcsock_release(). Installing and restoring the ops is serialized
with the clcsock lock, and SOCK_RCU_FREE is now set before the listener
callbacks are published.

It also compares a child's ops against the embedded table in
smc_tcp_syn_recv_sock(). The MPTCP ULP can no longer be attached to a
kernel socket that has been exposed through a file.

Closes: https://lore.kernel.org/netdev/20260713085238.16780-1-samsun1006219@gmail.com/

> diff --git a/net/mptcp/subflow.c b/net/mptcp/subflow.c
> index f0a6725d2c37..da7ac71344a9 100644
> --- a/net/mptcp/subflow.c
> +++ b/net/mptcp/subflow.c
> @@ -1984,10 +1984,11 @@ static int subflow_ulp_init(struct sock *sk)
>  	struct tcp_sock *tp = tcp_sk(sk);
>  	int err = 0;
>  
> -	/* disallow attaching ULP to a socket unless it has been
> -	 * created with sock_create_kern()
> +	/* Only attach to a kernel-created socket that has not been
> +	 * exposed through a file.
>  	 */
> -	if (!sk->sk_kern_sock) {
> +	if (!sk->sk_kern_sock ||
> +	    (sk->sk_socket && READ_ONCE(sk->sk_socket->file))) {

[Severity: High]

Does this check hold up against a concurrent smc_switch_to_fallback()?

smc_switch_to_fallback() exposes the clcsock with two plain stores and no
barrier between them:

net/smc/af_smc.c:smc_switch_to_fallback() {
    ...
	if (smc->sk.sk_socket && smc->sk.sk_socket->file) {
		smc->clcsock->file = smc->sk.sk_socket->file;
		smc->clcsock->file->private_data = smc->clcsock;
    ...
}

By then the file is already in the fd table. Another thread calling
setsockopt(fd, SOL_TCP, TCP_ULP, "mptcp") finds the clcsock through
sock_from_file(), which does a plain load of file->private_data. That
thread holds neither clcsock_release_lock nor the SMC sock lock.

The compiler may reorder those two stores, and so may arm64 or POWER. In
that case, can the second thread see private_data == clcsock while
clcsock->file still reads NULL?

The clcsock came from sock_create_kern(), so sk_kern_sock is 1 and both
checks would pass.

The MPTCP ULP would then attach to a TCP socket that userspace can reach
but that has no MPTCP parent, so ctx->conn is NULL. subflow_data_ready()
starts with:

	struct sock *parent = subflow->conn;

and subflow_state_change() also uses the parent. ctx->icsk_af_ops could
also capture the SMC-embedded table.

The commit message says:

    Reject attaching the MPTCP ULP to such a socket: otherwise userspace
    can bypass smc_setsockopt() and MPTCP can retain the embedded table in
    its subflow context.

Is that path still reachable through this window?

>  		err = -EOPNOTSUPP;
>  		goto out;
>  	}
> diff --git a/net/smc/af_smc.c b/net/smc/af_smc.c
> index e9f93b3ab435..dbbe7d6574e4 100644
> --- a/net/smc/af_smc.c
> +++ b/net/smc/af_smc.c

[ ... ]

> @@ -2682,18 +2684,25 @@ int smc_listen(struct socket *sock, int backlog)
>  	write_unlock_bh(&smc->clcsock->sk->sk_callback_lock);
>  
>  	/* save original ops */
> +	lock_sock(smc->clcsock->sk);
>  	smc->ori_af_ops = inet_csk(smc->clcsock->sk)->icsk_af_ops;
>  
>  	smc->af_ops = *smc->ori_af_ops;
>  	smc->af_ops.syn_recv_sock = smc_tcp_syn_recv_sock;
>  
> -	inet_csk(smc->clcsock->sk)->icsk_af_ops = &smc->af_ops;
> +	WRITE_ONCE(inet_csk(smc->clcsock->sk)->icsk_af_ops, &smc->af_ops);
> +	release_sock(smc->clcsock->sk);

[Severity: High]

Can this still install &smc->af_ops on a clcsock that is already a live
TCP listener?

Here is one possible interleaving. Three threads share an AF_SMC fd that
is in SMC_INIT:

T1: listen(fd)
    smc_listen()
        lock_sock(sk)            <- waits

T2: setsockopt(fd, SOL_TCP, TCP_FASTOPEN, ...)
    smc_setsockopt()
        smc_switch_to_fallback()
            file->private_data = clcsock
    (sk_state stays SMC_INIT)

T3: listen(fd)
    inet_listen() on the clcsock, which is now a plain TCP listener

T1 then resumes and passes the state checks in smc_listen(), then
installs &smc->af_ops. kernel_listen() returns 0 because
__inet_listen_sk() only adjusts the backlog when the socket is already in
TCP_LISTEN.

The listener receive path runs without the socket lock:

tcp_v4_rcv()
    tcp_v4_do_rcv()          (TCP_LISTEN)
    tcp_check_req()          (TCP_NEW_SYN_RECV)
        inet_csk(sk)->icsk_af_ops->syn_recv_sock()

A softirq can load the original tcp_v4_syn_recv_sock() before the
WRITE_ONCE() above. It can then clone the listener in
tcp_create_openreq_child() after it.

That child inherits &smc->af_ops but never goes through
smc_tcp_syn_recv_sock(). So the new check there never runs for it:

    if (inet_csk(child)->icsk_af_ops == &smc->af_ops)

Suppose userspace then accepts that child from the clcsock and closes the
SMC fd. smc_clcsock_release() restores only the listener's ops.

Once the RCU grace period ends and the smc_sock is freed, would the child
keep calling through the freed table (net_header_len, queue_xmit, and so
on)?

Taking lock_sock() on the clcsock serializes install against restore, but
it doesn't seem to exclude lockless listener processing. Should the
install be refused when the clcsock is already in TCP_LISTEN?

[Severity: High]

This problem wasn't introduced by this patch, but is the publication of
&smc->af_ops ordered after the stores that initialize it?

smc->ori_af_ops and the struct copy into smc->af_ops are plain stores.
The WRITE_ONCE() that publishes &smc->af_ops has no release semantics.

In the already-listening case above, the lockless reader is:

tcp_v4_rcv()
    tcp_check_req()
        inet_csk(sk)->icsk_af_ops->syn_recv_sock(...)

The compiler may reorder the plain stores, and so may arm64 or POWER.
Could the reader then see the new pointer before the struct copy is
visible?

sk_alloc() zeroes smc->af_ops, so the softirq could call a NULL
syn_recv_sock. It could also enter smc_tcp_syn_recv_sock() and
dereference a NULL smc->ori_af_ops here:

	child = smc->ori_af_ops->syn_recv_sock(sk, skb, req, dst, req_unhash,

The added lock_sock() does not synchronize with this reader, which never
takes sk_lock. Refusing to install on a socket that is already listening,
or publishing with smp_store_release(), would avoid this.

[ ... ]

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260928213153.36141-1-kylebot%40openai.com