[i2c] [PATCH] i2c: Clean up core locking

David Brownell david-b at pacbell.net
Fri Dec 7 19:04:46 CET 2007


> > > @@ -628,6 +624,7 @@ void i2c_del_driver(struct i2c_driver *d
> > >  	 * attached. If so, detach them to be able to kill the driver
> > >  	 * afterwards.
> > >  	 */
> > > +	down(&i2c_adapter_class.sem);
> > >  	list_for_each_entry(adap, &i2c_adapter_class.devices, dev.node) {
> > >  		if (driver->detach_adapter) {
> > >  			if (driver->detach_adapter(adap)) {
> > > @@ -652,12 +649,11 @@ void i2c_del_driver(struct i2c_driver *d
> > >  			}
> > >  		}
> > >  	}
> > > +	up(&i2c_adapter_class.sem);
> > >  
> > >   unregister:
> > >  	driver_unregister(&driver->driver);
> > 
> > There's a race here for legacy drivers.  Consider that someone can
> > add a device between the up() and the driver_unregister(), and the
> > attach_adapter() loop may have bound that device to this driver.
> > 
> > 
> > >  	pr_debug("i2c-core: driver [%s] unregistered\n", driver->driver.name);
> > > -
> > > -	mutex_unlock(&core_lists);
> > >  }
>
> Hmm, that's right, good catch. Damn legacy drivers :( What a pain.
>
> I've been thinking about it for 2 hours now, and it turns out that I
> simply don't understand it all good enough to make safe changes in this
> area. It's too complex for me it seems

Wouldn't it suffice to just add a flag to the driver saying whether
an attach_adapter() call would be valid?  You can prevent the loop
fairly directly, rather than by code flow.  Solving this puzzler by
codeflow morphing is probably a losing game.

- Dave




More information about the i2c mailing list