On Mon, Oct 31, 2016 at 09:46:24PM +0200, Vladimir Zapolskiy wrote: > Race condition between registering an I2C device driver and > deregistering an I2C adapter device which is assumed to manage that > I2C device may lead to a NULL pointer dereference due to the > uninitialized list head of driver clients. > > The root cause of the issue is that the I2C bus may know about the > registered device driver and thus it is matched by bus_for_each_drv(), > but the list of clients is not initialized and commonly it is NULL, > because I2C device drivers define struct i2c_driver as static and > clients field is expected to be initialized by I2C core: > > i2c_register_driver() i2c_del_adapter() > driver_register() ... > bus_add_driver() ... > ... bus_for_each_drv(..., __process_removed_adapter) > ... i2c_do_del_adapter() > ... list_for_each_entry_safe(..., &driver->clients, ...) > INIT_LIST_HEAD(&driver->clients); > > To solve the problem it is sufficient to do clients list head > initialization before calling driver_register(). > > The problem was found while using an I2C device driver with a sluggish That one must have been really sluggish :) > registration routine on a bus provided by a physically detachable I2C > master controller, but practically the oops may be reproduced under > the race between arbitraty I2C device driver registration and managing > I2C bus device removal e.g. by unbinding the latter over sysfs: > > % echo 21a4000.i2c > /sys/bus/platform/drivers/imx-i2c/unbind > Unable to handle kernel NULL pointer dereference at virtual address 00000000 > Internal error: Oops: 17 [#1] SMP ARM > CPU: 2 PID: 533 Comm: sh Not tainted 4.9.0-rc3+ #61 > Hardware name: Freescale i.MX6 Quad/DualLite (Device Tree) > task: e5ada400 task.stack: e4936000 > PC is at i2c_do_del_adapter+0x20/0xcc > LR is at __process_removed_adapter+0x14/0x1c > Flags: NzCv IRQs on FIQs on Mode SVC_32 ISA ARM Segment none > Control: 10c5387d Table: 35bd004a DAC: 00000051 > Process sh (pid: 533, stack limit = 0xe4936210) > Stack: (0xe4937d28 to 0xe4938000) > Backtrace: > [<c0667be0>] (i2c_do_del_adapter) from [<c0667cc0>] (__process_removed_adapter+0x14/0x1c) > [<c0667cac>] (__process_removed_adapter) from [<c0516998>] (bus_for_each_drv+0x6c/0xa0) > [<c051692c>] (bus_for_each_drv) from [<c06685ec>] (i2c_del_adapter+0xbc/0x284) > [<c0668530>] (i2c_del_adapter) from [<bf0110ec>] (i2c_imx_remove+0x44/0x164 [i2c_imx]) > [<bf0110a8>] (i2c_imx_remove [i2c_imx]) from [<c051a838>] (platform_drv_remove+0x2c/0x44) > [<c051a80c>] (platform_drv_remove) from [<c05183d8>] (__device_release_driver+0x90/0x12c) > [<c0518348>] (__device_release_driver) from [<c051849c>] (device_release_driver+0x28/0x34) > [<c0518474>] (device_release_driver) from [<c0517150>] (unbind_store+0x80/0x104) > [<c05170d0>] (unbind_store) from [<c0516520>] (drv_attr_store+0x28/0x34) > [<c05164f8>] (drv_attr_store) from [<c0298acc>] (sysfs_kf_write+0x50/0x54) > [<c0298a7c>] (sysfs_kf_write) from [<c029801c>] (kernfs_fop_write+0x100/0x214) > [<c0297f1c>] (kernfs_fop_write) from [<c0220130>] (__vfs_write+0x34/0x120) > [<c02200fc>] (__vfs_write) from [<c0221088>] (vfs_write+0xa8/0x170) > [<c0220fe0>] (vfs_write) from [<c0221e74>] (SyS_write+0x4c/0xa8) > [<c0221e28>] (SyS_write) from [<c0108a20>] (ret_fast_syscall+0x0/0x1c) > > Signed-off-by: Vladimir Zapolskiy <vladimir_zapolskiy@xxxxxxxxxx> Good catch! Applied to for-current, thanks!
Attachment:
signature.asc
Description: PGP signature
