From 147b36d5b70c083cc76770c47d60b347e8eaf231 Mon Sep 17 00:00:00 2001 From: Vladimir Zapolskiy Date: Mon, 31 Oct 2016 21:46:24 +0200 Subject: i2c: core: fix NULL pointer dereference under race condition 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 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: [] (i2c_do_del_adapter) from [] (__process_removed_adapter+0x14/0x1c) [] (__process_removed_adapter) from [] (bus_for_each_drv+0x6c/0xa0) [] (bus_for_each_drv) from [] (i2c_del_adapter+0xbc/0x284) [] (i2c_del_adapter) from [] (i2c_imx_remove+0x44/0x164 [i2c_imx]) [] (i2c_imx_remove [i2c_imx]) from [] (platform_drv_remove+0x2c/0x44) [] (platform_drv_remove) from [] (__device_release_driver+0x90/0x12c) [] (__device_release_driver) from [] (device_release_driver+0x28/0x34) [] (device_release_driver) from [] (unbind_store+0x80/0x104) [] (unbind_store) from [] (drv_attr_store+0x28/0x34) [] (drv_attr_store) from [] (sysfs_kf_write+0x50/0x54) [] (sysfs_kf_write) from [] (kernfs_fop_write+0x100/0x214) [] (kernfs_fop_write) from [] (__vfs_write+0x34/0x120) [] (__vfs_write) from [] (vfs_write+0xa8/0x170) [] (vfs_write) from [] (SyS_write+0x4c/0xa8) [] (SyS_write) from [] (ret_fast_syscall+0x0/0x1c) Signed-off-by: Vladimir Zapolskiy Signed-off-by: Wolfram Sang Cc: stable@kernel.org --- drivers/i2c/i2c-core.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'drivers/i2c') diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c index 1704fc84d647..b432b64e307a 100644 --- a/drivers/i2c/i2c-core.c +++ b/drivers/i2c/i2c-core.c @@ -2179,6 +2179,7 @@ int i2c_register_driver(struct module *owner, struct i2c_driver *driver) /* add the driver to the list of i2c drivers in the driver core */ driver->driver.owner = owner; driver->driver.bus = &i2c_bus_type; + INIT_LIST_HEAD(&driver->clients); /* When registration returns, the driver core * will have called probe() for all matching-but-unbound devices. @@ -2189,7 +2190,6 @@ int i2c_register_driver(struct module *owner, struct i2c_driver *driver) pr_debug("driver [%s] registered\n", driver->driver.name); - INIT_LIST_HEAD(&driver->clients); /* Walk the adapters that are already present */ i2c_for_each_dev(driver, __process_new_driver); -- cgit v1.2.3 From d052db11c153cfb469f13a4121966f30ecb57c66 Mon Sep 17 00:00:00 2001 From: Wolfram Sang Date: Sun, 6 Nov 2016 21:20:32 +0100 Subject: i2c: mux: demux-pinctrl: make drivers with no pinctrl work again Some drivers like i2c-gpio do not have dedicated pinctrl states. They broke when error checking for pinctrl was added. Detect them now, and in their case, simply skip over pinctrl configuration. Signed-off-by: Wolfram Sang Signed-off-by: Wolfram Sang --- drivers/i2c/muxes/i2c-demux-pinctrl.c | 22 ++++++++++++++++++++-- 1 file changed, 20 insertions(+), 2 deletions(-) (limited to 'drivers/i2c') diff --git a/drivers/i2c/muxes/i2c-demux-pinctrl.c b/drivers/i2c/muxes/i2c-demux-pinctrl.c index b3893f6282ba..3e6fe1760d82 100644 --- a/drivers/i2c/muxes/i2c-demux-pinctrl.c +++ b/drivers/i2c/muxes/i2c-demux-pinctrl.c @@ -69,10 +69,28 @@ static int i2c_demux_activate_master(struct i2c_demux_pinctrl_priv *priv, u32 ne goto err_with_revert; } - p = devm_pinctrl_get_select(adap->dev.parent, priv->bus_name); + /* + * Check if there are pinctrl states at all. Note: we cant' use + * devm_pinctrl_get_select() because we need to distinguish between + * the -ENODEV from devm_pinctrl_get() and pinctrl_lookup_state(). + */ + p = devm_pinctrl_get(adap->dev.parent); if (IS_ERR(p)) { ret = PTR_ERR(p); - goto err_with_put; + /* continue if just no pinctrl states (e.g. i2c-gpio), otherwise exit */ + if (ret != -ENODEV) + goto err_with_put; + } else { + /* there are states. check and use them */ + struct pinctrl_state *s = pinctrl_lookup_state(p, priv->bus_name); + + if (IS_ERR(s)) { + ret = PTR_ERR(s); + goto err_with_put; + } + ret = pinctrl_select_state(p, s); + if (ret < 0) + goto err_with_put; } priv->chan[new_chan].parent_adap = adap; -- cgit v1.2.3 From 93d710a65ef02fb7fd48ae207e78f460bd7a6089 Mon Sep 17 00:00:00 2001 From: Linus Walleij Date: Mon, 14 Nov 2016 15:34:17 +0100 Subject: i2c: mux: fix up dependencies We get the following build error from UM Linux after adding an entry to drivers/iio/gyro/Kconfig that issues "select I2C_MUX": ERROR: "devm_ioremap_resource" [drivers/i2c/muxes/i2c-mux-reg.ko] undefined! ERROR: "of_address_to_resource" [drivers/i2c/muxes/i2c-mux-reg.ko] undefined! It appears that the I2C mux core code depends on HAS_IOMEM for historical reasons, while CONFIG_I2C_MUX_REG does *not* have a direct dependency on HAS_IOMEM. This creates a situation where a allyesconfig or allmodconfig for UM Linux will select I2C_MUX, and will implicitly enable I2C_MUX_REG as well, and the compilation will fail for the register driver. Fix this up by making I2C_MUX_REG depend on HAS_IOMEM and removing the dependency from I2C_MUX. Reported-by: kbuild test robot Reported-by: Jonathan Cameron Signed-off-by: Linus Walleij Acked-by: Jonathan Cameron Acked-by: Peter Rosin Signed-off-by: Wolfram Sang Cc: stable@kernel.org --- drivers/i2c/Kconfig | 1 - drivers/i2c/muxes/Kconfig | 1 + 2 files changed, 1 insertion(+), 1 deletion(-) (limited to 'drivers/i2c') diff --git a/drivers/i2c/Kconfig b/drivers/i2c/Kconfig index d223650a97e4..11edabf425ae 100644 --- a/drivers/i2c/Kconfig +++ b/drivers/i2c/Kconfig @@ -59,7 +59,6 @@ config I2C_CHARDEV config I2C_MUX tristate "I2C bus multiplexing support" - depends on HAS_IOMEM help Say Y here if you want the I2C core to support the ability to handle multiplexed I2C bus topologies, by presenting each diff --git a/drivers/i2c/muxes/Kconfig b/drivers/i2c/muxes/Kconfig index e280c8ecc0b5..96de9ce5669b 100644 --- a/drivers/i2c/muxes/Kconfig +++ b/drivers/i2c/muxes/Kconfig @@ -63,6 +63,7 @@ config I2C_MUX_PINCTRL config I2C_MUX_REG tristate "Register-based I2C multiplexer" + depends on HAS_IOMEM help If you say yes to this option, support will be included for a register based I2C multiplexer. This driver provides access to -- cgit v1.2.3 From 68d85d0e03eab60c238ebe673c7cea1cf70275d4 Mon Sep 17 00:00:00 2001 From: Wei Yongjun Date: Sat, 29 Oct 2016 16:31:17 +0000 Subject: i2c: digicolor: use clk_disable_unprepare instead of clk_unprepare since clk_prepare_enable() is used to get i2c->clk, we should use clk_disable_unprepare() to release it for the error path. Signed-off-by: Wei Yongjun Acked-by: Baruch Siach Signed-off-by: Wolfram Sang --- drivers/i2c/busses/i2c-digicolor.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'drivers/i2c') diff --git a/drivers/i2c/busses/i2c-digicolor.c b/drivers/i2c/busses/i2c-digicolor.c index 49f2084f7bb5..50813a24c541 100644 --- a/drivers/i2c/busses/i2c-digicolor.c +++ b/drivers/i2c/busses/i2c-digicolor.c @@ -347,7 +347,7 @@ static int dc_i2c_probe(struct platform_device *pdev) ret = i2c_add_adapter(&i2c->adap); if (ret < 0) { - clk_unprepare(i2c->clk); + clk_disable_unprepare(i2c->clk); return ret; } -- cgit v1.2.3 From ad092de60f865c1ad94221bd06d381ecea446cc8 Mon Sep 17 00:00:00 2001 From: Alex Hemme Date: Sat, 19 Nov 2016 10:48:38 +0100 Subject: i2c: i2c-mux-pca954x: fix deselect enabling for device-tree Deselect functionality can be ignored for device-trees with "i2c-mux-idle-disconnect" entries if no platform_data is available. By enabling the deselect functionality outside the platform_data block the logic works as it did in previous kernels. Fixes: 7fcac9807175 ("i2c: i2c-mux-pca954x: convert to use an explicit i2c mux core") Cc: # v4.7+ Signed-off-by: Alex Hemme Signed-off-by: Ziyang Wu [touched up a few minor issues /peda] Signed-off-by: Peter Rosin Signed-off-by: Wolfram Sang --- drivers/i2c/muxes/i2c-mux-pca954x.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'drivers/i2c') diff --git a/drivers/i2c/muxes/i2c-mux-pca954x.c b/drivers/i2c/muxes/i2c-mux-pca954x.c index 1091346f2480..8bc3d36d2837 100644 --- a/drivers/i2c/muxes/i2c-mux-pca954x.c +++ b/drivers/i2c/muxes/i2c-mux-pca954x.c @@ -268,9 +268,9 @@ static int pca954x_probe(struct i2c_client *client, /* discard unconfigured channels */ break; idle_disconnect_pd = pdata->modes[num].deselect_on_exit; - data->deselect |= (idle_disconnect_pd - || idle_disconnect_dt) << num; } + data->deselect |= (idle_disconnect_pd || + idle_disconnect_dt) << num; ret = i2c_mux_add_adapter(muxc, force, num, class); -- cgit v1.2.3