i2c: guard against oopses from bad init sequences

From: David Brownell <[email protected]>

Guard I2C against oopsing because of init sequence problems, by verifying that i2c_init() has been called before calling any routines that rely on that initialization. This specific test just requires that bus_register(&i2c_bus_type) was called.

Examples of this kind of oopsing come from subystems and drivers which register I2C drivers in their subsys_initcall code but which are statically linked before I2C by drivers/Makefile.

Signed-off-by David Brownell <[email protected]>

---
Alternatively have postcore_initcall(i2c_init), which may be better ... the initcall levels are pretty limited, and in these cases the "subsystem" of interest builds on I2C and needs to work before device_initcall. Having I2C use subsys_initcall kind of forces things into fs_initcall.

I'd encourage the anti-oopsing paranoia in any case, even if i2c switches to postcore_initcall (or earlier).

 drivers/i2c/i2c-core.c |   10 ++++++++++
 1 file changed, 10 insertions(+)

drivers/i2c/i2c-core.c +10 -0

@@ -443,6 +443,12 @@ static int i2c_register_adapter(struct i
 
 	mutex_lock(&core_lock);
 
+	/* can't register until after driver model init */
+	if (WARN_ON(!i2c_bus_type.p)) {
+		res = -ENOENT;
+		goto out_list;
+	}
+
 	/* Add the adapter to the driver core.
 	 * If the parent pointer is not set up,
 	 * we add this adapter to the host bus.

@@ -696,6 +702,10 @@ int i2c_register_driver(struct module *o
 {
 	int res;
 
+	/* can't register until after driver model init */
+	if (WARN_ON(!i2c_bus_type.p))
+		return -ENOENT;
+
 	/* new style driver methods can't mix with legacy ones */
 	if (is_newstyle_driver(driver)) {
 		if (driver->attach_adapter || driver->detach_adapter

Hi David,

Quoted 10 lines · David Brownell

From: David Brownell <[email protected]>

Guard I2C against oopsing because of init sequence problems...

Alternatively have postcore_initcall(i2c_init), which may be better ... the initcall levels are pretty limited, and in these cases the "subsystem" of interest builds on I2C and needs to work before device_initcall. Having I2C use subsys_initcall kind of forces things into fs_initcall.

Quoted 2 lines

I'd encourage the anti-oopsing paranoia in any case, even if i2c switches to postcore_initcall (or earlier).

I'm confused. If i2c_register_adapter() is called before i2c_init(), your patch prevents an oops, but things will still not work, right?

So
this doesn't seem to add much value. In any case we need to fix the init sequence, so let's just do that.

From: David Brownell <[email protected]>

Guard I2C against oopsing because of init sequence problems, by verifying that i2c_init() has been called before calling any routines that rely on that initialization. This specific test just requires that bus_register(&i2c_bus_type) was called.

Examples of this kind of oopsing come from subystems and drivers which register I2C drivers in their subsys_initcall code but which are statically linked before I2C by drivers/Makefile.

Signed-off-by David Brownell <[email protected]>

---
Alternatively have postcore_initcall(i2c_init), which may be better ... the initcall levels are pretty limited, and in these cases the "subsystem" of interest builds on I2C and needs to work before device_initcall. Having I2C use subsys_initcall kind of forces things into fs_initcall.

I'd encourage the anti-oopsing paranoia in any case, even if i2c switches to postcore_initcall (or earlier).

 drivers/i2c/i2c-core.c |   10 ++++++++++
 1 file changed, 10 insertions(+)

drivers/i2c/i2c-core.c +0 -0

 	/* Add the adapter to the driver core.
 	 * If the parent pointer is not set up,
 	 * we add this adapter to the host bus.

Quoted 6 lines

drivers/i2c/i2c-core.c +0 -0

@@ -443,6 +443,12 @@ static int i2c_register_adapter(struct i
 
 	mutex_lock(&core_lock);
 

drivers/i2c/i2c-core.c static int i2c_register_adapter(struct i · line 451

Quoted 6 lines

drivers/i2c/i2c-core.c +6 -0

+	/* can't register until after driver model init */
+	if (WARN_ON(!i2c_bus_type.p)) {
+		res = -ENOENT;
+		goto out_list;
+	}
+

Why don't you test before acquiring core_lock? Or even, before doing anything else, as you do in i2c_register_driver. That's more consistent and makes the error path lighter.

So, I would apply the following patch if that's OK with you:

 drivers/i2c/i2c-core.c |    8 ++++++++
 1 file changed, 8 insertions(+)

drivers/i2c/i2c-core.c +4 -0

@@ -437,6 +437,10 @@ static int i2c_register_adapter(struct i
 {
 	int res = 0, dummy;
 
+	/* Can't register until after driver model init */
+	if (WARN_ON(!i2c_bus_type.p))
+		return -EAGAIN;
+
 	mutex_init(&adap->bus_lock);
 	mutex_init(&adap->clist_lock);
 	INIT_LIST_HEAD(&adap->clients);