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@xxxxxxxxxxx> > > --- > > > > 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, Bootlin Embedded Linux and Kernel engineering https://bootlin.com
