Commit 45ddcb42 authored by Christoph Hellwig's avatar Christoph Hellwig Committed by Alex Williamson

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: default avatarChristoph Hellwig <hch@lst.de>
Reviewed-by: default avatarCornelia Huck <cohuck@redhat.com>
Reviewed-by: default avatarGreg Kroah-Hartman <gregkh@linuxfoundation.org>
Link: https://lore.kernel.org/r/20210617142218.1877096-5-hch@lst.deSigned-off-by: default avatarAlex Williamson <alex.williamson@redhat.com>
parent ef6dcbdd
...@@ -491,15 +491,6 @@ EXPORT_SYMBOL_GPL(device_bind_driver); ...@@ -491,15 +491,6 @@ EXPORT_SYMBOL_GPL(device_bind_driver);
static atomic_t probe_count = ATOMIC_INIT(0); static atomic_t probe_count = ATOMIC_INIT(0);
static DECLARE_WAIT_QUEUE_HEAD(probe_waitqueue); 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, static ssize_t state_synced_show(struct device *dev,
struct device_attribute *attr, char *buf) struct device_attribute *attr, char *buf)
{ {
...@@ -547,10 +538,9 @@ static int call_driver_probe(struct device *dev, struct device_driver *drv) ...@@ -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) 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) && bool test_remove = IS_ENABLED(CONFIG_DEBUG_TEST_DRIVER_REMOVE) &&
!drv->suppress_bind_attrs; !drv->suppress_bind_attrs;
int ret = -EPROBE_DEFER, probe_ret = 0; int ret;
if (defer_all_probes) { if (defer_all_probes) {
/* /*
...@@ -559,17 +549,13 @@ static int really_probe(struct device *dev, struct device_driver *drv) ...@@ -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. * wait_for_device_probe() right after that to avoid any races.
*/ */
dev_dbg(dev, "Driver %s force probe deferral\n", drv->name); dev_dbg(dev, "Driver %s force probe deferral\n", drv->name);
driver_deferred_probe_add(dev); return -EPROBE_DEFER;
return ret;
} }
ret = device_links_check_suppliers(dev); ret = device_links_check_suppliers(dev);
if (ret == -EPROBE_DEFER)
driver_deferred_probe_add_trigger(dev, local_trigger_count);
if (ret) if (ret)
return ret; return ret;
atomic_inc(&probe_count);
pr_debug("bus: '%s': %s: probing driver %s with device %s\n", pr_debug("bus: '%s': %s: probing driver %s with device %s\n",
drv->bus->name, __func__, drv->name, dev_name(dev)); drv->bus->name, __func__, drv->name, dev_name(dev));
if (!list_empty(&dev->devres_head)) { if (!list_empty(&dev->devres_head)) {
...@@ -604,13 +590,13 @@ static int really_probe(struct device *dev, struct device_driver *drv) ...@@ -604,13 +590,13 @@ static int really_probe(struct device *dev, struct device_driver *drv)
goto probe_failed; goto probe_failed;
} }
probe_ret = call_driver_probe(dev, drv); ret = call_driver_probe(dev, drv);
if (probe_ret) { if (ret) {
/* /*
* Return probe errors as positive values so that the callers * Return probe errors as positive values so that the callers
* can distinguish them from other errors. * can distinguish them from other errors.
*/ */
ret = -probe_ret; ret = -ret;
goto probe_failed; goto probe_failed;
} }
...@@ -681,11 +667,7 @@ static int really_probe(struct device *dev, struct device_driver *drv) ...@@ -681,11 +667,7 @@ static int really_probe(struct device *dev, struct device_driver *drv)
dev->pm_domain->dismiss(dev); dev->pm_domain->dismiss(dev);
pm_runtime_reinit(dev); pm_runtime_reinit(dev);
dev_pm_set_driver_flags(dev, 0); dev_pm_set_driver_flags(dev, 0);
if (probe_ret == -EPROBE_DEFER)
driver_deferred_probe_add_trigger(dev, local_trigger_count);
done: done:
atomic_dec(&probe_count);
wake_up_all(&probe_waitqueue);
return ret; return ret;
} }
...@@ -739,21 +721,7 @@ void wait_for_device_probe(void) ...@@ -739,21 +721,7 @@ void wait_for_device_probe(void)
} }
EXPORT_SYMBOL_GPL(wait_for_device_probe); EXPORT_SYMBOL_GPL(wait_for_device_probe);
/** static int __driver_probe_device(struct device_driver *drv, struct device *dev)
* 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 ret = 0; int ret = 0;
...@@ -784,6 +752,42 @@ static int driver_probe_device(struct device_driver *drv, struct device *dev) ...@@ -784,6 +752,42 @@ static int driver_probe_device(struct device_driver *drv, struct device *dev)
return ret; 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) static inline bool cmdline_requested_async_probing(const char *drv_name)
{ {
return parse_option_str(async_probe_drv_names, 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) ...@@ -1051,12 +1055,14 @@ int device_driver_attach(struct device_driver *drv, struct device *dev)
int ret; int ret;
__device_driver_lock(dev, dev->parent); __device_driver_lock(dev, dev->parent);
ret = driver_probe_device(drv, dev); ret = __driver_probe_device(drv, dev);
__device_driver_unlock(dev, dev->parent); __device_driver_unlock(dev, dev->parent);
/* also return probe errors as normal negative errnos */ /* also return probe errors as normal negative errnos */
if (ret > 0) if (ret > 0)
ret = -ret; ret = -ret;
if (ret == -EPROBE_DEFER)
return -EAGAIN;
return ret; return ret;
} }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment