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