| Message ID | 20240202222603.141240-1-hamza.mahfooz@amd.com |
|---|---|
| State | New |
| Headers | show |
| Series | [1/3] driver core: bus: introduce can_remove() | expand |
On Fri, Feb 02, 2024 at 05:25:54PM -0500, Hamza Mahfooz wrote: > Currently, drivers have no mechanism to block requests to unbind > devices. And that is by design. > However, this can cause resource leaks and leave the device in > an inconsistent state, such that rebinding the device may cause a hang > or otherwise prevent the device from being rebound. That is a driver bug, please fix your driver. > So, introduce the can_remove() callback to allow drivers to indicate > if it isn't appropriate to remove a device at the given time. Nope, sorry, the driver needs to be fixed. What broken driver are you needing this for? Also realize, only root can unbind drivers (and it can also unload modules), so if the root user really wants to do this, it can, and should be possible to. sorry, greg k-h
Am 02.02.24 um 23:25 schrieb Hamza Mahfooz: > Currently, drivers have no mechanism to block requests to unbind > devices. However, this can cause resource leaks and leave the device in > an inconsistent state, such that rebinding the device may cause a hang > or otherwise prevent the device from being rebound. So, introduce > the can_remove() callback to allow drivers to indicate if it isn't > appropriate to remove a device at the given time. Well that is nonsense. When you physically remove a device (e.g. unplug it) then there is nothing in software you can do to prevent that. Regards, Christian. > > Cc: stable@vger.kernel.org > Signed-off-by: Hamza Mahfooz <hamza.mahfooz@amd.com> > --- > drivers/base/bus.c | 4 ++++ > include/linux/device/bus.h | 2 ++ > 2 files changed, 6 insertions(+) > > diff --git a/drivers/base/bus.c b/drivers/base/bus.c > index daee55c9b2d9..7c259b01ea99 100644 > --- a/drivers/base/bus.c > +++ b/drivers/base/bus.c > @@ -239,6 +239,10 @@ static ssize_t unbind_store(struct device_driver *drv, const char *buf, > > dev = bus_find_device_by_name(bus, NULL, buf); > if (dev && dev->driver == drv) { > + if (dev->bus && dev->bus->can_remove && > + !dev->bus->can_remove(dev)) > + return -EBUSY; > + > device_driver_detach(dev); > err = count; > } > diff --git a/include/linux/device/bus.h b/include/linux/device/bus.h > index 5ef4ec1c36c3..c9d4af0ed3b8 100644 > --- a/include/linux/device/bus.h > +++ b/include/linux/device/bus.h > @@ -46,6 +46,7 @@ struct fwnode_handle; > * 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. > + * @can_remove: Called before attempting to remove a device from this bus. > * @remove: Called when a device removed from this bus. > * @shutdown: Called at shut-down time to quiesce the device. > * > @@ -85,6 +86,7 @@ struct bus_type { > int (*uevent)(const struct device *dev, struct kobj_uevent_env *env); > int (*probe)(struct device *dev); > void (*sync_state)(struct device *dev); > + bool (*can_remove)(struct device *dev); > void (*remove)(struct device *dev); > void (*shutdown)(struct device *dev); >
diff --git a/drivers/base/bus.c b/drivers/base/bus.c index daee55c9b2d9..7c259b01ea99 100644 --- a/drivers/base/bus.c +++ b/drivers/base/bus.c @@ -239,6 +239,10 @@ static ssize_t unbind_store(struct device_driver *drv, const char *buf, dev = bus_find_device_by_name(bus, NULL, buf); if (dev && dev->driver == drv) { + if (dev->bus && dev->bus->can_remove && + !dev->bus->can_remove(dev)) + return -EBUSY; + device_driver_detach(dev); err = count; } diff --git a/include/linux/device/bus.h b/include/linux/device/bus.h index 5ef4ec1c36c3..c9d4af0ed3b8 100644 --- a/include/linux/device/bus.h +++ b/include/linux/device/bus.h @@ -46,6 +46,7 @@ struct fwnode_handle; * 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. + * @can_remove: Called before attempting to remove a device from this bus. * @remove: Called when a device removed from this bus. * @shutdown: Called at shut-down time to quiesce the device. * @@ -85,6 +86,7 @@ struct bus_type { int (*uevent)(const struct device *dev, struct kobj_uevent_env *env); int (*probe)(struct device *dev); void (*sync_state)(struct device *dev); + bool (*can_remove)(struct device *dev); void (*remove)(struct device *dev); void (*shutdown)(struct device *dev);
Currently, drivers have no mechanism to block requests to unbind devices. However, this can cause resource leaks and leave the device in an inconsistent state, such that rebinding the device may cause a hang or otherwise prevent the device from being rebound. So, introduce the can_remove() callback to allow drivers to indicate if it isn't appropriate to remove a device at the given time. Cc: stable@vger.kernel.org Signed-off-by: Hamza Mahfooz <hamza.mahfooz@amd.com> --- drivers/base/bus.c | 4 ++++ include/linux/device/bus.h | 2 ++ 2 files changed, 6 insertions(+)