From: "Danilo Krummrich" <dakr@kernel.org>
To: "Alan Stern" <stern@rowland.harvard.edu>
Cc: "Johan Hovold" <johan@kernel.org>,
"Greg Kroah-Hartman" <gregkh@linuxfoundation.org>,
"Rafael J. Wysocki" <rafael@kernel.org>,
"Dmitry Torokhov" <dmitry.torokhov@gmail.com>,
<driver-core@lists.linux.dev>, <linux-usb@vger.kernel.org>,
<linux-kernel@vger.kernel.org>,
"Mario Peter" <mario.peter@leica-geosystems.com>,
<stable@vger.kernel.org>
Subject: Re: [PATCH] driver core: take parent lock on deferred probe
Date: Tue, 13 Oct 2026 23:01:16 +0200 [thread overview]
Message-ID: <DM40HVOBKMFI.3W1DK371ZWI80@kernel.org> (raw)
In-Reply-To: <ba24846a-7403-4bb1-a0cb-869cf1d44e05@rowland.harvard.edu>
On Tue Oct 13, 2026 at 9:11 PM CEST, Alan Stern wrote:
> On Tue, Oct 13, 2026 at 07:43:08PM +0200, Danilo Krummrich wrote:
>> On Tue Oct 13, 2026 at 4:06 PM CEST, Johan Hovold wrote:
>> > +void bus_probe_device_deferred(struct device *dev)
>> > +{
>> > + if (dev->parent && dev->bus->need_parent_lock)
>> > + device_lock(dev->parent);
>>
>> This looks like it has the same potential UAF as the other patch. It's quite
>> unfortunate we need the whole need_parent_lock thing just for USB. IIUC the
>> main reason USB needs this is because interface devices share resources of USB
>> devices and we need to keep the locking order to avoid inversions, right? Is
>> there more to this from the USB side of things?
>
> That's pretty much it.
>
>> I think we can probably do something like this
>>
>> struct device *parent = NULL;
>>
>> scoped_guard(device, dev) {
>> if (dev->p->dead)
>> return;
>>
>> if (dev->bus->need_parent_lock)
>> parent = get_device(dev->parent);
>> }
>>
>> to avoid the problem, but it's still a bit ugly.
>>
>> > + __bus_probe_device(dev);
>> > + if (dev->parent && dev->bus->need_parent_lock)
>> > + device_unlock(dev->parent);
>> > +}
>
> Wouldn't it be simpler just to make each USB interface take a reference
> to its parent device? And likewise for USB devices, if needed?
The driver core already holds a reference count of the parent between
device_add() and device_del(), which is where a "parent" is technically defined.
But the deferred retry can extend beyond device_del().
USB interfaces already take an additional reference to their USB device in
usb_set_configuration() and drop it in usb_release_interface(), so they should
already be protected against this particular UAF.
The issue concerns a USB device and its parent hub's USB device. (This makes me
wonder whether probing a USB device actually requires the parent USB HUB
device's lock to be held, or whether this is just a consequence of sharing the
same usb_bus_type?)
In theory a USB device could also keep a reference count to its parent, but then
the driver core code would rely on an implementation detail of its users; I'd
prefer not to make driver core's access to dev->parent depend on references
maintained by individual subsystems.
I'd even go a step further any say that if subsystems try to keep this pointer
valid beyond what's guaranteed by the driver core, they should do this with a
separate bus specific pointer. The reason is that otherwise the lifetime and
ownership definition of this pointer does not depend on API contracts, but on
individual implementation details, which is a very common source for all kinds
of bugs.
prev parent reply other threads:[~2026-10-13 21:01 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-10-13 14:06 [PATCH] driver core: take parent lock on deferred probe Johan Hovold
2026-10-13 14:24 ` sashiko-bot
2026-10-13 17:43 ` Danilo Krummrich
2026-10-13 19:11 ` Alan Stern
2026-10-13 21:01 ` Danilo Krummrich [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=DM40HVOBKMFI.3W1DK371ZWI80@kernel.org \
--to=dakr@kernel.org \
--cc=dmitry.torokhov@gmail.com \
--cc=driver-core@lists.linux.dev \
--cc=gregkh@linuxfoundation.org \
--cc=johan@kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-usb@vger.kernel.org \
--cc=mario.peter@leica-geosystems.com \
--cc=rafael@kernel.org \
--cc=stable@vger.kernel.org \
--cc=stern@rowland.harvard.edu \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox