[PATCH] driver core: use READ_ONCE() for dev->driver in dev_has_sync_state()

[PATCH] driver core: use READ_ONCE() for dev->driver in dev_has_sync_state()

[Danilo Krummrich]

dev_has_sync_state() reads dev->driver twice without holding
device_lock() -- once for the NULL check and once to dereference
->sync_state. Some callers only hold device_links_write_lock, which
doesn't prevent a concurrent unbind from clearing dev->driver via
device_unbind_cleanup().

Fix it by reading dev->driver exactly once with READ_ONCE(), pairing
with the WRITE_ONCE() in device_set_driver().

Link: https://lore.kernel.org/driver-core/DHW8QPU1VU1F.3P6PH69HLFBYC@kernel.org/
Fixes: ac338acf514e ("driver core: Add dev_has_sync_state()")
Signed-off-by: Danilo Krummrich <dakr@kernel.org>
---
 include/linux/device.h | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/include/linux/device.h b/include/linux/device.h
index ac972e7bead4..4c1c9cb8570a 100644
--- a/include/linux/device.h
+++ b/include/linux/device.h
@@ -1018,9 +1018,12 @@ static inline void device_lock_assert(struct device *dev)
 
 static inline bool dev_has_sync_state(struct device *dev)
 {
+	struct device_driver *drv;
+
 	if (!dev)
 		return false;
-	if (dev->driver && dev->driver->sync_state)
+	drv = READ_ONCE(dev->driver);
+	if (drv && drv->sync_state)
 		return true;
 	if (dev->bus && dev->bus->sync_state)
 		return true;

base-commit: 5b484311507b5d403c1f7a45f6aa3778549e268b
-- 
2.53.0

Re: [PATCH] driver core: use READ_ONCE() for dev->driver in dev_has_sync_state()

[Saravana Kannan]

On Sat, Apr 18, 2026 at 9:22 AM Danilo Krummrich <dakr@kernel.org> wrote:
>
> dev_has_sync_state() reads dev->driver twice without holding
> device_lock() -- once for the NULL check and once to dereference
> ->sync_state. Some callers only hold device_links_write_lock, which
> doesn't prevent a concurrent unbind from clearing dev->driver via
> device_unbind_cleanup().
>
> Fix it by reading dev->driver exactly once with READ_ONCE(), pairing
> with the WRITE_ONCE() in device_set_driver().
>
> Link: https://lore.kernel.org/driver-core/DHW8QPU1VU1F.3P6PH69HLFBYC@kernel.org/
> Fixes: ac338acf514e ("driver core: Add dev_has_sync_state()")
> Signed-off-by: Danilo Krummrich <dakr@kernel.org>

Reviewed-by: Saravana Kannan <saravanak@kernel.org>

-Saravana

> ---
>  include/linux/device.h | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/include/linux/device.h b/include/linux/device.h
> index ac972e7bead4..4c1c9cb8570a 100644
> --- a/include/linux/device.h
> +++ b/include/linux/device.h
> @@ -1018,9 +1018,12 @@ static inline void device_lock_assert(struct device *dev)
>
>  static inline bool dev_has_sync_state(struct device *dev)
>  {
> +       struct device_driver *drv;
> +
>         if (!dev)
>                 return false;
> -       if (dev->driver && dev->driver->sync_state)
> +       drv = READ_ONCE(dev->driver);
> +       if (drv && drv->sync_state)
>                 return true;
>         if (dev->bus && dev->bus->sync_state)
>                 return true;
>
> base-commit: 5b484311507b5d403c1f7a45f6aa3778549e268b
> --
> 2.53.0
>

Re: [PATCH] driver core: use READ_ONCE() for dev->driver in dev_has_sync_state()

[Greg KH]

On Sat, Apr 18, 2026 at 06:22:18PM +0200, Danilo Krummrich wrote:
> dev_has_sync_state() reads dev->driver twice without holding
> device_lock() -- once for the NULL check and once to dereference
> ->sync_state. Some callers only hold device_links_write_lock, which
> doesn't prevent a concurrent unbind from clearing dev->driver via
> device_unbind_cleanup().
> 
> Fix it by reading dev->driver exactly once with READ_ONCE(), pairing
> with the WRITE_ONCE() in device_set_driver().
> 
> Link: https://lore.kernel.org/driver-core/DHW8QPU1VU1F.3P6PH69HLFBYC@kernel.org/
> Fixes: ac338acf514e ("driver core: Add dev_has_sync_state()")
> Signed-off-by: Danilo Krummrich <dakr@kernel.org>
> ---
>  include/linux/device.h | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/include/linux/device.h b/include/linux/device.h
> index ac972e7bead4..4c1c9cb8570a 100644
> --- a/include/linux/device.h
> +++ b/include/linux/device.h
> @@ -1018,9 +1018,12 @@ static inline void device_lock_assert(struct device *dev)
>  
>  static inline bool dev_has_sync_state(struct device *dev)
>  {
> +	struct device_driver *drv;
> +
>  	if (!dev)
>  		return false;
> -	if (dev->driver && dev->driver->sync_state)
> +	drv = READ_ONCE(dev->driver);
> +	if (drv && drv->sync_state)
>  		return true;
>  	if (dev->bus && dev->bus->sync_state)
>  		return true;
> 
> base-commit: 5b484311507b5d403c1f7a45f6aa3778549e268b
> -- 
> 2.53.0
> 
> 

Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

Re: [PATCH] driver core: use READ_ONCE() for dev->driver in dev_has_sync_state()

[Rafael J. Wysocki]

On Sat, Apr 18, 2026 at 6:22 PM Danilo Krummrich <dakr@kernel.org> wrote:
>
> dev_has_sync_state() reads dev->driver twice without holding
> device_lock() -- once for the NULL check and once to dereference
> ->sync_state. Some callers only hold device_links_write_lock, which
> doesn't prevent a concurrent unbind from clearing dev->driver via
> device_unbind_cleanup().
>
> Fix it by reading dev->driver exactly once with READ_ONCE(), pairing
> with the WRITE_ONCE() in device_set_driver().
>
> Link: https://lore.kernel.org/driver-core/DHW8QPU1VU1F.3P6PH69HLFBYC@kernel.org/
> Fixes: ac338acf514e ("driver core: Add dev_has_sync_state()")
> Signed-off-by: Danilo Krummrich <dakr@kernel.org>

Reviewed-by: Rafael J. Wysocki (Intel) <rafael@kernel.org>

> ---
>  include/linux/device.h | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/include/linux/device.h b/include/linux/device.h
> index ac972e7bead4..4c1c9cb8570a 100644
> --- a/include/linux/device.h
> +++ b/include/linux/device.h
> @@ -1018,9 +1018,12 @@ static inline void device_lock_assert(struct device *dev)
>
>  static inline bool dev_has_sync_state(struct device *dev)
>  {
> +       struct device_driver *drv;
> +
>         if (!dev)
>                 return false;
> -       if (dev->driver && dev->driver->sync_state)
> +       drv = READ_ONCE(dev->driver);
> +       if (drv && drv->sync_state)
>                 return true;
>         if (dev->bus && dev->bus->sync_state)
>                 return true;
>

On a somewhat related note, I'm wondering if this function can be
moved to drivers/base/base.h?

It doesn't look like having it defined in device.h is particularly useful.