From 45ddcb42949f825f0caa25352e825cede94b6aba Mon Sep 17 00:00:00 2001 From: Christoph Hellwig Date: Thu, 17 Jun 2021 16:22:12 +0200 Subject: [PATCH] driver core: Don't return EPROBE_DEFER to userspace during sysfs bind EPROBE_DEFER is an internal kernel error code and it should not be leaked to userspace via the bind_store() sysfs. Userspace doesn't have this constant and cannot understand it. Further, it doesn't really make sense to have userspace trigger a deferred probe via bind_store(), which could eventually succeed, while simultaneously returning an error back. Resolve this by splitting driver_probe_device so that the version used by the sysfs binding that turns EPROBE_DEFER into -EAGAIN, while the one used for internally binding keeps the error code, and calls driver_deferred_probe_add where needed. This also allows to nicely split out the defer_all_probes / probe_count checks so that they actually allow for full device_{block,unblock}_probing protection while not bothering the sysfs bind case. Signed-off-by: Christoph Hellwig Reviewed-by: Cornelia Huck Reviewed-by: Greg Kroah-Hartman Link: https://lore.kernel.org/r/20210617142218.1877096-5-hch@lst.de Signed-off-by: Alex Williamson --- drivers/base/dd.c | 84 +++++++++++++++++++++++++++++-------------------------- 1 file changed, 45 insertions(+), 39 deletions(-) diff --git a/drivers/base/dd.c b/drivers/base/dd.c index 25341f5..1d80124 100644 --- a/drivers/base/dd.c +++ b/drivers/base/dd.c @@ -491,15 +491,6 @@ EXPORT_SYMBOL_GPL(device_bind_driver); static atomic_t probe_count = ATOMIC_INIT(0); static DECLARE_WAIT_QUEUE_HEAD(probe_waitqueue); -static void driver_deferred_probe_add_trigger(struct device *dev, - int local_trigger_count) -{ - driver_deferred_probe_add(dev); - /* Did a trigger occur while probing? Need to re-trigger if yes */ - if (local_trigger_count != atomic_read(&deferred_trigger_count)) - driver_deferred_probe_trigger(); -} - static ssize_t state_synced_show(struct device *dev, struct device_attribute *attr, char *buf) { @@ -547,10 +538,9 @@ static int call_driver_probe(struct device *dev, struct device_driver *drv) static int really_probe(struct device *dev, struct device_driver *drv) { - int local_trigger_count = atomic_read(&deferred_trigger_count); bool test_remove = IS_ENABLED(CONFIG_DEBUG_TEST_DRIVER_REMOVE) && !drv->suppress_bind_attrs; - int ret = -EPROBE_DEFER, probe_ret = 0; + int ret; if (defer_all_probes) { /* @@ -559,17 +549,13 @@ static int really_probe(struct device *dev, struct device_driver *drv) * wait_for_device_probe() right after that to avoid any races. */ dev_dbg(dev, "Driver %s force probe deferral\n", drv->name); - driver_deferred_probe_add(dev); - return ret; + return -EPROBE_DEFER; } ret = device_links_check_suppliers(dev); - if (ret == -EPROBE_DEFER) - driver_deferred_probe_add_trigger(dev, local_trigger_count); if (ret) return ret; - atomic_inc(&probe_count); pr_debug("bus: '%s': %s: probing driver %s with device %s\n", drv->bus->name, __func__, drv->name, dev_name(dev)); if (!list_empty(&dev->devres_head)) { @@ -604,13 +590,13 @@ re_probe: goto probe_failed; } - probe_ret = call_driver_probe(dev, drv); - if (probe_ret) { + ret = call_driver_probe(dev, drv); + if (ret) { /* * Return probe errors as positive values so that the callers * can distinguish them from other errors. */ - ret = -probe_ret; + ret = -ret; goto probe_failed; } @@ -681,11 +667,7 @@ pinctrl_bind_failed: dev->pm_domain->dismiss(dev); pm_runtime_reinit(dev); dev_pm_set_driver_flags(dev, 0); - if (probe_ret == -EPROBE_DEFER) - driver_deferred_probe_add_trigger(dev, local_trigger_count); done: - atomic_dec(&probe_count); - wake_up_all(&probe_waitqueue); return ret; } @@ -739,21 +721,7 @@ void wait_for_device_probe(void) } EXPORT_SYMBOL_GPL(wait_for_device_probe); -/** - * driver_probe_device - attempt to bind device & driver together - * @drv: driver to bind a device to - * @dev: device to try to bind to the driver - * - * This function returns -ENODEV if the device is not registered, -EBUSY if it - * already has a driver, 0 if the device is bound successfully and a positive - * (inverted) error code for failures from the ->probe method. - * - * This function must be called with @dev lock held. When called for a - * USB interface, @dev->parent lock must be held as well. - * - * If the device has a parent, runtime-resume the parent before driver probing. - */ -static int driver_probe_device(struct device_driver *drv, struct device *dev) +static int __driver_probe_device(struct device_driver *drv, struct device *dev) { int ret = 0; @@ -784,6 +752,42 @@ static int driver_probe_device(struct device_driver *drv, struct device *dev) return ret; } +/** + * driver_probe_device - attempt to bind device & driver together + * @drv: driver to bind a device to + * @dev: device to try to bind to the driver + * + * This function returns -ENODEV if the device is not registered, -EBUSY if it + * already has a driver, 0 if the device is bound successfully and a positive + * (inverted) error code for failures from the ->probe method. + * + * This function must be called with @dev lock held. When called for a + * USB interface, @dev->parent lock must be held as well. + * + * If the device has a parent, runtime-resume the parent before driver probing. + */ +static int driver_probe_device(struct device_driver *drv, struct device *dev) +{ + int trigger_count = atomic_read(&deferred_trigger_count); + int ret; + + atomic_inc(&probe_count); + ret = __driver_probe_device(drv, dev); + if (ret == -EPROBE_DEFER || ret == EPROBE_DEFER) { + driver_deferred_probe_add(dev); + + /* + * Did a trigger occur while probing? Need to re-trigger if yes + */ + if (trigger_count != atomic_read(&deferred_trigger_count) && + !defer_all_probes) + driver_deferred_probe_trigger(); + } + atomic_dec(&probe_count); + wake_up_all(&probe_waitqueue); + return ret; +} + static inline bool cmdline_requested_async_probing(const char *drv_name) { return parse_option_str(async_probe_drv_names, drv_name); @@ -1051,12 +1055,14 @@ int device_driver_attach(struct device_driver *drv, struct device *dev) int ret; __device_driver_lock(dev, dev->parent); - ret = driver_probe_device(drv, dev); + ret = __driver_probe_device(drv, dev); __device_driver_unlock(dev, dev->parent); /* also return probe errors as normal negative errnos */ if (ret > 0) ret = -ret; + if (ret == -EPROBE_DEFER) + return -EAGAIN; return ret; } -- 2.7.4