Re: [PATCH v2 1/9] driver core: Enable suppliers to implement fine grained sync_state support

["Danilo Krummrich" <dakr@kernel.org>]

On Wed Apr 22, 2026 at 12:07 PM CEST, Ulf Hansson wrote:
> On Sat, 18 Apr 2026 at 13:23, Danilo Krummrich <dakr@kernel.org> wrote:
>> On Fri Apr 10, 2026 at 12:40 PM CEST, Ulf Hansson wrote:
>> > @@ -1126,6 +1128,9 @@ static void __device_links_queue_sync_state(struct device *dev,
>> >       if (dev->state_synced)
>> >               return;
>> >
>> > +     if (dev->driver && dev->driver->queue_sync_state)
>> > +             dev->driver->queue_sync_state(dev);
>>
>> This seems to be called without the device lock being held, which seems to allow
>> the queue_sync_state() callback to execute concurrently with remove(). This
>> opens the door for all kinds of UAF conditions in drivers.
>
> If that were the case, this whole function would be unsafe even before
> this change. I assume this isn't because of how the function is being
> called, but I may be wrong.

This function does not issue any driver callbacks intentionally; the existing
sync_state() callback is deferred to device_links_flush_sync_list(), which is
called without the device_links_write_lock() held, but takes the device_lock()
to protect against other concurrent driver callbacks, such as remove().

I.e. we can't take the device_lock() when the device_links_write_lock() is held,
as it would be prone to lock inversion.

The documentation of __device_links_queue_sync_state() actually slightly hints
at this, but focuses more on the other reason for the deferred semantics -- the
sync_state() callback may want to call device link APIs.

> Anyway, let me add a get/put_device() here somewhere, to ensure we
> prevent this from happening. I assume that is what you are proposing?

No, an additional device reference count won't protect against other concurrent
driver callbacks, such as remove().