diff mbox series

[RFC] i2c: hold the core_lock for the whole execution of i2c_register_adapter()

Message ID 20181204124514.GD20015@sstepien
State New
Headers show
Series [RFC] i2c: hold the core_lock for the whole execution of i2c_register_adapter() | expand

Commit Message

Stepien, Slawomir (Nokia - PL/Wroclaw) Dec. 4, 2018, 12:45 p.m. UTC
There is a race condition between the i2c_get_adapter() and the
i2c_add_adapter() if this mutex isn't hold for the whole execution of
i2c_register_adapter().

If the mutex isn't locked, it is possible to find idr that points to
adapter that hasn't been registered yet (i.e. it's
kobj.state_initialized is still false), which will end up with warning
message:

"... is not initialized, yet kobject_get() is being called."

This patch will change how the locking is arranged around
i2c_register_adapter() call and will prevent such situations. The part
of the i2c_register_adapter() that do not need to be under the lock has
been moved to a new function i2c_process_adapter.

Signed-off-by: Sławomir Stępień <slawomir.stepien@nokia.com>
---
 drivers/i2c/i2c-core-base.c | 61 +++++++++++++++++++++++++------------
 1 file changed, 41 insertions(+), 20 deletions(-)
diff mbox series

Patch

diff --git a/drivers/i2c/i2c-core-base.c b/drivers/i2c/i2c-core-base.c
index 28460f6a60cc..15a51f5d905a 100644
--- a/drivers/i2c/i2c-core-base.c
+++ b/drivers/i2c/i2c-core-base.c
@@ -1264,6 +1264,21 @@  static int i2c_register_adapter(struct i2c_adapter *adap)
 
 	dev_dbg(&adap->dev, "adapter [%s] registered\n", adap->name);
 
+out_reg:
+	init_completion(&adap->dev_released);
+	device_unregister(&adap->dev);
+	wait_for_completion(&adap->dev_released);
+out_list:
+	idr_remove(&i2c_adapter_idr, adap->nr);
+	return res;
+}
+
+static void i2c_process_adapter(struct i2c_adapter *adap)
+{
+#ifdef CONFIG_I2C_COMPAT
+	int res;
+#endif
+
 	pm_runtime_no_callbacks(&adap->dev);
 	pm_suspend_ignore_children(&adap->dev, true);
 	pm_runtime_enable(&adap->dev);
@@ -1290,18 +1305,6 @@  static int i2c_register_adapter(struct i2c_adapter *adap)
 	mutex_lock(&core_lock);
 	bus_for_each_drv(&i2c_bus_type, NULL, adap, __process_new_adapter);
 	mutex_unlock(&core_lock);
-
-	return 0;
-
-out_reg:
-	init_completion(&adap->dev_released);
-	device_unregister(&adap->dev);
-	wait_for_completion(&adap->dev_released);
-out_list:
-	mutex_lock(&core_lock);
-	idr_remove(&i2c_adapter_idr, adap->nr);
-	mutex_unlock(&core_lock);
-	return res;
 }
 
 /**
@@ -1313,15 +1316,24 @@  static int i2c_register_adapter(struct i2c_adapter *adap)
  */
 static int __i2c_add_numbered_adapter(struct i2c_adapter *adap)
 {
-	int id;
+	int id, ret;
 
 	mutex_lock(&core_lock);
 	id = idr_alloc(&i2c_adapter_idr, adap, adap->nr, adap->nr + 1, GFP_KERNEL);
-	mutex_unlock(&core_lock);
-	if (WARN(id < 0, "couldn't get idr"))
+	if (WARN(id < 0, "couldn't get idr")) {
+		mutex_unlock(&core_lock);
 		return id == -ENOSPC ? -EBUSY : id;
+	}
+
+	ret = i2c_register_adapter(adap);
+	mutex_unlock(&core_lock);
+
+	if (ret < 0)
+		return ret;
 
-	return i2c_register_adapter(adap);
+	i2c_process_adapter(adap);
+
+	return 0;
 }
 
 /**
@@ -1341,7 +1353,7 @@  static int __i2c_add_numbered_adapter(struct i2c_adapter *adap)
 int i2c_add_adapter(struct i2c_adapter *adapter)
 {
 	struct device *dev = &adapter->dev;
-	int id;
+	int id, ret;
 
 	if (dev->of_node) {
 		id = of_alias_get_id(dev->of_node, "i2c");
@@ -1354,13 +1366,22 @@  int i2c_add_adapter(struct i2c_adapter *adapter)
 	mutex_lock(&core_lock);
 	id = idr_alloc(&i2c_adapter_idr, adapter,
 		       __i2c_first_dynamic_bus_num, 0, GFP_KERNEL);
-	mutex_unlock(&core_lock);
-	if (WARN(id < 0, "couldn't get idr"))
+	if (WARN(id < 0, "couldn't get idr")) {
+		mutex_unlock(&core_lock);
 		return id;
+	}
 
 	adapter->nr = id;
 
-	return i2c_register_adapter(adapter);
+	ret = i2c_register_adapter(adapter);
+	mutex_unlock(&core_lock);
+
+	if (ret < 0)
+		return ret;
+
+	i2c_process_adapter(adapter);
+
+	return 0;
 }
 EXPORT_SYMBOL(i2c_add_adapter);