Re: [PATCH] Driver core: fix race in dev_driver_string

[Dmitry Torokhov]

On Friday 04 December 2009 02:07:11 pm Alan Stern wrote:
> On Fri, 4 Dec 2009, Oliver Neukum wrote:
> > OK, yes. It's a bad example. However this is tricky.
> >
> > This is a bug then:
> >
> > mutex_lock(...);
> >
> > if (instance->error) {
> > 	rv = instance->error;
> > 	instance->error  = 0;
> > 	dev_dbg(instance->dev,...);
> 
> Unless you can guarantee at this point that instance->dev isn't stale,
> it is indeed a bug.
> 
> > 	goto err_out;
> > }
> >
> > rv = -ENODEV;
> > if (instance->disconnected)
> > 	goto err_out;
> >
> > > One approach is to set instance->dev to NULL in disconnect().  That
> > > wouldn't do much good for your dev_dbg(), though.  A better solution is
> > > to refcount the instance->dev pointer: Take a reference to the device
> > > when setting instance->dev and drop it when clearing instance->dev (or
> > > when instance is freed).
> >
> > That would mean that I am forced to adopt refcounting just to print
> > something. This seems very inelegant.
> 
> What can I say?  When the something you want to print can be
> deallocated at any time, there isn't much choice.
> 
> Maybe reference counting is inelegant; it depends on your point of
> view.  Can you think of a more elegant way to make sure that a pointer
> isn't stale?

Yes, just say "no" to device_create() and friends. Embed device structure in
yours, be mindful of lifetime rules and only use "your" device (i.e device
bound to your driver). This way, as long as your refcount your instance you
can rest assured the device structure is there as well.

-- 
Dmitry
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/