linux-kernel - Re: [PATCH v2] usb/hcd: Send a uevent signaling that the host controller has died

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: <Pine.LNX.4.44L0.1904171419260.1400-100000@iolanthe.rowland.org>
Date:   Wed, 17 Apr 2019 14:24:06 -0400 (EDT)
From:   Alan Stern <stern@...land.harvard.edu>
To:     Raul Rangel <rrangel@...omium.org>
cc:     Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        <linux-usb@...r.kernel.org>, <groeck@...omium.org>,
        <oneukum@...e.com>, <djkurtz@...omium.org>, <zwisler@...omium.org>,
        Sebastian Andrzej Siewior <bigeasy@...utronix.de>,
        Martin Blumenstingl <martin.blumenstingl@...glemail.com>,
        Dmitry Torokhov <dtor@...omium.org>,
        Suwan Kim <suwan.kim027@...il.com>,
        <linux-kernel@...r.kernel.org>,
        "Gustavo A. R. Silva" <gustavo@...eddedor.com>,
        Miquel Raynal <miquel.raynal@...tlin.com>,
        Johan Hovold <johan@...nel.org>,
        Mathias Nyman <mathias.nyman@...ux.intel.com>
Subject: Re: [PATCH v2] usb/hcd: Send a uevent signaling that the host
 controller has died

On Wed, 17 Apr 2019, Raul Rangel wrote:

> > > Also where would be a good place to document this?
> > 
> > Documentation/ABI/ is a good start.
> I'll add something to Documentation/ABI/testing/xhci-uevent

Your patch will apply to all host controllers, not just xhci.  The
documentation filename should reflect this.  Perhaps "usb-uevent"?

> > Why do you need to lock something that is "dead"?  And why is the idr
> > lock the correct one here?
> We need to ensure that root_hub is not null. Though I'm not sure the
> lock is entirely necessary in this case. usb_remove_hcd stops the work
> item before it sets the rhdev to null. The reason I picked
> usb_bus_idr_lock was because it's the same lock that usb_remove_hcd uses
> when setting rhdev = NULL.
> 
> Alan, what do you think? Should I remove the lock?

You're both right; the lock isn't needed because the work is stopped
before the root hub gets removed.  Acquiring the lock doesn't do any
harm, but it isn't needed so you probably should remove it.  In fact, 
you don't even need to test for whether hcd->self.root_hub is 
non-NULL.

Alan Stern