[PATCH RFC] driver core: bus: Correct return value for storing bus attribute drivers_probe

Zijun Hu posted 1 patch 1 year, 3 months ago
drivers/base/bus.c | 23 ++++++++++++++++++++++-
1 file changed, 22 insertions(+), 1 deletion(-)
[PATCH RFC] driver core: bus: Correct return value for storing bus attribute drivers_probe
Posted by Zijun Hu 1 year, 3 months ago
From: Zijun Hu <quic_zijuhu@quicinc.com>

drivers_probe_store() regards bus_rescan_devices_helper()'s returned
value 0 as success when find driver for a single device user specify
that is wrong since the following 3 failed cases also return 0:

(1) the device is dead
(2) bus fails to match() the device with any its driver
(3) bus fails to probe() the device with any its driver

Fixed by only regarding successfully attaching the device to a driver
as success.

Fixes: b8c5cec23d5c ("Driver core: udev triggered device-<>driver binding")
Cc: stable@vger.kernel.org
Signed-off-by: Zijun Hu <quic_zijuhu@quicinc.com>
---
 drivers/base/bus.c | 23 ++++++++++++++++++++++-
 1 file changed, 22 insertions(+), 1 deletion(-)

diff --git a/drivers/base/bus.c b/drivers/base/bus.c
index abf090ace833..0a994e63785c 100644
--- a/drivers/base/bus.c
+++ b/drivers/base/bus.c
@@ -40,6 +40,20 @@ static struct kset *bus_kset;
 	struct driver_attribute driver_attr_##_name =		\
 		__ATTR_IGNORE_LOCKDEP(_name, _mode, _show, _store)
 
+/* Bus rescans drivers for a single device */
+static int __must_check bus_rescan_single_device(struct device *dev)
+{
+	int ret;
+
+	if (dev->parent && dev->bus->need_parent_lock)
+		device_lock(dev->parent);
+	ret = device_attach(dev);
+	if (dev->parent && dev->bus->need_parent_lock)
+		device_unlock(dev->parent);
+
+	return ret;
+}
+
 static int __must_check bus_rescan_devices_helper(struct device *dev,
 						void *data);
 
@@ -311,12 +325,19 @@ static ssize_t drivers_probe_store(const struct bus_type *bus,
 {
 	struct device *dev;
 	int err = -EINVAL;
+	int res;
 
 	dev = bus_find_device_by_name(bus, NULL, buf);
 	if (!dev)
 		return -ENODEV;
-	if (bus_rescan_devices_helper(dev, NULL) == 0)
+
+	res = bus_rescan_single_device(dev);
+	/* Propagate error code upwards as far as possible */
+	if (res < 0)
+		err = res;
+	else if (res == 1)
 		err = count;
+
 	put_device(dev);
 	return err;
 }

---
base-commit: 888f67e621dda5c2804a696524e28d0ca4cf0a80
change-id: 20240826-fix_drivers_probe-88c6a2cc1899

Best regards,
-- 
Zijun Hu <quic_zijuhu@quicinc.com>
Re: [PATCH RFC] driver core: bus: Correct return value for storing bus attribute drivers_probe
Posted by Zijun Hu 1 year, 3 months ago
On 2024/8/26 20:23, Zijun Hu wrote:
> From: Zijun Hu <quic_zijuhu@quicinc.com>
> 
> drivers_probe_store() regards bus_rescan_devices_helper()'s returned
> value 0 as success when find driver for a single device user specify
> that is wrong since the following 3 failed cases also return 0:
> 
> (1) the device is dead
> (2) bus fails to match() the device with any its driver
> (3) bus fails to probe() the device with any its driver
> 
> Fixed by only regarding successfully attaching the device to a driver
> as success.
> 
> Fixes: b8c5cec23d5c ("Driver core: udev triggered device-<>driver binding")
> Cc: stable@vger.kernel.org
> Signed-off-by: Zijun Hu <quic_zijuhu@quicinc.com>
> ---
>  drivers/base/bus.c | 23 ++++++++++++++++++++++-
>  1 file changed, 22 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/base/bus.c b/drivers/base/bus.c
> index abf090ace833..0a994e63785c 100644
> --- a/drivers/base/bus.c
> +++ b/drivers/base/bus.c
> @@ -40,6 +40,20 @@ static struct kset *bus_kset;
>  	struct driver_attribute driver_attr_##_name =		\
>  		__ATTR_IGNORE_LOCKDEP(_name, _mode, _show, _store)
>  
> +/* Bus rescans drivers for a single device */
> +static int __must_check bus_rescan_single_device(struct device *dev)
> +{
> +	int ret;
> +
> +	if (dev->parent && dev->bus->need_parent_lock)
> +		device_lock(dev->parent);
> +	ret = device_attach(dev);
> +	if (dev->parent && dev->bus->need_parent_lock)
> +		device_unlock(dev->parent);
> +
> +	return ret;
> +}
> +
>  static int __must_check bus_rescan_devices_helper(struct device *dev,
>  						void *data);
>  
> @@ -311,12 +325,19 @@ static ssize_t drivers_probe_store(const struct bus_type *bus,
>  {
>  	struct device *dev;
>  	int err = -EINVAL;
> +	int res;
>  
>  	dev = bus_find_device_by_name(bus, NULL, buf);
>  	if (!dev)
>  		return -ENODEV;
> -	if (bus_rescan_devices_helper(dev, NULL) == 0)
> +
> +	res = bus_rescan_single_device(dev);
> +	/* Propagate error code upwards as far as possible */
> +	if (res < 0)
> +		err = res;
> +	else if (res == 1)
>  		err = count;
> +
>  	put_device(dev);
>  	return err;
>  }
> 

please ignore RFC for this patch, will push it within a patch
series.

thanks
> ---
> base-commit: 888f67e621dda5c2804a696524e28d0ca4cf0a80
> change-id: 20240826-fix_drivers_probe-88c6a2cc1899
> 
> Best regards,