RE: [PATCH v4 1/2] i2c: imx: Fix slave registration race and error handling
From: Carlos Song <hidden>
Date: 2026-06-29 09:30:17
Also in:
imx, linux-i2c, lkml, stable
-----Original Message-----
From: Liem <redacted>
Sent: Monday, June 29, 2026 10:38 AM
To: Carlos Song (OSS) <redacted>
Cc: andi.shyti@kernel.org; Biwen Li <redacted>; festevam@gmail.com;
Frank Li [off-list ref]; Frank Li (OSS) [off-list ref];
imx@lists.linux.dev; kernel@pengutronix.de; liem16213@gmail.com;
linux-arm-kernel@lists.infradead.org; linux-i2c@vger.kernel.org;
linux-kernel@vger.kernel.org; o.rempel@pengutronix.de;
s.hauer@pengutronix.de; stable@vger.kernel.org; wsa@kernel.org
Subject: [EXT] [PATCH v4 1/2] i2c: imx: Fix slave registration race and error
handling
Caution: This is an external email. Please take care when clicking links or opening
attachments. When in doubt, report the message using the 'Report this email'
button
In i2c_imx_reg_slave(), the slave pointer was assigned before
pm_runtime_resume_and_get(). If pm_runtime_resume_and_get() failed, the
error path returned without clearing i2c_imx->slave, leaving it non-NULL and
causing all subsequent registration attempts to fail with -EBUSY.
Additionally, because this driver uses a shared IRQ, the interrupt handler
i2c_imx_isr() can execute concurrently and, after acquiring slave_lock,
dereference i2c_imx->slave. The previous fix attempt added a lockless
i2c_imx->slave = NULL on the error path, but that could race with the ISR under
the lock and still cause a NULL pointer dereference.
Fix both issues by deferring the assignment of i2c_imx->slave and
i2c_imx->last_slave_event to after a successful resume, and by performing the
assignment inside the slave_lock critical section.
This guarantees that the slave pointer is never left stale on the error path and is
always valid when observed by the interrupt handler.
Fixes: f7414cd6923f ("i2c: imx: support slave mode for imx I2C driver")
Cc: stable@vger.kernel.org
Signed-off-by: Liem <redacted>Hi, Liem LGTM. Thank you very much. Acked-by: Carlos Song <redacted>
quoted hunk ↗ jump to hunk
--- v3 -> v4: - Instead of clearing the slave pointer on error, defer the assignment until after pm_runtime_resume_and_get() succeeds, and take slave_lock to avoid racing with the shared IRQ handler. Suggested by Sashiko and Carlos Song --- drivers/i2c/busses/i2c-imx.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-)diff --git a/drivers/i2c/busses/i2c-imx.c b/drivers/i2c/busses/i2c-imx.c index28313d0fad37..2398c406e913 100644--- a/drivers/i2c/busses/i2c-imx.c +++ b/drivers/i2c/busses/i2c-imx.c@@ -930,9 +930,6 @@ static int i2c_imx_reg_slave(struct i2c_client *client) if (i2c_imx->slave) return -EBUSY; - i2c_imx->slave = client; - i2c_imx->last_slave_event = I2C_SLAVE_STOP; - /* Resume */ ret = pm_runtime_resume_and_get(i2c_imx->adapter.dev.parent); if (ret < 0) {@@ -940,6 +937,11 @@ static int i2c_imx_reg_slave(struct i2c_client *client) return ret; } + scoped_guard(spinlock_irqsave, &i2c_imx->slave_lock) { + i2c_imx->slave = client; + i2c_imx->last_slave_event = I2C_SLAVE_STOP; + } + i2c_imx_slave_init(i2c_imx); return 0; --2.34.1