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

Tomi Valkeinen
2024-11-29T09:54:42Z

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