Re: [PATCH] i2c: atr: fix race condition caused by static local variable in i2c_atr_map_msgs()
From: Tomi Valkeinen
Date: Thu Oct 29 2026 - 11:38:53 EST
On 29/10/2026 10:25, Luca Ceresoli wrote:
Hello,The code change looks valid to me.
+Cc Romain, Cosmin, Tomi
On Thu Oct 29, 2026 at 8:08 AM CET, zhangjiao2 wrote:
From: zhang jiao <zhangjiao2@xxxxxxxxxxxxxxxxxxxx>
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@xxxxxxxxxxxxxxxxxxxx>
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.
I only had a quick look, so take this with a grain of salt, but I think i2c_atr_lock_bus() is always taken when i2c_atr_map_msgs() is called. Thus the accesses to c2a are serialized in one atr. With multiple atrs this might trigger. But I think it's not out of the question that the compiler would optimize it to a local variable, based on how it is used (but I didn't actually dissassemble).
So, yes, it would be interesting to hear if this was observed on HW or just fixing code that looks wrong.
Tomi