From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from out-172.mta1.migadu.com (out-172.mta1.migadu.com [95.215.58.172]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 23D4C368954 for ; Mon, 9 Mar 2026 07:49:11 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=95.215.58.172 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1773042552; cv=none; b=PIlxDlh8MMvq1PV3L8Nt5AtHCBTxV34rnXZjBPZ76HOaFMPybrjN9AOTQ4jw0hxtkj6BJhKhkw0QaR7Mgi5XwU6YEF52lorge772dwdbNrYnH69tsqwvktmb6MPwIgRDr/I3NHR4oFSzNpUF7vQzVVgIXL2jetO2hTxtZZrruuE= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1773042552; c=relaxed/simple; bh=BK4w5zW2pmkFpleQWkhZHxMFzScEqMW5tGEXIvdyQDw=; h=MIME-Version:Date:Content-Type:From:Message-ID:Subject:To:Cc: In-Reply-To:References; b=LOzLljAVeWbtw2mRKzxbX2/p3OAp7m+zx91kBohdJde7QZvWf82tFeCyZvDv+HPwIVQdtqbb9JxvtdlloHXNcMegPxBYqdiiNik4ffudI3TYFkq/FbS1otOMbpxPFn1pyJ/6CM6WC8/DV0Jy77pIRSEB+uvsJRwsxgb+gDpnxb0= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=linux.dev; spf=pass smtp.mailfrom=linux.dev; dkim=pass (1024-bit key) header.d=linux.dev header.i=@linux.dev header.b=DIY1YApT; arc=none smtp.client-ip=95.215.58.172 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=linux.dev Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=linux.dev Authentication-Results: smtp.subspace.kernel.org; dkim=pass (1024-bit key) header.d=linux.dev header.i=@linux.dev header.b="DIY1YApT" Precedence: bulk X-Mailing-List: netdev@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux.dev; s=key1; t=1773042538; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=OCSi7v9xxHe6xaQA7QoLIUaLr3gtY2UScb9IicHrzKY=; b=DIY1YApTPAmh2yqbiET3733BiWelpy9U/WwuQtXrce/3cvcg2c8zS0buSnCk62q3MvR1n6 fapihjKpNeRvKoLWA52fRkX/W15XUINzyrnRjGaXo05xcYnVDhFkFfFA+sghZz+puu/vjE S+e/k2RssYQ2w4g9/oinrEIGjEhp8KM= Date: Mon, 09 Mar 2026 07:48:54 +0000 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable X-Report-Abuse: Please report any abuse attempt to abuse@migadu.com and include these headers. From: "Jiayuan Chen" Message-ID: TLS-Required: No Subject: Re: [PATCH net v2] net/smc: fix NULL dereference and UAF in smc_tcp_syn_recv_sock() To: "D. Wythe" Cc: netdev@vger.kernel.org, syzbot+827ae2bfb3a3529333e9@syzkaller.appspotmail.com, "Eric Dumazet" , "D. Wythe" , "Dust Li" , "Sidraya Jayagond" , "Wenjia Zhang" , "Mahanta Jambigi" , "Tony Lu" , "Wen Gu" , "David S. Miller" , "Jakub Kicinski" , "Paolo Abeni" , "Simon Horman" , linux-rdma@vger.kernel.org, linux-s390@vger.kernel.org, linux-kernel@vger.kernel.org In-Reply-To: <20260309060611.GA130186@j66a10360.sqa.eu95> References: <20260309023846.18516-1-jiayuan.chen@linux.dev> <20260309060611.GA130186@j66a10360.sqa.eu95> X-Migadu-Flow: FLOW_OUT March 9, 2026 at 14:06, "D. Wythe" wrote: [...] > > Reproducer was verified with mdelay injection and smc_run, > > the issue no longer occurs with this patch applied. > >=20=20 >=20> [1] https://syzkaller.appspot.com/bug?extid=3D827ae2bfb3a3529333e9 > >=20=20 >=20> Fixes: 8270d9c21041 ("net/smc: Limit backlog connections") > > Reported-by: syzbot+827ae2bfb3a3529333e9@syzkaller.appspotmail.com > > Closes: https://lore.kernel.org/all/67eaf9b8.050a0220.3c3d88.004a.GA= E@google.com/T/ > > Suggested-by: Eric Dumazet > > Signed-off-by: Jiayuan Chen > > --- > > v2: > > - Use rcu_read_lock() + refcount_inc_not_zero() instead of > > read_lock_bh(sk_callback_lock) + sock_hold(), since this > > is the TCP handshake hot path and read_lock_bh is too > > expensive under SYN flood. > > - Set SOCK_RCU_FREE on SMC listen socket to ensure > > RCU-deferred freeing. > >=20=20 >=20> v1: https://lore.kernel.org/netdev/20260307032158.372165-1-jiayuan= .chen@linux.dev/ > > --- > > net/smc/af_smc.c | 9 +++++++++ > > 1 file changed, 9 insertions(+) > >=20=20 >=20> diff --git a/net/smc/af_smc.c b/net/smc/af_smc.c > > index d0119afcc6a1..72ac1d8c62d4 100644 > > --- a/net/smc/af_smc.c > > +++ b/net/smc/af_smc.c > > @@ -131,7 +131,13 @@ static struct sock *smc_tcp_syn_recv_sock(const= struct sock *sk, > > struct smc_sock *smc; > > struct sock *child; > >=20=20 >=20> + rcu_read_lock(); > > smc =3D smc_clcsock_user_data(sk); > > + if (!smc || !refcount_inc_not_zero(&smc->sk.sk_refcnt)) { > > + rcu_read_unlock(); > > + return NULL; > > + } > > + rcu_read_unlock(); > >=20=20 >=20> if (READ_ONCE(sk->sk_ack_backlog) + atomic_read(&smc->queued_smc_h= s) > > > sk->sk_max_ack_backlog) > > @@ -153,11 +159,13 @@ static struct sock *smc_tcp_syn_recv_sock(cons= t struct sock *sk, > > if (inet_csk(child)->icsk_af_ops =3D=3D inet_csk(sk)->icsk_af_ops) > > inet_csk(child)->icsk_af_ops =3D smc->ori_af_ops; > > } > > + sock_put(&smc->sk); > > return child; > >=20=20 >=20> drop: > > dst_release(dst); > > tcp_listendrop(sk); > > + sock_put(&smc->sk); > > return NULL; > > } > >=20=20 >=20> @@ -2691,6 +2699,7 @@ int smc_listen(struct socket *sock, int back= log) > > write_unlock_bh(&smc->clcsock->sk->sk_callback_lock); > > goto out; > > } > > + sock_set_flag(sk, SOCK_RCU_FREE); > >=20 >=20This RCU approach looks good to me. Since SOCK_RCU_FREE is now enable= d, > other callers of smc_clcsock_user_data() should also follow this > RCU-based pattern. It will eventually allow us to completely remove the > annoying sk_callback_lock. >=20 >=20D. Wythe >=20 Hi=20D. Wythe, Thanks for the suggestion. I agree that converting all smc_clcsock_user_d= ata() callers to RCU is a reasonable direction, and it would allow us to eventu= ally remove the sk_callback_lock dependency for sk_user_data access. However, I'd prefer to keep this patch focused on fixing the specific bug= in smc_tcp_syn_recv_sock(), since it needs to be backported to stable trees. Mixing a bug fix with broader refactoring makes backporting harder and in= creases the risk of regressions. Also, converting the other callers is not entirely trivial. For example: - smc_fback_state_change/data_ready/write_space/error_report(): the sk_callback_lock there protects not only sk_user_data but also the consistency of the saved callback pointers (e.g.,smc->clcsk_state_chang= e). Switching to RCU requires careful ordering analysis against the write s= ide in smc_fback_restore_callbacks(). Additionally, fallback sockets would = also need SOCK_RCU_FREE. - smc_clcsock_data_ready(): the sock_hold() would need to become refcount_inc_not_zero() to handle = the case where the refcount has already reached zero. I'd like to address these conversions in a follow-up patch series. What do you think? > >=20 >=20> sk->sk_max_ack_backlog =3D backlog; > > sk->sk_ack_backlog =3D 0; > > sk->sk_state =3D SMC_LISTEN; > > --=20 >=20> 2.43.0 > > >