linux-kernel - Re: [PATCH] USB: serial: fix sysfs-attribute removal deadlock

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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <1398392217.2805.150.camel@ThinkPad-T5421.cn.ibm.com>
Date:	Fri, 25 Apr 2014 10:16:57 +0800
From:	Li Zhong <zhong@...ux.vnet.ibm.com>
To:	Johan Hovold <jhovold@...il.com>
Cc:	Tejun Heo <tj@...nel.org>,
	Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
	Alan Stern <stern@...land.harvard.edu>,
	linux-usb@...r.kernel.org, linux-kernel@...r.kernel.org,
	rafael.j.wysocki@...el.com
Subject: Re: [PATCH] USB: serial: fix sysfs-attribute removal deadlock

On Thu, 2014-04-24 at 16:52 +0200, Johan Hovold wrote:
> On Thu, Apr 24, 2014 at 10:35:17AM -0400, Tejun Heo wrote:
> > On Thu, Apr 24, 2014 at 04:29:15PM +0800, Li Zhong wrote:
> > > On Wed, 2014-04-23 at 10:19 -0400, Tejun Heo wrote:
> > > > cc'ing Li Zhong who's working on a simliar issue in the following
> > > > thread and quoting whole body.
> > > > 
> > > >   http://thread.gmane.org/gmane.linux.kernel/1680706
> > > > 
> > > > Li, this is another variation of the same problem.  Maybe this can be
> > > > covered by your work too?
> > > 
> > > It seems to me that it is about write something to driver attribute, and
> > > driver unloading. If so, maybe it's not easy to reuse the help functions
> > > created for device attribute, and device removing.
> > > 
> > > But I guess the idea to break the active protection could still be
> > > applied here:
> > > 
> > > Maybe we could try_module_get() here (like the other option suggested by
> > > Johan?), and break active protection if we could get the module,
> > > something like below? 
> > 
> > I don't get why try_module_get() matters here.  We can't call into
> > ->store if the object at hand is already destroyed and the underlying
> > module can't go away if the target device is still alive.
> > try_module_get() doesn't actually protect the object.  Why does that
> > matter?  This is self removal, right?  Can you please take a look at
> > kernfs_remove_self()?
> 
> No, this isn't self removal. The driver-attribute (not device-attribute)
> store operation simply grabs a lock that is also held while the driver
> is being deregistered at module unload. Taking a reference to the module
> in this case will prevent deregistration while store is running.
> 
> But it seems like this can be solved for usb-serial by simply not
> holding the lock while deregistering.

I didn't look carefully about this lock. 

But I'm not sure whether there are such requirements for driver
attributes:

some lock needs be grabbed in the driver attributes store callbacks, and
the same lock also needs to be grabbed during driver unregister. 

If we have such requirements currently or in the future, I think they
could all be solved by breaking active protection after get the module
reference.

Thanks, Zhong

> 
> Johan
> 


--
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/