From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mx0a-001b2d01.pphosted.com (mx0a-001b2d01.pphosted.com [148.163.156.1]) (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 2C0E332E757; Fri, 14 Aug 2026 05:51:12 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=148.163.156.1 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1786686674; cv=none; b=XBqgss03q9peLuupFJnsABKJdt/eNKRTA3bXV1KeGcHl9soxbgM9UTsoMbPTI68ow20ZQ0EG+mhPeXzP5su6NeynKWF5MaZwwf3HVVTYPc/OmoUqgX1obkToGAx4x/shU+4WQx80kRBVkb9cnW06NB4u6+HwD8tR4U6MOyaR1vk= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1786686674; c=relaxed/simple; bh=legwQO67Wx71fu+HcFSnTK9KLw55ULYVh2jQ3gdEnkw=; h=Message-ID:Date:MIME-Version:Subject:To:Cc:References:From: In-Reply-To:Content-Type; b=Hyl/ga5DI6jO5uVrbE+O0LA4wlhX+AT3te0M9aGIWVnjchPKD8+N1fsN+lN5Ce3WXlNqYDL/fAkmwAaqXb1vDTKRjDu4JG9faNRpoNfb/4mKBEDrqxfvscQpCpWYaDUvZmLMic/aXED5BFFTHkVW6XpW/JwZqTr6XP4to2yeomg= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=linux.ibm.com; spf=pass smtp.mailfrom=linux.ibm.com; dkim=pass (2048-bit key) header.d=ibm.com header.i=@ibm.com header.b=DXW7deOQ; arc=none smtp.client-ip=148.163.156.1 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=linux.ibm.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=linux.ibm.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=ibm.com header.i=@ibm.com header.b="DXW7deOQ" Received: from pps.filterd (m0353729.ppops.net [127.0.0.1]) by mx0a-001b2d01.pphosted.com (8.18.1.11/8.18.1.11) with ESMTP id 67E4WC8w2472364; Fri, 14 Aug 2026 05:50:59 GMT DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=ibm.com; h=cc :content-transfer-encoding:content-type:date:from:in-reply-to :message-id:mime-version:references:subject:to; s=pp1; bh=CDv10R kwwxEWVAEmBijpke427km5/zn9Zt9PS5qoPJM=; b=DXW7deOQMqRw7k92qWa7FZ 5J7C77K+xL9io4YoawFXfFbXPf3kj7m5hBYBI90uY8GbwjRt+r1DP0nPaOfdQYWQ ms2dbe69YTcc6acEdY1mAS3u+V/6jT/9LziIxufb4Nht4PWUjx2izuY00JWdHMbW gpsKnsWZ/j7BPhryzFZy37k1yBemKg7hFDBKY+nUeLJ8O4iNGUHXH8l/rKFlvwcB WRCNHpNRR4ZagANVhEHYmbZfWbHOcex4VdRD5gTnKwAOewIhSKRr2aB9qQx+CGq3 lm80wke0WamFtdAsYhhzF0NiJ9fLHsFBnS3std852abg5ugkkcM0capHmtb5jHkw == Received: from ppma12.dal12v.mail.ibm.com (dc.9e.1632.ip4.static.sl-reverse.com [50.22.158.220]) by mx0a-001b2d01.pphosted.com (PPS) with ESMTPS id 4fwvk0bc6h-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Fri, 14 Aug 2026 05:50:58 +0000 (GMT) Received: from pps.filterd (ppma12.dal12v.mail.ibm.com [127.0.0.1]) by ppma12.dal12v.mail.ibm.com (8.18.1.7/8.18.1.7) with ESMTP id 67E5fNmK007894; Fri, 14 Aug 2026 05:50:57 GMT Received: from smtprelay05.fra02v.mail.ibm.com ([9.218.2.225]) by ppma12.dal12v.mail.ibm.com (PPS) with ESMTPS id 4fxesqe7q6-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Fri, 14 Aug 2026 05:50:57 +0000 (GMT) Received: from smtpav01.fra02v.mail.ibm.com (smtpav01.fra02v.mail.ibm.com [10.20.54.100]) by smtprelay05.fra02v.mail.ibm.com (8.14.9/8.14.9/NCO v10.0) with ESMTP id 67E5oqD651053052 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK); Fri, 14 Aug 2026 05:50:52 GMT Received: from smtpav01.fra02v.mail.ibm.com (unknown [127.0.0.1]) by IMSVA (Postfix) with ESMTP id E5B2A20040; Fri, 14 Aug 2026 05:50:51 +0000 (GMT) Received: from smtpav01.fra02v.mail.ibm.com (unknown [127.0.0.1]) by IMSVA (Postfix) with ESMTP id C81A72004B; Fri, 14 Aug 2026 05:50:48 +0000 (GMT) Received: from [9.123.7.41] (unknown [9.123.7.41]) by smtpav01.fra02v.mail.ibm.com (Postfix) with ESMTP; Fri, 14 Aug 2026 05:50:48 +0000 (GMT) Message-ID: <5843e6e1-6d9c-4e12-810a-b1b898238497@linux.ibm.com> Date: Fri, 14 Aug 2026 11:20:47 +0530 Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH net v4] net/smc: order the CDC receive path against buffer publication To: hexlabsecurity@proton.me, Tony Lu , Paolo Abeni , Eric Dumazet , "David S. Miller" , "D. Wythe" , Wen Gu , Jakub Kicinski , Mahanta Jambigi , Dust Li Cc: Hans Wippel , netdev@vger.kernel.org, linux-kernel@vger.kernel.org, linux-s390@vger.kernel.org, Wenjia Zhang , linux-rdma@vger.kernel.org, Ursula Braun , Simon Horman References: <20260728-b4-disp-52ee4e7d-v4-1-0dda94b0f397@proton.me> Content-Language: en-US From: Sidraya Jayagond In-Reply-To: <20260728-b4-disp-52ee4e7d-v4-1-0dda94b0f397@proton.me> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-TM-AS-GCONF: 00 X-Proofpoint-Reinject: loops=2 maxloops=12 X-Proofpoint-Spam-Details-Enc: AW1haW4tMjYwODE0MDA0MSBTYWx0ZWRfXwHlR1tySlO6y 8C3bBcomIB3V113GWeAi9bX/nt+v8wqwlFX3Yj1y21cRebhF1Qz7xbLFcwI6xxZ9IS4zXpP/qK1 Cj0VuW4FTgXWGYXHJ2/5wKjDnG2v8tF3juyVrBWYw7b/WSU2rfGfmqKGOVME+7jUSC4IAjxLBMJ 4F+LzhdI27FApJp1/cT5/aOWkbIZT1Y9YyNCCvSsC9Sp5K1x4FjaOu09dY4b23l7V4YH6m2c0qs LuV1hBUJLMZCG9dRjJUtLzp6AoS2S5H3HI+0DTUwY7mZvw6Vj8k2+Ix76bhj+oZ8RkHtDJmtQFU rknTJ9JIRcQkSMIaM1yfTX5QnPQ0FgM0Cb0B6cyw4LPWVJSD7IYe3tjxqPy2oiwpL/F6VIqmhxx XsTjF4/cbzDl6q0hbEx67Xev+uuHUHWy4Yu9RtHO1XLHBkQMO5guhe4uOsEDqrstoOaqLzjwMvt gYmGMdVONErigTzSd2w== X-Proofpoint-Spam-Info: AW1haW4tMjYwODE0MDA0MSBTYWx0ZWRfX3sfxsh1TyByl I8uYknlJW2ZdpQ/rNOa5uoZtfze5m/ghK9SjeLDcXMoC95fv7ZlxneXc+P7oWbblyda/xkF/4Nk //TtdSYSrcPTyBP36VMTlCdgOEpSKRo= X-Authority-Analysis: v=2.4 cv=RqD16imK c=1 sm=1 tr=0 ts=6a7eacc3 cx=c_pps a=bLidbwmWQ0KltjZqbj+ezA==:117 a=bLidbwmWQ0KltjZqbj+ezA==:17 a=IkcTkHD0fZMA:10 a=Sv0fKeRqtYgA:10 a=VkNPw1HP01LnGYTKEx00:22 a=RnoormkPH1_aCDwRdu11:22 a=uAbxVGIbfxUO_5tXvNgY:22 a=c92rfblmAAAA:8 a=VwQbUJbxAAAA:8 a=VnNF1IyMAAAA:8 a=RyPN-GdZV6BfmeyMKGwA:9 a=QEXdDO2ut3YA:10 a=GvGzcOZaWPEFPQC_NcjD:22 X-Proofpoint-GUID: Op55Yzd43VFgMXqy4f7Os2h-yCtYdmNT X-Proofpoint-ORIG-GUID: H3WQVXCp-wHl0X8X52LiU0RxKF7Hsg2V X-Proofpoint-Virus-Version: vendor=baseguard engine=ICAP:2.0.293,Aquarius:18.0.1176,Hydra:6.1.134,FMLib:17.12.100.49 definitions=2026-08-14_02,2026-08-12_01,2025-10-01_01 X-Proofpoint-Spam-Details: rule=outbound_notspam policy=outbound score=0 phishscore=0 priorityscore=1501 suspectscore=0 lowpriorityscore=0 clxscore=1015 adultscore=0 bulkscore=0 malwarescore=0 impostorscore=0 spamscore=0 classifier=typeunknown authscore=0 authtc= authcc= route=outbound adjust=0 reason=mlx scancount=1 engine=8.22.0-2606150000 definitions=main-2608140041 On 28/07/26 10:22 pm, Bryam Vargas via B4 Relay wrote: > From: Bryam Vargas > > The SMC CDC receive handlers dereference conn->rmb_desc, and on the > SMC-D DMB-nocopy path conn->sndbuf_desc, but both are published after the > connection is already reachable to a peer: rmb_desc once the connection > is in the link group's token tree, the nocopy ghost sndbuf_desc later > still, in smcd_buf_attach() after the ISM receive tasklet is armed. A CDC > in that window hits a handler with the buffer unset -- a NULL dereference > and host DoS -- or, on a weakly ordered CPU, non-NULL but not yet > initialised. Both are also published before the receive state > (bytes_to_rcv, sndbuf_space), so an early CDC's accounting can be > overwritten by setup. > > Initialise the receive state first and publish both buffers last with > smp_store_release(), consuming them with smp_load_acquire() and bailing > while unset, as the handlers already do for a killed connection. Gate the > whole sndbuf consumer trigger on the send buffer, not just the nocopy > accounting: smc_tx_prepared_sends() and smc_tx_pending() dereference it > too. Conforming peers are unaffected. > > Fixes: 69cb7dc0218b ("net/smc: add common buffer size in send and receive buffer descriptors") > Closes: https://sashiko.dev/#/patchset/20260714-b4-disp-835288a6-v2-1-581555ef2145@proton.me?part=1 > Cc: stable@vger.kernel.org > Signed-off-by: Bryam Vargas > --- > v4: Add the Fixes: tag Jakub asked for. 69cb7dc0218b is where the CDC path > started reading the buffer length through a descriptor pointer -- it replaced > conn->rmbe_size and conn->sndbuf_size with conn->rmb_desc->len and > conn->sndbuf_desc->len -- so it is the first commit where an unpublished > buffer can be dereferenced here. The token lookup itself is older > (5f08318f617b, 2017), but it read a scalar, so there was nothing to > dereference. The SMC-D DMB-nocopy hunks cover the ghost sndbuf_desc, added > by ae2be35cbed2; a tree without that commit needs only the rmb_desc part. > v3: https://lore.kernel.org/all/20260716-b4-disp-aa52955a-v3-1-03a4411a7549@proton.me/ > - Publish rmb_desc and the ghost sndbuf_desc after the receive state is > initialised, not before. The earlier revision released the pointer first, > which let an early CDC's accounting be overwritten by setup. > - Gate the whole sndbuf consumer trigger on sndbuf_desc, not only the nocopy > accounting: smc_tx_prepared_sends() and smc_tx_pending() dereference it too. > The v2 review raised both. > v2: https://lore.kernel.org/all/20260714-b4-disp-835288a6-v2-1-581555ef2145@proton.me/ > v1: https://lore.kernel.org/all/20260711-b4-disp-c36a9798-v1-1-340b0c6053fb@proton.me/ > > herd7 models both orderings. Plain accesses allow the "pointer published, buffer > stale" outcome and flag a data race; release/acquire forbid it. A publish-order > litmus shows the lost update is allowed with the store released first and never > with it released last. af_smc runs over an RDMA fabric or an ISM device, so the > weak-memory arm is model-level; litmus tests and reproducer on request. > > Happy to split rmb/sndbuf for a cleaner stable backport. > --- > net/smc/smc_cdc.c | 50 +++++++++++++++++++++++++++++++++++++++++--------- > net/smc/smc_core.c | 26 ++++++++++++++++++++++---- > 2 files changed, 63 insertions(+), 13 deletions(-) > > diff --git a/net/smc/smc_cdc.c b/net/smc/smc_cdc.c > index 32d6d03df321..ea61b1e75c72 100644 > --- a/net/smc/smc_cdc.c > +++ b/net/smc/smc_cdc.c > @@ -332,8 +332,20 @@ static void smc_cdc_msg_recv_action(struct smc_sock *smc, > { > union smc_host_cursor cons_old, prod_old; > struct smc_connection *conn = &smc->conn; > + struct smc_buf_desc *sndbuf_desc; > int diff_cons, diff_prod, diff_tx; > > + /* Acquire the send buffer once, pairing with the smp_store_release() in > + * __smc_buf_create()/smcd_buf_attach(). On the SMC-D DMB-nocopy path > + * the ghost sndbuf_desc is attached only after the connection is already > + * reachable to the ISM device, so it can still be unset here; every > + * sndbuf_desc consumer below (the nocopy accounting and the sndbuf > + * consumer trigger, which dereferences it via smc_tx_prepared_sends()) > + * is skipped while it is NULL to avoid a NULL deref and a load of an > + * uninitialised buffer. > + */ > + sndbuf_desc = smp_load_acquire(&conn->sndbuf_desc); > + > smc_curs_copy(&prod_old, &conn->local_rx_ctrl.prod, conn); > smc_curs_copy(&cons_old, &conn->local_rx_ctrl.cons, conn); > smc_cdc_msg_to_host(&conn->local_rx_ctrl, cdc, conn); > @@ -351,14 +363,17 @@ static void smc_cdc_msg_recv_action(struct smc_sock *smc, > > /* if local sndbuf shares the same memory region with > * peer RMB, then update tx_curs_fin and sndbuf_space > - * here since peer has already consumed the data. > + * here since peer has already consumed the data. The ghost > + * sndbuf_desc (acquired above) may still be unset in the SMC-D > + * DMB-nocopy setup window, so skip the update while it is NULL. > */ > if (conn->lgr->is_smcd && > - smc_ism_support_dmb_nocopy(conn->lgr->smcd)) { > + smc_ism_support_dmb_nocopy(conn->lgr->smcd) && > + sndbuf_desc) { > /* Calculate consumed data and > * increment free send buffer space. > */ > - diff_tx = smc_curs_diff(conn->sndbuf_desc->len, > + diff_tx = smc_curs_diff(sndbuf_desc->len, > &conn->tx_curs_fin, > &conn->local_rx_ctrl.cons); > /* increase local sndbuf space and fin_curs */ > @@ -391,10 +406,15 @@ static void smc_cdc_msg_recv_action(struct smc_sock *smc, > conn->urg_state = SMC_URG_NOTYET; > } > > - /* trigger sndbuf consumer: RDMA write into peer RMBE and CDC */ > - if ((diff_cons && smc_tx_prepared_sends(conn)) || > - conn->local_rx_ctrl.prod_flags.cons_curs_upd_req || > - conn->local_rx_ctrl.prod_flags.urg_data_pending) { > + /* trigger sndbuf consumer: RDMA write into peer RMBE and CDC. > + * smc_tx_prepared_sends() and smc_tx_pending() dereference sndbuf_desc, > + * so skip the whole trigger while it is unset (the SMC-D DMB-nocopy > + * setup window): there is nothing to send without a send buffer. > + */ > + if (sndbuf_desc && > + ((diff_cons && smc_tx_prepared_sends(conn)) || > + conn->local_rx_ctrl.prod_flags.cons_curs_upd_req || > + conn->local_rx_ctrl.prod_flags.urg_data_pending)) { > if (!sock_owned_by_user(&smc->sk)) > smc_tx_pending(conn); > else > @@ -443,13 +463,21 @@ static void smcd_cdc_rx_tsklet(struct tasklet_struct *t) > { > struct smc_connection *conn = from_tasklet(conn, t, rx_tsklet); > struct smcd_cdc_msg *data_cdc; > + struct smc_buf_desc *rmb_desc; > struct smcd_cdc_msg cdc; > struct smc_sock *smc; > > if (!conn || conn->killed) > return; > + /* Pair with smp_store_release() in __smc_buf_create(): the connection > + * is published before its RMB is allocated, so bail while rmb_desc is > + * unset to avoid a NULL deref and a load of an uninitialised buffer. > + */ > + rmb_desc = smp_load_acquire(&conn->rmb_desc); > + if (!rmb_desc) > + return; > > - data_cdc = (struct smcd_cdc_msg *)conn->rmb_desc->cpu_addr; > + data_cdc = (struct smcd_cdc_msg *)rmb_desc->cpu_addr; > smcd_curs_copy(&cdc.prod, &data_cdc->prod, conn); > smcd_curs_copy(&cdc.cons, &data_cdc->cons, conn); > smc = container_of(conn, struct smc_sock, conn); > @@ -483,7 +511,11 @@ static void smc_cdc_rx_handler(struct ib_wc *wc, void *buf) > lgr = smc_get_lgr(link); > read_lock_bh(&lgr->conns_lock); > conn = smc_lgr_find_conn(ntohl(cdc->token), lgr); > - if (!conn || conn->out_of_sync) { > + /* Pair with smp_store_release() in __smc_buf_create(): bail while the > + * RMB is unset (smc_cdc_msg_recv_action() dereferences it) to avoid a > + * NULL deref and a stale-buffer read in the connection setup window. > + */ > + if (!conn || conn->out_of_sync || !smp_load_acquire(&conn->rmb_desc)) { > read_unlock_bh(&lgr->conns_lock); > return; > } > diff --git a/net/smc/smc_core.c b/net/smc/smc_core.c > index b4208cb186c5..33bf9cc979ef 100644 > --- a/net/smc/smc_core.c > +++ b/net/smc/smc_core.c > @@ -2499,15 +2499,26 @@ static int __smc_buf_create(struct smc_sock *smc, bool is_smcd, bool is_rmb) > } > > if (is_rmb) { > - conn->rmb_desc = buf_desc; > conn->rmbe_size_comp = bufsize_comp; > smc->sk.sk_rcvbuf = bufsize * 2; > atomic_set(&conn->bytes_to_rcv, 0); > conn->rmbe_update_limit = > smc_rmb_wnd_update_limit(buf_desc->len); > + /* Publish the receive buffer last, with release semantics: the > + * connection is already in the link group's token tree, so a > + * concurrent CDC receive handler must observe the fully > + * initialised receive state above (and the buffer) once it sees > + * a non-NULL rmb_desc. Pairs with the smp_load_acquire() in the > + * CDC receive path. > + */ > + smp_store_release(&conn->rmb_desc, buf_desc); > if (is_smcd) > smc_ism_set_conn(conn); /* map RMB/smcd_dev to conn */ > } else { > + /* Plain store: this send-buffer pass runs before the RMB pass, > + * whose smp_store_release(&conn->rmb_desc) then publishes this > + * store too, and the CDC receive path is gated on rmb_desc. > + */ > conn->sndbuf_desc = buf_desc; > smc->sk.sk_sndbuf = bufsize * 2; > atomic_set(&conn->sndbuf_space, bufsize); > @@ -2599,9 +2610,16 @@ int smcd_buf_attach(struct smc_sock *smc) > buf_desc->cpu_addr = > (u8 *)buf_desc->cpu_addr + sizeof(struct smcd_cdc_msg); > buf_desc->len -= sizeof(struct smcd_cdc_msg); > - conn->sndbuf_desc = buf_desc; > - conn->sndbuf_desc->used = 1; > - atomic_set(&conn->sndbuf_space, conn->sndbuf_desc->len); > + buf_desc->used = 1; > + atomic_set(&conn->sndbuf_space, buf_desc->len); > + /* Publish the ghost send buffer last, with release semantics: the > + * connection is already reachable to the ISM device (smc_ism_set_conn() > + * ran in __smc_buf_create()), so the CDC receive tasklet must observe > + * the fully initialised ghost buffer once it sees a non-NULL > + * sndbuf_desc. Pairs with smp_load_acquire() in > + * smc_cdc_msg_recv_action(). > + */ > + smp_store_release(&conn->sndbuf_desc, buf_desc); > return 0; > > free: > > --- > base-commit: e095f249e2209674f6366f6db0383a2b96e19239 > change-id: 20260728-b4-disp-52ee4e7d-348754fe91ed > > Best regards, > -- > Bryam Vargas > > > Reviewed-by: Sidraya Jayagond Thank You, Sidraya