[PATCH] soc: fsl: dpio: publish the dpaa2_io object only after it is initialised

Subsystems: freescale soc drivers, the rest

COLD17d

3 messages, 3 authors, 17d ago · open the first message on its own page

[PATCH] soc: fsl: dpio: publish the dpaa2_io object only after it is initialised

From: Jaidev Shastri <hidden>
Date: 2026-09-22 00:50:10

From: Jaidev Shastri <redacted>

dpaa2_io_create() adds the new object to dpio_list and dpio_by_cpu[]
under dpio_list_lock, but service_select_by_cpu() reads dpio_by_cpu[]
without the lock on behalf of dpaa2_io_service_select() and
dpaa2_io_service_register().

obj->dev is assigned after the lock is dropped, so a reader can pick the
object up and pass a NULL supplier to device_link_add(), which fails
with -EINVAL and fails the consumer's probe. The publication is a plain
store, so a reader that does not take the lock is also not ordered
against the stores that set obj->swp, the notification list and the
object's spinlocks.

dpaa2-eth probes from the deferred probe worker and retries whenever
another device binds, so it runs while the remaining DPIO objects are
still being created on multi-core LS2 and LX2 parts.

Finish the object before publishing it and store dpio_by_cpu[] with
smp_store_release(), paired with smp_load_acquire() in
service_select_by_cpu(). service_select() takes the lock and is
unchanged.

Found with MBCheck, a static herd7-based memory consistency checker.

Signed-off-by: Jaidev Shastri <redacted>
---
 drivers/soc/fsl/dpio/dpio-service.c | 27 +++++++++++++++++++--------
 1 file changed, 19 insertions(+), 8 deletions(-)
diff --git a/drivers/soc/fsl/dpio/dpio-service.c b/drivers/soc/fsl/dpio/dpio-service.c
index 317ca50b0..2dbd14aed 100644
--- a/drivers/soc/fsl/dpio/dpio-service.c
+++ b/drivers/soc/fsl/dpio/dpio-service.c
@@ -70,8 +70,12 @@ static inline struct dpaa2_io *service_select_by_cpu(struct dpaa2_io *d,
 	if (cpu < 0)
 		cpu = raw_smp_processor_id();
 
-	/* If a specific cpu was requested, pick it up immediately */
-	return dpio_by_cpu[cpu];
+	/*
+	 * If a specific cpu was requested, pick it up immediately. Pairs with
+	 * the smp_store_release() in dpaa2_io_create(): the object is only
+	 * used once every field written before the publication is visible.
+	 */
+	return smp_load_acquire(&dpio_by_cpu[cpu]);
 }
 
 static inline struct dpaa2_io *service_select(struct dpaa2_io *d)
@@ -177,12 +181,6 @@ struct dpaa2_io *dpaa2_io_create(const struct dpaa2_io_desc *desc,
 	if (obj->dpio_desc.receives_notifications)
 		qbman_swp_push_set(obj->swp, 0, 1);
 
-	spin_lock(&dpio_list_lock);
-	list_add_tail(&obj->node, &dpio_list);
-	if (desc->cpu >= 0 && !dpio_by_cpu[desc->cpu])
-		dpio_by_cpu[desc->cpu] = obj;
-	spin_unlock(&dpio_list_lock);
-
 	obj->dev = dev;
 
 	memset(&obj->rx_dim, 0, sizeof(obj->rx_dim));
@@ -191,6 +189,19 @@ struct dpaa2_io *dpaa2_io_create(const struct dpaa2_io_desc *desc,
 	obj->bytes = 0;
 	obj->frames = 0;
 
+	/*
+	 * dpaa2_io_service_select() reads dpio_by_cpu[] without taking
+	 * dpio_list_lock, so the object must be complete before it is
+	 * published and the publication needs release semantics.
+	 */
+	spin_lock(&dpio_list_lock);
+	list_add_tail(&obj->node, &dpio_list);
+	if (desc->cpu >= 0 && !dpio_by_cpu[desc->cpu]) {
+		/* Pairs with the smp_load_acquire() in service_select_by_cpu(). */
+		smp_store_release(&dpio_by_cpu[desc->cpu], obj);
+	}
+	spin_unlock(&dpio_list_lock);
+
 	return obj;
 }
 
---
base-commit: 93f51579e7df248780214094418f205253383cc5
change-id: 20260921-mb-dpio-f3f7dcf0ef41

Best regards,
--  
Jaidev Shastri [off-list ref]


Re: [PATCH] soc: fsl: dpio: publish the dpaa2_io object only after it is initialised

From: Anthony Pighin <hidden>
Date: 2026-10-01 00:10:55

On Mon, Sep 22, 2026, Jaidev Shastri wrote:
obj->dev is assigned after the lock is dropped, so a reader can pick the
object up and pass a NULL supplier to device_link_add(), which fails
with -EINVAL and fails the consumer's probe.
The object comes from kmalloc(), so obj->dev is not NULL but whatever
was left in that memory. We hit this on LX2160A boards as an
intermittent boot panic, with the deferred dpaa2-eth probe racing the
DPIO probes.

  Unable to handle kernel paging request at virtual address deadbeefdeadbfd7
  pc : device_link_add+0x80/0x700
  x20: deadbeefdeadbeef
  Call trace:
   device_link_add+0x80/0x700
   dpaa2_io_service_register+0x40/0x120 [fsl_mc_dpio]
   dpaa2_eth_setup_dpio+0x128/0x468 [fsl_dpaa2_eth]
   dpaa2_eth_probe+0x1dc/0x908 [fsl_dpaa2_eth]

Widening the window with an msleep(50) after the spin_unlock() makes it
panic on every boot. With this patch applied to 6.12 and the same
msleep(50) kept after the spin_unlock(), five boots in a row were clean.

Since this crashes real systems, could v2 say so in the commit message
and carry these tags so it reaches the stable kernels?

Fixes: cf9ff75d15a9 ("soc: fsl: dpio: store a backpointer to the device backing the dpaa2_io")
Fixes: 69651bd8d303 ("soc: fsl: dpio: add Net DIM integration")
Cc: stable@vger.kernel.org

Tested-by: Anthony Pighin <redacted>

Re: [PATCH] soc: fsl: dpio: publish the dpaa2_io object only after it is initialised

From: Ioana Ciornei <ioana.ciornei@nxp.com>
Date: 2026-10-02 09:50:55

On Mon, Sep 21, 2026 at 08:50:07PM -0400, Jaidev Shastri via B4 Relay wrote:
[You don't often get email from devnull+jaidevshastri.vt.edu@kernel.org. Learn why this is important at https://aka.ms/LearnAboutSenderIdentification ]

From: Jaidev Shastri <redacted>

dpaa2_io_create() adds the new object to dpio_list and dpio_by_cpu[]
under dpio_list_lock, but service_select_by_cpu() reads dpio_by_cpu[]
without the lock on behalf of dpaa2_io_service_select() and
dpaa2_io_service_register().

obj->dev is assigned after the lock is dropped, so a reader can pick the
object up and pass a NULL supplier to device_link_add(), which fails
with -EINVAL and fails the consumer's probe. The publication is a plain
store, so a reader that does not take the lock is also not ordered
against the stores that set obj->swp, the notification list and the
object's spinlocks.

dpaa2-eth probes from the deferred probe worker and retries whenever
another device binds, so it runs while the remaining DPIO objects are
still being created on multi-core LS2 and LX2 parts.

Finish the object before publishing it and store dpio_by_cpu[] with
smp_store_release(), paired with smp_load_acquire() in
service_select_by_cpu(). service_select() takes the lock and is
unchanged.

Found with MBCheck, a static herd7-based memory consistency checker.

Signed-off-by: Jaidev Shastri <redacted>
Could you please amend the commit message so that you incorporate
Jaidev's feedback and submit a v2?

With that,

Reviewed-by: Ioana Ciornei <ioana.ciornei@nxp.com>
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help