diff mbox series

[v3,8/9] i2c: Support dynamic address translation

Message ID 20241125-fpc202-v3-8-34e86bcb5b56@bootlin.com (mailing list archive)
State New
Headers show
Series misc: Support TI FPC202 dual-port controller | expand

Commit Message

Romain Gantois Nov. 25, 2024, 8:45 a.m. UTC
The i2c-atr module keeps a list of associations between I2C client aliases
and I2C addresses. This represents the address translation table which is
programmed into an ATR channel at any given time. This list is only updated
when a new client is bound to the channel.

However in some cases, an ATR channel can have more downstream clients than
available aliases. One example of this is an SFP module that is bound to an
FPC202 port. The FPC202 port can only access up to two logical I2C
addresses. However, the SFP module may expose up to three logical I2C
addresses: its EEPROM on 7-bit addresses 0x50 and 0x51, and a PHY
transceiver on address 0x56.

In cases like these, it is necessary to reconfigure the channel's
translation table on the fly, so that all three I2C addresses can be
accessed when needed.

Add an optional "dynamic_c2a" flag to the i2c-atr module which allows it to
provide on-the-fly address translation. This is achieved by modifying an
ATR channel's translation table whenever an I2C transaction with unmapped
clients is requested.

Add a mutex to protect alias_list. This prevents
i2c_atr_dynamic_attach/detach_addr from racing with the bus notifier
handler to modify alias_list.

Signed-off-by: Romain Gantois <romain.gantois@bootlin.com>
---
 drivers/i2c/i2c-atr.c         | 244 ++++++++++++++++++++++++++++++++----------
 drivers/media/i2c/ds90ub960.c |   2 +-
 include/linux/i2c-atr.h       |  13 ++-
 3 files changed, 202 insertions(+), 57 deletions(-)

Comments

Tomi Valkeinen Nov. 29, 2024, 9:54 a.m. UTC | #1
Hi Romain,

On 25/11/2024 10:45, Romain Gantois wrote:
> The i2c-atr module keeps a list of associations between I2C client aliases
> and I2C addresses. This represents the address translation table which is
> programmed into an ATR channel at any given time. This list is only updated
> when a new client is bound to the channel.
> 
> However in some cases, an ATR channel can have more downstream clients than
> available aliases. One example of this is an SFP module that is bound to an
> FPC202 port. The FPC202 port can only access up to two logical I2C
> addresses. However, the SFP module may expose up to three logical I2C
> addresses: its EEPROM on 7-bit addresses 0x50 and 0x51, and a PHY
> transceiver on address 0x56.
> 
> In cases like these, it is necessary to reconfigure the channel's
> translation table on the fly, so that all three I2C addresses can be
> accessed when needed.
> 
> Add an optional "dynamic_c2a" flag to the i2c-atr module which allows it to
> provide on-the-fly address translation. This is achieved by modifying an
> ATR channel's translation table whenever an I2C transaction with unmapped
> clients is requested.
> 
> Add a mutex to protect alias_list. This prevents
> i2c_atr_dynamic_attach/detach_addr from racing with the bus notifier
> handler to modify alias_list.
> 
> Signed-off-by: Romain Gantois <romain.gantois@bootlin.com>
> ---
>   drivers/i2c/i2c-atr.c         | 244 ++++++++++++++++++++++++++++++++----------
>   drivers/media/i2c/ds90ub960.c |   2 +-
>   include/linux/i2c-atr.h       |  13 ++-
>   3 files changed, 202 insertions(+), 57 deletions(-)

This fails with:

WARNING: CPU: 1 PID: 360 at lib/list_debug.c:35 
__list_add_valid_or_report+0xe4/0x100

as the i2c_atr_create_c2a() calls list_add(), but i2c_atr_attach_addr(), 
which is changed to use i2c_atr_create_c2a(), also calls list_add().

Also, if you add i2c_atr_create_c2a() which hides the allocation and 
list_add, I think it makes sense to add a i2c_atr_destroy_c2a() to 
revert that.

There's also a memory error "BUG: KASAN: slab-use-after-free in 
__lock_acquire+0xc4/0x375c" (see below) when unloading the ub960 or 
ub953 driver. I haven't looked at that yet.

  Tomi

   316.204193] BUG: KASAN: slab-use-after-free in __lock_acquire+0xc4/0x375c
[  316.211059] Read of size 4 at addr cc44d558 by task rmmod/1505
[  316.216979]
[  316.218475] CPU: 1 UID: 0 PID: 1505 Comm: rmmod Not tainted 6.12.0+ #2
[  316.225097] Hardware name: Generic DRA74X (Flattened Device Tree)
[  316.231231] Call trace:
[  316.231262]  unwind_backtrace from show_stack+0x18/0x1c
[  316.239105]  show_stack from dump_stack_lvl+0x6c/0x8c
[  316.244232]  dump_stack_lvl from print_report+0x130/0x538
[  316.249694]  print_report from kasan_report+0x98/0xd8
[  316.254821]  kasan_report from __lock_acquire+0xc4/0x375c
[  316.260284]  __lock_acquire from lock_acquire.part.0+0x128/0x340
[  316.266357]  lock_acquire.part.0 from _raw_spin_lock+0x4c/0x5c
[  316.272247]  _raw_spin_lock from i2c_atr_release_alias+0x20/0x98 
[i2c_atr]
[  316.279235]  i2c_atr_release_alias [i2c_atr] from 
i2c_atr_bus_notifier_call+0x150/0x46c [i2c_atr]
[  316.288238]  i2c_atr_bus_notifier_call [i2c_atr] from 
notifier_call_chain+0x68/0x23c
[  316.296081]  notifier_call_chain from 
blocking_notifier_call_chain+0x5c/0x74
[  316.303192]  blocking_notifier_call_chain from bus_notify+0x3c/0x48
[  316.309539]  bus_notify from device_del+0xf0/0x514
[  316.314392]  device_del from device_unregister+0x28/0x80
[  316.319763]  device_unregister from __unregister_client+0x84/0x90
[  316.325927]  __unregister_client from device_for_each_child+0xc0/0x12c
[  316.332550]  device_for_each_child from i2c_del_adapter+0x28c/0x45c
[  316.338897]  i2c_del_adapter from i2c_atr_del_adapter+0x10c/0x250 
[i2c_atr]
[  316.345947]  i2c_atr_del_adapter [i2c_atr] from 
ub953_remove+0x48/0xcc [ds90ub953]
[  316.353637]  ub953_remove [ds90ub953] from i2c_device_remove+0x54/0xf8
[  316.360260]  i2c_device_remove from 
device_release_driver_internal+0x218/0x2a8
[  316.367553]  device_release_driver_internal from 
bus_remove_device+0x130/0x1cc
[  316.374847]  bus_remove_device from device_del+0x1f8/0x514
[  316.380401]  device_del from device_unregister+0x28/0x80
[  316.385772]  device_unregister from ub960_remove+0xb0/0x244 [ds90ub960]
[  316.392517]  ub960_remove [ds90ub960] from i2c_device_remove+0x54/0xf8
[  316.399139]  i2c_device_remove from 
device_release_driver_internal+0x218/0x2a8
[  316.406433]  device_release_driver_internal from driver_detach+0x6c/0xc4
[  316.413238]  driver_detach from bus_remove_driver+0xa0/0x134
[  316.418945]  bus_remove_driver from i2c_del_driver+0x48/0x84
[  316.424682]  i2c_del_driver from sys_delete_module+0x280/0x3cc
[  316.430572]  sys_delete_module from ret_fast_syscall+0x0/0x1c
Romain Gantois Dec. 3, 2024, 8:59 a.m. UTC | #2
Hi,

On vendredi 29 novembre 2024 10:54:35 heure normale d’Europe centrale Tomi Valkeinen wrote:
> Hi Romain,
> 
...
> > ATR channel's translation table whenever an I2C transaction with unmapped
> > clients is requested.
> > 
> > Add a mutex to protect alias_list. This prevents
> > i2c_atr_dynamic_attach/detach_addr from racing with the bus notifier
> > handler to modify alias_list.
> > 
> > Signed-off-by: Romain Gantois <romain.gantois@bootlin.com>
> > ---
> > 
> >   drivers/i2c/i2c-atr.c         | 244
> >   ++++++++++++++++++++++++++++++++----------
> >   drivers/media/i2c/ds90ub960.c |   2 +-
> >   include/linux/i2c-atr.h       |  13 ++-
> >   3 files changed, 202 insertions(+), 57 deletions(-)
> 
> This fails with:
> 
> WARNING: CPU: 1 PID: 360 at lib/list_debug.c:35
> __list_add_valid_or_report+0xe4/0x100
> 
> as the i2c_atr_create_c2a() calls list_add(), but i2c_atr_attach_addr(),
> which is changed to use i2c_atr_create_c2a(), also calls list_add().
> 
> Also, if you add i2c_atr_create_c2a() which hides the allocation and
> list_add, I think it makes sense to add a i2c_atr_destroy_c2a() to
> revert that.
> 

Sure, I just thought that it was safer to have an explicit "kfree" in the
code, as it would be clear that the c2a pointer shouldn't be used after this.
But setting the pointer to NULL after calling i2c_atr_destroy_c2a() would
essentially achieve the same thing, so I'll be going with your suggestion.

> There's also a memory error "BUG: KASAN: slab-use-after-free in
> __lock_acquire+0xc4/0x375c" (see below) when unloading the ub960 or
> ub953 driver. I haven't looked at that yet.
> 

I don't have the hardware to actually reproduce this but I'll see if I can
find out what the problem is by reading the code.

Thanks,
Romain Gantois Dec. 9, 2024, 12:42 p.m. UTC | #3
Hi Tomi,

On vendredi 29 novembre 2024 10:54:35 heure normale d’Europe centrale Tomi 
Valkeinen wrote:
> Hi Romain,
> 
> On 25/11/2024 10:45, Romain Gantois wrote:
> > The i2c-atr module keeps a list of associations between I2C client aliases
...
> > i2c_atr_dynamic_attach/detach_addr from racing with the bus notifier
> > handler to modify alias_list.
> > 
> > Signed-off-by: Romain Gantois <romain.gantois@bootlin.com>
> > ---
> > 
> >   drivers/i2c/i2c-atr.c         | 244
> >   ++++++++++++++++++++++++++++++++----------
> >   drivers/media/i2c/ds90ub960.c |   2 +-
> >   include/linux/i2c-atr.h       |  13 ++-
> >   3 files changed, 202 insertions(+), 57 deletions(-)
> 
> This fails with:
> 
> WARNING: CPU: 1 PID: 360 at lib/list_debug.c:35
> __list_add_valid_or_report+0xe4/0x100
> 
> as the i2c_atr_create_c2a() calls list_add(), but i2c_atr_attach_addr(),
> which is changed to use i2c_atr_create_c2a(), also calls list_add().
> 
> Also, if you add i2c_atr_create_c2a() which hides the allocation and
> list_add, I think it makes sense to add a i2c_atr_destroy_c2a() to
> revert that.
> 
> There's also a memory error "BUG: KASAN: slab-use-after-free in
> __lock_acquire+0xc4/0x375c" (see below) when unloading the ub960 or
> ub953 driver. I haven't looked at that yet.

I think I've found what's causing this KASAN splat.  i2c_atr_del_adapter is
freeing it's alias pool before setting atr->adapter[chan_id] to NULL. So
there's a time window during which bus notifications can trigger a call
to i2c_atr_attach_addr, leading to a UAF on the alias pool struct.

I'll fix this in v4.

Thanks,
Romain Gantois Dec. 10, 2024, 3:21 p.m. UTC | #4
Hi Tomi,

On lundi 9 décembre 2024 13:42:29 heure normale d’Europe centrale Romain 
Gantois wrote:
> Hi Tomi,
> 
...
> > This fails with:
> > 
> > WARNING: CPU: 1 PID: 360 at lib/list_debug.c:35
> > __list_add_valid_or_report+0xe4/0x100
> > 
> > as the i2c_atr_create_c2a() calls list_add(), but i2c_atr_attach_addr(),
> > which is changed to use i2c_atr_create_c2a(), also calls list_add().
> > 
> > Also, if you add i2c_atr_create_c2a() which hides the allocation and
> > list_add, I think it makes sense to add a i2c_atr_destroy_c2a() to
> > revert that.
> > 
> > There's also a memory error "BUG: KASAN: slab-use-after-free in
> > __lock_acquire+0xc4/0x375c" (see below) when unloading the ub960 or
> > ub953 driver. I haven't looked at that yet.
> 
> I think I've found what's causing this KASAN splat.  i2c_atr_del_adapter is
> freeing it's alias pool before setting atr->adapter[chan_id] to NULL. So
> there's a time window during which bus notifications can trigger a call
> to i2c_atr_attach_addr, leading to a UAF on the alias pool struct.

It seems like my initial theory was wrong. The bus notifier wouldn't trigger 
after the adapter has been removed.

However, the "alias_pool->shared" flag is not set anywhere in the i2c-atr 
module! So a more likely theory is that the common alias pool is being
freed when the first channel is deleted. Thus, the second channel is
still referencing a freed alias pool during it's deletion, hence the UAF.

Properly setting the "shared" flag of alias pools owned by the i2c_atr struct
should fix this.

Thanks,
diff mbox series

Patch

diff --git a/drivers/i2c/i2c-atr.c b/drivers/i2c/i2c-atr.c
index f7c4d39ad3b48ad64be25b8462394e569aee57d4..cc9ef19078b43ed57f876b0a132ecf979df54730 100644
--- a/drivers/i2c/i2c-atr.c
+++ b/drivers/i2c/i2c-atr.c
@@ -16,6 +16,7 @@ 
 #include <linux/mutex.h>
 #include <linux/slab.h>
 #include <linux/spinlock.h>
+#include <linux/lockdep.h>
 
 #define ATR_MAX_ADAPTERS 100	/* Just a sanity limit */
 #define ATR_MAX_SYMLINK_LEN 11	/* Longest name is 10 chars: "channel-99" */
@@ -27,9 +28,17 @@ 
  * @alias:  I2C alias address assigned by the driver.
  *          This is the address that will be used to issue I2C transactions
  *          on the parent (physical) bus.
+ * @fixed:  Alias pair cannot be replaced during dynamic address attachment.
+ *          This flag is necessary for situations where a single I2C transaction
+ *          contains more distinct target addresses than the ATR channel can handle.
+ *          It marks addresses that have already been attached to an alias so
+ *          that their alias pair is not evicted by a subsequent address in the same
+ *          transaction.
+ *
  */
 struct i2c_atr_alias_pair {
 	struct list_head node;
+	bool fixed;
 	u16 addr;
 	u16 alias;
 };
@@ -58,6 +67,7 @@  struct i2c_atr_alias_pool {
  * @adap:            The &struct i2c_adapter for the channel
  * @atr:             The parent I2C ATR
  * @chan_id:         The ID of this channel
+ * @alias_pairs_lock: Mutex protecting @alias_pairs
  * @alias_pairs:     List of @struct i2c_atr_alias_pair containing the
  *                   assigned aliases
  * @alias_pool:      Pool of available client aliases
@@ -71,6 +81,8 @@  struct i2c_atr_chan {
 	struct i2c_atr *atr;
 	u32 chan_id;
 
+	/* Lock alias_pairs during attach/detach */
+	struct mutex alias_pairs_lock;
 	struct list_head alias_pairs;
 	struct i2c_atr_alias_pool *alias_pool;
 
@@ -89,6 +101,7 @@  struct i2c_atr_chan {
  * @algo:      The &struct i2c_algorithm for adapters
  * @lock:      Lock for the I2C bus segment (see &struct i2c_lock_operations)
  * @max_adapters: Maximum number of adapters this I2C ATR can have
+ * @flags:     Optional features associated with this ATR
  * @alias_pool: Optional common pool of available client aliases
  * @i2c_nb:    Notifier for remote client add & del events
  * @adapter:   Array of adapters
@@ -104,6 +117,7 @@  struct i2c_atr {
 	/* lock for the I2C bus segment (see struct i2c_lock_operations) */
 	struct mutex lock;
 	int max_adapters;
+	unsigned short flags;
 
 	struct i2c_atr_alias_pool *alias_pool;
 
@@ -153,16 +167,128 @@  static void i2c_atr_free_alias_pool(struct i2c_atr_alias_pool *alias_pool)
 	kfree(alias_pool);
 }
 
+/* Must be called with alias_pairs_lock held */
+static struct i2c_atr_alias_pair *i2c_atr_create_c2a(struct i2c_atr_chan *chan,
+						     u16 alias, u16 addr)
+{
+	struct i2c_atr_alias_pair *c2a;
+
+	lockdep_assert_held(&chan->alias_pairs_lock);
+
+	c2a = kzalloc(sizeof(*c2a), GFP_KERNEL);
+	if (!c2a)
+		return NULL;
+
+	c2a->addr = addr;
+	c2a->alias = alias;
+
+	list_add(&c2a->node, &chan->alias_pairs);
+
+	return c2a;
+}
+
+static int i2c_atr_reserve_alias(struct i2c_atr_alias_pool *alias_pool)
+{
+	unsigned long idx;
+	u16 alias;
+
+	spin_lock(&alias_pool->lock);
+
+	idx = find_first_zero_bit(alias_pool->use_mask, alias_pool->size);
+	if (idx >= alias_pool->size) {
+		spin_unlock(&alias_pool->lock);
+		return -EBUSY;
+	}
+
+	set_bit(idx, alias_pool->use_mask);
+
+	alias = alias_pool->aliases[idx];
+
+	spin_unlock(&alias_pool->lock);
+	return alias;
+}
+
+static void i2c_atr_release_alias(struct i2c_atr_alias_pool *alias_pool, u16 alias)
+{
+	unsigned int idx;
+
+	spin_lock(&alias_pool->lock);
+
+	for (idx = 0; idx < alias_pool->size; ++idx) {
+		if (alias_pool->aliases[idx] == alias) {
+			clear_bit(idx, alias_pool->use_mask);
+			spin_unlock(&alias_pool->lock);
+			return;
+		}
+	}
+
+	spin_unlock(&alias_pool->lock);
+}
+
+/* Must be called with alias_pairs_lock held */
 static struct i2c_atr_alias_pair *
-i2c_atr_find_mapping_by_addr(const struct list_head *list, u16 phys_addr)
+i2c_atr_find_mapping_by_addr(struct i2c_atr_chan *chan, u16 addr)
 {
+	struct i2c_atr *atr = chan->atr;
 	struct i2c_atr_alias_pair *c2a;
+	struct list_head *alias_pairs;
+	u16 alias;
+	int ret;
+
+	lockdep_assert_held(&chan->alias_pairs_lock);
 
-	list_for_each_entry(c2a, list, node) {
-		if (c2a->addr == phys_addr)
+	alias_pairs = &chan->alias_pairs;
+
+	list_for_each_entry(c2a, alias_pairs, node) {
+		if (c2a->addr == addr)
 			return c2a;
 	}
 
+	if (!(atr->flags & I2C_ATR_FLAG_DYNAMIC_C2A))
+		return NULL;
+
+	ret = i2c_atr_reserve_alias(chan->alias_pool);
+	if (ret < 0) {
+		// If no free aliases are left, replace an existing one
+		if (unlikely(list_empty(alias_pairs)))
+			return NULL;
+
+		list_for_each_entry_reverse(c2a, alias_pairs, node)
+			if (!c2a->fixed)
+				break;
+
+		if (c2a->fixed)
+			return NULL;
+
+		atr->ops->detach_addr(atr, chan->chan_id, c2a->addr);
+		c2a->addr = addr;
+
+		// Move updated entry to beginning of list
+		list_move(&c2a->node, alias_pairs);
+
+		alias = c2a->alias;
+	} else {
+		alias = ret;
+
+		c2a = i2c_atr_create_c2a(chan, alias, addr);
+		if (!c2a)
+			goto err_release_alias;
+	}
+
+	ret = atr->ops->attach_addr(atr, chan->chan_id, c2a->addr, c2a->alias);
+	if (ret) {
+		dev_err(atr->dev, "failed to attach 0x%02x on channel %d: err %d\n",
+			addr, chan->chan_id, ret);
+		goto err_del_c2a;
+	}
+
+	return c2a;
+
+err_del_c2a:
+	list_del(&c2a->node);
+	kfree(c2a);
+err_release_alias:
+	i2c_atr_release_alias(chan->alias_pool, alias);
 	return NULL;
 }
 
@@ -178,7 +304,7 @@  static int i2c_atr_map_msgs(struct i2c_atr_chan *chan, struct i2c_msg *msgs,
 {
 	struct i2c_atr *atr = chan->atr;
 	static struct i2c_atr_alias_pair *c2a;
-	int i;
+	int i, ret = 0;
 
 	/* Ensure we have enough room to save the original addresses */
 	if (unlikely(chan->orig_addrs_size < num)) {
@@ -194,11 +320,13 @@  static int i2c_atr_map_msgs(struct i2c_atr_chan *chan, struct i2c_msg *msgs,
 		chan->orig_addrs_size = num;
 	}
 
+	mutex_lock(&chan->alias_pairs_lock);
+
 	for (i = 0; i < num; i++) {
 		chan->orig_addrs[i] = msgs[i].addr;
 
-		c2a = i2c_atr_find_mapping_by_addr(&chan->alias_pairs,
-						   msgs[i].addr);
+		c2a = i2c_atr_find_mapping_by_addr(chan, msgs[i].addr);
+
 		if (!c2a) {
 			dev_err(atr->dev, "client 0x%02x not mapped!\n",
 				msgs[i].addr);
@@ -206,13 +334,19 @@  static int i2c_atr_map_msgs(struct i2c_atr_chan *chan, struct i2c_msg *msgs,
 			while (i--)
 				msgs[i].addr = chan->orig_addrs[i];
 
-			return -ENXIO;
+			ret = -ENXIO;
+			goto out_unlock;
 		}
 
+		// Prevent c2a from being overwritten by another client in this transaction
+		c2a->fixed = true;
+
 		msgs[i].addr = c2a->alias;
 	}
 
-	return 0;
+out_unlock:
+	mutex_unlock(&chan->alias_pairs_lock);
+	return ret;
 }
 
 /*
@@ -225,10 +359,24 @@  static int i2c_atr_map_msgs(struct i2c_atr_chan *chan, struct i2c_msg *msgs,
 static void i2c_atr_unmap_msgs(struct i2c_atr_chan *chan, struct i2c_msg *msgs,
 			       int num)
 {
+	struct i2c_atr_alias_pair *c2a;
 	int i;
 
 	for (i = 0; i < num; i++)
 		msgs[i].addr = chan->orig_addrs[i];
+
+	mutex_lock(&chan->alias_pairs_lock);
+
+	if (unlikely(list_empty(&chan->alias_pairs)))
+		goto out_unlock;
+
+	// unfix c2a entries so that subsequent transfers can reuse their aliases
+	list_for_each_entry(c2a, &chan->alias_pairs, node) {
+		c2a->fixed = false;
+	}
+
+out_unlock:
+	mutex_unlock(&chan->alias_pairs_lock);
 }
 
 static int i2c_atr_master_xfer(struct i2c_adapter *adap, struct i2c_msg *msgs,
@@ -266,14 +414,23 @@  static int i2c_atr_smbus_xfer(struct i2c_adapter *adap, u16 addr,
 	struct i2c_atr *atr = chan->atr;
 	struct i2c_adapter *parent = atr->parent;
 	struct i2c_atr_alias_pair *c2a;
+	u16 alias;
+
+	mutex_lock(&chan->alias_pairs_lock);
+
+	c2a = i2c_atr_find_mapping_by_addr(chan, addr);
 
-	c2a = i2c_atr_find_mapping_by_addr(&chan->alias_pairs, addr);
 	if (!c2a) {
 		dev_err(atr->dev, "client 0x%02x not mapped!\n", addr);
+		mutex_unlock(&chan->alias_pairs_lock);
 		return -ENXIO;
 	}
 
-	return i2c_smbus_xfer(parent, c2a->alias, flags, read_write, command,
+	alias = c2a->alias;
+
+	mutex_unlock(&chan->alias_pairs_lock);
+
+	return i2c_smbus_xfer(parent, alias, flags, read_write, command,
 			      size, data);
 }
 
@@ -315,44 +472,6 @@  static const struct i2c_lock_operations i2c_atr_lock_ops = {
 	.unlock_bus =  i2c_atr_unlock_bus,
 };
 
-static int i2c_atr_reserve_alias(struct i2c_atr_alias_pool *alias_pool)
-{
-	unsigned long idx;
-	u16 alias;
-
-	spin_lock(&alias_pool->lock);
-
-	idx = find_first_zero_bit(alias_pool->use_mask, alias_pool->size);
-	if (idx >= alias_pool->size) {
-		spin_unlock(&alias_pool->lock);
-		return -EBUSY;
-	}
-
-	set_bit(idx, alias_pool->use_mask);
-
-	alias = alias_pool->aliases[idx];
-
-	spin_unlock(&alias_pool->lock);
-	return alias;
-}
-
-static void i2c_atr_release_alias(struct i2c_atr_alias_pool *alias_pool, u16 alias)
-{
-	unsigned int idx;
-
-	spin_lock(&alias_pool->lock);
-
-	for (idx = 0; idx < alias_pool->size; ++idx) {
-		if (alias_pool->aliases[idx] == alias) {
-			clear_bit(idx, alias_pool->use_mask);
-			spin_unlock(&alias_pool->lock);
-			return;
-		}
-	}
-
-	spin_unlock(&alias_pool->lock);
-}
-
 static int i2c_atr_attach_addr(struct i2c_adapter *adapter,
 			       u16 addr)
 {
@@ -370,7 +489,9 @@  static int i2c_atr_attach_addr(struct i2c_adapter *adapter,
 
 	alias = ret;
 
-	c2a = kzalloc(sizeof(*c2a), GFP_KERNEL);
+	mutex_lock(&chan->alias_pairs_lock);
+
+	c2a = i2c_atr_create_c2a(chan, alias, addr);
 	if (!c2a) {
 		ret = -ENOMEM;
 		goto err_release_alias;
@@ -378,7 +499,7 @@  static int i2c_atr_attach_addr(struct i2c_adapter *adapter,
 
 	ret = atr->ops->attach_addr(atr, chan->chan_id, addr, alias);
 	if (ret)
-		goto err_free;
+		goto err_del_c2a;
 
 	dev_dbg(atr->dev, "chan%u: addr 0x%02x mapped at alias 0x%02x\n",
 		chan->chan_id, addr, alias);
@@ -387,13 +508,16 @@  static int i2c_atr_attach_addr(struct i2c_adapter *adapter,
 	c2a->alias = alias;
 	list_add(&c2a->node, &chan->alias_pairs);
 
-	return 0;
+	goto out_unlock;
 
-err_free:
+err_del_c2a:
+	list_del(&c2a->node);
 	kfree(c2a);
+	c2a = NULL;
 err_release_alias:
 	i2c_atr_release_alias(chan->alias_pool, alias);
-
+out_unlock:
+	mutex_unlock(&chan->alias_pairs_lock);
 	return ret;
 }
 
@@ -406,13 +530,18 @@  static void i2c_atr_detach_addr(struct i2c_adapter *adapter,
 
 	atr->ops->detach_addr(atr, chan->chan_id, addr);
 
-	c2a = i2c_atr_find_mapping_by_addr(&chan->alias_pairs, addr);
+	mutex_lock(&chan->alias_pairs_lock);
+
+	c2a = i2c_atr_find_mapping_by_addr(chan, addr);
 	if (!c2a) {
 		 /* This should never happen */
 		dev_warn(atr->dev, "Unable to find address mapping\n");
+		mutex_unlock(&chan->alias_pairs_lock);
 		return;
 	}
 
+	mutex_unlock(&chan->alias_pairs_lock);
+
 	i2c_atr_release_alias(chan->alias_pool, c2a->alias);
 
 	dev_dbg(atr->dev,
@@ -538,7 +667,8 @@  static int i2c_atr_parse_alias_pool(struct i2c_atr *atr)
 }
 
 struct i2c_atr *i2c_atr_new(struct i2c_adapter *parent, struct device *dev,
-			    const struct i2c_atr_ops *ops, int max_adapters)
+			    const struct i2c_atr_ops *ops, int max_adapters,
+			    u16 flags)
 {
 	struct i2c_atr *atr;
 	int ret;
@@ -559,6 +689,7 @@  struct i2c_atr *i2c_atr_new(struct i2c_adapter *parent, struct device *dev,
 	atr->dev = dev;
 	atr->ops = ops;
 	atr->max_adapters = max_adapters;
+	atr->flags = flags;
 
 	if (parent->algo->master_xfer)
 		atr->algo.master_xfer = i2c_atr_master_xfer;
@@ -631,6 +762,7 @@  int i2c_atr_add_adapter(struct i2c_atr *atr, struct i2c_atr_adap_desc *desc)
 	chan->atr = atr;
 	chan->chan_id = chan_id;
 	INIT_LIST_HEAD(&chan->alias_pairs);
+	mutex_init(&chan->alias_pairs_lock);
 	mutex_init(&chan->orig_addrs_lock);
 
 	snprintf(chan->adap.name, sizeof(chan->adap.name), "i2c-%d-atr-%d",
@@ -707,6 +839,7 @@  int i2c_atr_add_adapter(struct i2c_atr *atr, struct i2c_atr_adap_desc *desc)
 err_fwnode_put:
 	fwnode_handle_put(dev_fwnode(&chan->adap.dev));
 	mutex_destroy(&chan->orig_addrs_lock);
+	mutex_destroy(&chan->alias_pairs_lock);
 	kfree(chan);
 	return ret;
 }
@@ -743,6 +876,7 @@  void i2c_atr_del_adapter(struct i2c_atr *atr, u32 chan_id)
 
 	fwnode_handle_put(fwnode);
 	mutex_destroy(&chan->orig_addrs_lock);
+	mutex_destroy(&chan->alias_pairs_lock);
 	kfree(chan->orig_addrs);
 	kfree(chan);
 }
diff --git a/drivers/media/i2c/ds90ub960.c b/drivers/media/i2c/ds90ub960.c
index 5a836731af6c701ef4070eb2dbab36cbdd86e0a2..06e7450d69877485360852ea9d811723b2ec8cb3 100644
--- a/drivers/media/i2c/ds90ub960.c
+++ b/drivers/media/i2c/ds90ub960.c
@@ -1099,7 +1099,7 @@  static int ub960_init_atr(struct ub960_data *priv)
 	struct i2c_adapter *parent_adap = priv->client->adapter;
 
 	priv->atr = i2c_atr_new(parent_adap, dev, &ub960_atr_ops,
-				priv->hw_data->num_rxports);
+				priv->hw_data->num_rxports, 0);
 	if (IS_ERR(priv->atr))
 		return PTR_ERR(priv->atr);
 
diff --git a/include/linux/i2c-atr.h b/include/linux/i2c-atr.h
index 1c3a5bcd939fc56f4a6ca1b6a5cc0ac2c17083b7..9287f064b683cc3cf83016b720c4e6ccd38df426 100644
--- a/include/linux/i2c-atr.h
+++ b/include/linux/i2c-atr.h
@@ -65,6 +65,7 @@  struct i2c_atr_adap_desc {
  * @dev:          The device acting as an ATR
  * @ops:          Driver-specific callbacks
  * @max_adapters: Maximum number of child adapters
+ * @flags:        Options to pass to the ATR framework. Allowed flags are I2C_ATR_FLAG_*
  *
  * The new ATR helper is connected to the parent adapter but has no child
  * adapters. Call i2c_atr_add_adapter() to add some.
@@ -74,7 +75,17 @@  struct i2c_atr_adap_desc {
  * Return: pointer to the new ATR helper object, or ERR_PTR
  */
 struct i2c_atr *i2c_atr_new(struct i2c_adapter *parent, struct device *dev,
-			    const struct i2c_atr_ops *ops, int max_adapters);
+			    const struct i2c_atr_ops *ops, int max_adapters,
+			    u16 flags);
+
+/*
+ * I2C ATR flags
+ *
+ * I2C_ATR_FLAG_DYNAMIC_C2A: Dynamically update translation table when transfers
+ *                           targeting unmapped addresses are requested.
+ *
+ */
+#define I2C_ATR_FLAG_DYNAMIC_C2A BIT(0)
 
 /**
  * i2c_atr_delete - Delete an I2C ATR helper.