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

[Ulf Hansson]

On Sat, 18 Apr 2026 at 13:23, Danilo Krummrich <dakr at kernel.org> wrote:
>
> On Fri Apr 10, 2026 at 12:40 PM CEST, Ulf Hansson wrote:
> > The common sync_state support isn't fine grained enough for some types of
> > suppliers, like power domains for example. Especially when a supplier
> > provides multiple independent power domains, each with their own set of
> > consumers. In these cases we need to wait for all consumers for all the
> > provided power domains before invoking the supplier's ->sync_state().
> >
> > To allow a more fine grained sync_state support to be implemented on per
> > supplier's driver basis, let's add a new optional callback. As soon as
> > there is an update worth to consider in regards to managing sync_state for
> > a supplier device, __device_links_queue_sync_state() invokes the callback.
> >
> > Tested-by: Geert Uytterhoeven <geert+renesas at glider.be>
> > Signed-off-by: Ulf Hansson <ulf.hansson at linaro.org>
> > ---
> >  drivers/base/core.c           | 7 ++++++-
> >  include/linux/device/driver.h | 7 +++++++
> >  2 files changed, 13 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/base/core.c b/drivers/base/core.c
> > index 09b98f02f559..4085a011d8ca 100644
> > --- a/drivers/base/core.c
> > +++ b/drivers/base/core.c
> > @@ -1106,7 +1106,9 @@ int device_links_check_suppliers(struct device *dev)
> >   * Queues a device for a sync_state() callback when the device links write lock
> >   * isn't held. This allows the sync_state() execution flow to use device links
> >   * APIs.  The caller must ensure this function is called with
> > - * device_links_write_lock() held.
> > + * device_links_write_lock() held.  Note, if the optional queue_sync_state()
> > + * callback has been assigned too, it gets called for every update to allowing a
>
> s/allowing/allow/
>
> > + * more fine grained support to be implemented on per supplier basis.
> >   *
> >   * This function does a get_device() to make sure the device is not freed while
> >   * on this list.
> > @@ -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.

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?

>
> This also made me aware that the above dev_has_sync_state() is probably broken,
> as it also performs the following check without the device lock being held.
>
>         dev->driver && dev->driver->sync_state
>
> I think nothing prevents dev->driver to become NULL concurrently; in this case
> READ_ONCE() should be sufficient though as it doesn't execute the callback.
>
> I will send a patch for this.

Okay, thanks!

>
> > +
> >       list_for_each_entry(link, &dev->links.consumers, s_node) {
> >               if (!device_link_test(link, DL_FLAG_MANAGED))
> >                       continue;
> > diff --git a/include/linux/device/driver.h b/include/linux/device/driver.h
> > index bbc67ec513ed..bc9ae1cbe03c 100644
> > --- a/include/linux/device/driver.h
> > +++ b/include/linux/device/driver.h
> > @@ -68,6 +68,12 @@ enum probe_type {
> >   *           be called at late_initcall_sync level. If the device has
> >   *           consumers that are never bound to a driver, this function
> >   *           will never get called until they do.
> > + * @queue_sync_state: Similar to the ->sync_state() callback, but called to
> > + *           allow syncing device state to software state in a more fine
> > + *           grained way. It is called when there is an updated state that
> > + *           may be worth to consider for any of the consumers linked to
> > + *           this device. If implemented, the ->sync_state() callback is
> > + *           required too.
>
> What happens if this is not the case? Maybe worth to check and warn about this
> in driver_register().

Good point!

I believe I should also add a check in dev_set_drv_queue_sync_state()
that is added in patch2.

>
> >   * @remove:  Called when the device is removed from the system to
> >   *           unbind a device from this driver.
> >   * @shutdown:        Called at shut-down time to quiesce the device.
> > @@ -110,6 +116,7 @@ struct device_driver {
> >
> >       int (*probe) (struct device *dev);
> >       void (*sync_state)(struct device *dev);
> > +     void (*queue_sync_state)(struct device *dev);
> >       int (*remove) (struct device *dev);
> >       void (*shutdown) (struct device *dev);
> >       int (*suspend) (struct device *dev, pm_message_t state);
> > --
> > 2.43.0
>

Thanks for reviewing!

Kind regards
Uffe