mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH] i2c: atr: fix race condition caused by static local variable in i2c_atr_map_msgs()
@ 2026-10-29  7:08 zhangjiao2
  2026-10-29  8:25 ` Luca Ceresoli
  0 siblings, 1 reply; 2+ messages in thread
From: zhangjiao2 @ 2026-10-29  7:08 UTC (permalink / raw)
  To: andi.shyti, dumitru.ceclan, sakari.ailus, luca.ceresoli,
	andriy.shevchenko, gonglinkai, rosenp, stable, linux-kernel,
	trivial
  Cc: zhang jiao

From: zhang jiao <zhangjiao2@cmss.chinamobile.com>

The variable `c2a` in i2c_atr_map_msgs() is declared `static`, which means
it is shared across all invocations of this function regardless of which
ATR channel is being serviced. Since each channel has its own
orig_addrs_lock and alias_pairs_lock mutexes, two threads can execute
i2c_atr_map_msgs() concurrently on different channels, racing on the
shared `c2a` pointer.

This can cause:
- The wrong alias to be used for address translation, sending I2C
  transactions to the wrong device.
- The `fixed` flag of an unrelated mapping to be corrupted, breaking the
  alias replacement logic.

Remove the `static` keyword so that `c2a` becomes a regular local
variable, eliminating the shared state between concurrent calls.

Signed-off-by: zhang jiao <zhangjiao2@cmss.chinamobile.com>
---
 drivers/i2c/i2c-atr.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/i2c/i2c-atr.c b/drivers/i2c/i2c-atr.c
index e6d2af659d81..c9ad46db438d 100644
--- a/drivers/i2c/i2c-atr.c
+++ b/drivers/i2c/i2c-atr.c
@@ -360,7 +360,7 @@ static int i2c_atr_map_msgs(struct i2c_atr_chan *chan, struct i2c_msg *msgs,
 			    int num)
 {
 	struct i2c_atr *atr = chan->atr;
-	static struct i2c_atr_alias_pair *c2a;
+	struct i2c_atr_alias_pair *c2a;
 	int i, ret = 0;
 
 	/* Ensure we have enough room to save the original addresses */
-- 
2.43.0



^ permalink raw reply	[flat|nested] 2+ messages in thread

* Re: [PATCH] i2c: atr: fix race condition caused by static local variable in i2c_atr_map_msgs()
  2026-10-29  7:08 [PATCH] i2c: atr: fix race condition caused by static local variable in i2c_atr_map_msgs() zhangjiao2
@ 2026-10-29  8:25 ` Luca Ceresoli
  0 siblings, 0 replies; 2+ messages in thread
From: Luca Ceresoli @ 2026-10-29  8:25 UTC (permalink / raw)
  To: zhangjiao2, andi.shyti, dumitru.ceclan, sakari.ailus,
	luca.ceresoli, andriy.shevchenko, gonglinkai, rosenp, stable,
	linux-kernel, trivial
  Cc: romain.gantois, Cosmin Tanislav, Tomi Valkeinen

Hello,

+Cc Romain, Cosmin, Tomi

On Thu Oct 29, 2026 at 8:08 AM CET, zhangjiao2 wrote:
> From: zhang jiao <zhangjiao2@cmss.chinamobile.com>
>
> The variable `c2a` in i2c_atr_map_msgs() is declared `static`, which means
> it is shared across all invocations of this function regardless of which
> ATR channel is being serviced. Since each channel has its own
> orig_addrs_lock and alias_pairs_lock mutexes, two threads can execute
> i2c_atr_map_msgs() concurrently on different channels, racing on the
> shared `c2a` pointer.
>
> This can cause:
> - The wrong alias to be used for address translation, sending I2C
>   transactions to the wrong device.
> - The `fixed` flag of an unrelated mapping to be corrupted, breaking the
>   alias replacement logic.
>
> Remove the `static` keyword so that `c2a` becomes a regular local
> variable, eliminating the shared state between concurrent calls.
>
> Signed-off-by: zhang jiao <zhangjiao2@cmss.chinamobile.com>

Was this potential bug observed on real hardware? I'm a bit surprised
nobody noticed before.

Also, if it's a bugfix, it should have a Fixes: tag and possibly Cc:
stable.

Code-wise it looks OK. Not sure a bug can really happen but I don't see any
reason this variable should be static: it is always set before being read
by this function.

Also please keep Romain, Cosmin and Tomi in Cc in future iterations, they
worked significantly on this code.

Luca

--
Luca Ceresoli, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2026-10-29  8:25 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-10-29  7:08 [PATCH] i2c: atr: fix race condition caused by static local variable in i2c_atr_map_msgs() zhangjiao2
2026-10-29  8:25 ` Luca Ceresoli

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®