linux-kernel - Re: [PATCH] Driver core: fix race in dev_driver

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <200912041423.10632.dmitry.torokhov@gmail.com>
Date:	Fri, 4 Dec 2009 14:23:10 -0800
From:	Dmitry Torokhov <dmitry.torokhov@...il.com>
To:	Alan Stern <stern@...land.harvard.edu>
Cc:	Oliver Neukum <oliver@...kum.org>, Greg KH <gregkh@...e.de>,
	stable@...nel.org, Rickard Bellini <rickard.bellini@...csson.com>,
	"linux-usb@...r.kernel.org" <linux-usb@...r.kernel.org>,
	Torgny Johansson <torgny.johansson@...csson.com>,
	Kernel development list <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] Driver core: fix race in dev_driver_string

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@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/