All of lore.kernel.org
 help / color / mirror / Atom feed
From: Liem <liem16213@gmail.com>
To: carlos.song@oss.nxp.com
Cc: andi.shyti@kernel.org, biwen.li@nxp.com, festevam@gmail.com,
	frank.li@nxp.com, frank.li@oss.nxp.com, 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: [PATCH v4 1/2] i2c: imx: Fix slave registration race and error handling
Date: Mon, 29 Jun 2026 10:38:28 +0800	[thread overview]
Message-ID: <20260629023829.152651-2-liem16213@gmail.com> (raw)
In-Reply-To: <20260629023829.152651-1-liem16213@gmail.com>

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 <liem16213@gmail.com>
---
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
index 28313d0fad37..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


  reply	other threads:[~2026-06-29  2:38 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-06-25  7:11 [PATCH] i2c: imx: Fix slave registration error path and missing NULL check Liem
2026-06-25  7:26 ` sashiko-bot
2026-06-25 11:17 ` Carlos Song (OSS)
2026-06-25 16:02 ` [PATCH v2] i2c: imx: Fix slave registration error path and missing timer cleanup Liem
2026-06-25 16:15   ` sashiko-bot
2026-06-25 16:16   ` Frank Li
2026-06-26  1:55     ` liem
2026-06-26  2:58     ` [PATCH v3 0/2] Fix slave mode corner issues Liem
2026-06-26  2:58       ` [PATCH v3 1/2] i2c: imx: Clear slave pointer on registration error Liem
2026-06-26  6:23         ` Carlos Song (OSS)
2026-06-26  8:30           ` liem
2026-06-29  2:38           ` [PATCH v4 0/2] i2c: imx: Fix slave mode corner issues Liem
2026-06-29  2:38             ` Liem [this message]
2026-06-29  2:55               ` [PATCH v4 1/2] i2c: imx: Fix slave registration race and error handling sashiko-bot
2026-06-29  9:30               ` Carlos Song
2026-06-29 14:13               ` Frank Li
2026-06-30  6:49                 ` liem
2026-06-29  2:38             ` [PATCH v4 2/2] i2c: imx: Cancel hrtimer before clearing slave pointer Liem
2026-06-29  2:48               ` sashiko-bot
2026-06-29 14:14               ` Frank Li
2026-06-26  2:58       ` [PATCH v3 " Liem
2026-06-26  6:26         ` Carlos Song (OSS)

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260629023829.152651-2-liem16213@gmail.com \
    --to=liem16213@gmail.com \
    --cc=andi.shyti@kernel.org \
    --cc=biwen.li@nxp.com \
    --cc=carlos.song@oss.nxp.com \
    --cc=festevam@gmail.com \
    --cc=frank.li@nxp.com \
    --cc=frank.li@oss.nxp.com \
    --cc=imx@lists.linux.dev \
    --cc=kernel@pengutronix.de \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-i2c@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=o.rempel@pengutronix.de \
    --cc=s.hauer@pengutronix.de \
    --cc=stable@vger.kernel.org \
    --cc=wsa@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.