Thread (12 messages) 12 messages, 6 authors, 2026-01-30

Re: [PATCH net v2] net/smc: fix one NULL pointer dereference in smc_ib_is_sg_need_sync()

From: "D. Wythe" <alibuda@linux.alibaba.com >
Date: 2026-01-28 05:51:00
Also in: linux-rdma, linux-s390

On Tue, Jan 27, 2026 at 02:00:04PM +0200, Leon Romanovsky wrote:
On Mon, Jan 26, 2026 at 12:45:01PM +0800, D. Wythe wrote:
quoted
On Tue, Sep 09, 2025 at 12:45:32PM +0300, Leon Romanovsky wrote:
quoted
On Thu, Aug 28, 2025 at 08:41:17PM +0800, Liu Jian wrote:
quoted
BUG: kernel NULL pointer dereference, address: 00000000000002ec
PGD 0 P4D 0
Oops: Oops: 0000 [#1] SMP PTI
CPU: 28 UID: 0 PID: 343 Comm: kworker/28:1 Kdump: loaded Tainted: G        OE       6.17.0-rc2+ #9 NONE
Tainted: [O]=OOT_MODULE, [E]=UNSIGNED_MODULE
Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 1.15.0-1 04/01/2014
Workqueue: smc_hs_wq smc_listen_work [smc]
RIP: 0010:smc_ib_is_sg_need_sync+0x9e/0xd0 [smc]
...
Call Trace:
 <TASK>
 smcr_buf_map_link+0x211/0x2a0 [smc]
 __smc_buf_create+0x522/0x970 [smc]
 smc_buf_create+0x3a/0x110 [smc]
 smc_find_rdma_v2_device_serv+0x18f/0x240 [smc]
 ? smc_vlan_by_tcpsk+0x7e/0xe0 [smc]
 smc_listen_find_device+0x1dd/0x2b0 [smc]
 smc_listen_work+0x30f/0x580 [smc]
 process_one_work+0x18c/0x340
 worker_thread+0x242/0x360
 kthread+0xe7/0x220
 ret_from_fork+0x13a/0x160
 ret_from_fork_asm+0x1a/0x30
 </TASK>

If the software RoCE device is used, ibdev->dma_device is a null pointer.
As a result, the problem occurs. Null pointer detection is added to
prevent problems.

Fixes: 0ef69e788411c ("net/smc: optimize for smc_sndbuf_sync_sg_for_device and smc_rmb_sync_sg_for_cpu")
Signed-off-by: Liu Jian <redacted>
---
v1->v2:
move the check outside of loop.
 net/smc/smc_ib.c | 3 +++
 1 file changed, 3 insertions(+)
diff --git a/net/smc/smc_ib.c b/net/smc/smc_ib.c
index 53828833a3f7..a42ef3f77b96 100644
--- a/net/smc/smc_ib.c
+++ b/net/smc/smc_ib.c
@@ -742,6 +742,9 @@ bool smc_ib_is_sg_need_sync(struct smc_link *lnk,
 	unsigned int i;
 	bool ret = false;
 
+	if (!lnk->smcibdev->ibdev->dma_device)
+		return ret;
Please use ib_uses_virt_dma() function for that.

It is clearly stated in the code:
  2784 struct ib_device {
  2785         /* Do not access @dma_device directly from ULP nor from HW drivers. */
  2786         struct device                *dma_device;     

Thanks
Hi Leon,

Sorry for the late reply, I just noticed this and thank you for your review.
I agree completely with your feedback: we should not be accessing ibdev->dma_device
directly. Following your advice, replacing the

	if (!lnk->smcibdev->ibdev->dma_device)

check with ib_uses_virt_dma() is straightforward and absolutely the correct
thing to do for that part of the logic.

However, this has led me to a further challenge. The main purpose of the
smc_ib_is_sg_need_sync() function is to get the result of dma_need_sync().
This means that even after correctly using ib_uses_virt_dma(), the function
still needs to call dma_need_sync(ibdev->dma_device, ...), which forces us
right back into the direct access pattern we are trying to eliminate.
I would like to challenge the use of dma_need_sync() in smc_ib.c. From
what I see, it is used to avoid calls to dma_sync_single_*_device()
which will be NOP anyway in that case.

Why did you copy that logic from XSK?
You are right. I just noticed that the DMA has already introduced a
similar optimization.

The check in SMC was added back in 2022, while the DMA introduced
the internal "skip redundant sync" mechanism in 2024 (commit f406c8e4b770).

Given this, it might be better to simply remove this redundant check
from SMC now, which would also eliminate the need for direct access to
ibdev->dma_device. I need to perform some tests to confirm this.

Thanks for your feedback.

D. Wythe
quoted
Since the RDMA doesn't currently offer a helper for this "check" functionality,
I'd like to propose adding one. What are your thoughts on a new helper like
the one below, which would allow us to solve this cleanly?

/* ib_verbs.h */
static inline bool ib_dma_need_sync(struct ib_device *dev, dma_addr_t dma_addr) {
	return !ib_uses_virt_dma(dev) && dma_need_sync(dev->dma_device, dma_addr);
}
If we're discussing wrappers, it's likely better to provide a wrapper around
dma_sync_sg_for_cpu() for use in ib_dma_sync_sg_for_cpu(), rather than
open‑coding the logic.

Thanks
quoted
Best wishes,
D. Wythe
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help