Allow userspace to request device probing even if defer_all_probes is true
PATCH 1/1 Allow userspace to request device probing even if defer_all_probes is true
From: Matthew Garrett <[email protected]>
Userspace may wish to make a policy decision to allow certain devices to be attached, such as keyboards. Add a force_probe sysfs node to each device, which if written will trigger a probe even if defer_all_probes is currently true.
--- .../ABI/testing/sysfs-devices-force_probe | 10 +++++ drivers/base/base.h | 4 +- drivers/base/bus.c | 2 +- drivers/base/core.c | 7 ++- drivers/base/dd.c | 51 ++++++++++++++++++---- 5 files changed, 62 insertions(+), 12 deletions(-) create mode 100644 Documentation/ABI/testing/sysfs-devices-force_probe
Documentation/ABI/testing/sysfs-devices-force_probe new file +10 -0
@@ -0,0 +1,10 @@ +What: /sys/devices/.../force_probe +Date: December 2016 +KernelVersion: 4.11 +Contact: Matthew Garrett <[email protected]> +Description: + The /sys/devices/.../force_probe attribute is + present for all devices. If deferred probing is globally + enabled and the device has no driver bound, a write to this + node will trigger probing. This attribute reads as 1 if the + device currently has a driver bound, and 0 otherwise.
drivers/base/base.h +3 -1
@@ -112,7 +112,8 @@ extern void device_release_driver_internal(struct device *dev, struct device *parent);extern void driver_detach (struct device_driver *drv); -extern int driver_probe_device (struct device_driver *drv,struct device *dev); +extern int driver_probe_device (struct device_driver *drv,struct device *dev, +bool force);extern void driver_deferred_probe_del (struct device *dev);static inline int driver_match_device (struct device_driver *drv,struct device *dev)
@@ -140,6 +141,7 @@ extern struct kset *devices_kset;extern void devices_kset_move_last (struct device *dev);extern struct device_attribute dev_attr_deferred_probe; +extern struct device_attribute dev_attr_force_probe;#if defined(CONFIG_MODULES ) && defined(CONFIG_SYSFS )extern void module_add_driver (struct module *mod,struct device_driver *drv);
drivers/base/bus.c +1 -1
@@ -216,7 +216,7 @@ static ssize_t bind_store(struct device_driver *drv, const char *buf,if (dev->parent)/* Needed for USB */ device_lock (dev->parent);device_lock (dev); - err =driver_probe_device (drv, dev); + err =driver_probe_device (drv, dev, true);device_unlock (dev);if (dev->parent)device_unlock (dev->parent);
drivers/base/core.c +6 -1
@@ -1064,8 +1064,13 @@ static int device_add_attrs(struct device *dev)if (error) goto err_remove_online; -return 0 ; + error =device_create_file (dev, &dev_attr_force_probe); +if (error) + goto err_remove_deferred_probe; +return 0 ; + err_remove_deferred_probe: +device_remove_file (dev, &dev_attr_deferred_probe); err_remove_online:device_remove_file (dev, &dev_attr_online); err_remove_dev_groups:
drivers/base/dd.c +42 -9
122 lines not on this page · raw message
Signature · 6 lines
2.7.4 -- Kees Cook Nexus Security
Greg Kroah-Hartman
maintainerQuoted 4 lines · Kees Cook
From: Matthew Garrett <[email protected]>
Userspace may wish to make a policy decision to allow certain devices to be attached, such as keyboards.
I don't understand what that sentance means. Why wouldn't keyboards be attached?
Quoted 2 lines
Add a
force_probesysfs node to each device, which if written will trigger a probe even ifdefer_all_probesis currently true.
Why not just manually trigger the bind of the device? I don't understand the problem here that is being addressed, nor do I understand how this would be used. More explaination please.
thanks,
greg k-h
Quoted 6 lines · Greg Kroah-Hartman
Quoted 2 lines
Add a
force_probesysfs node to each device, which if written will trigger a probe even ifdefer_all_probesis currently true.Why not just manually trigger the bind of the device? I don't understand the problem here that is being addressed, nor do I understand how this would be used. More explaination please.
Userspace doesn't know the order that the kernel will use when attempting to bind drivers, so punting binding out to userspace may result in different behaviour. The kernel already has the code to do this, so we should just reuse it.
Greg Kroah-Hartman
maintainerQuoted 5 lines · Matthew Garrett
Quoted 3 lines · Greg Kroah-Hartman
Quoted 2 lines
Add a
force_probesysfs node to each device, which if written will trigger a probe even ifdefer_all_probesis currently true.
Quoted 7 lines
Quoted 3 lines
Why not just manually trigger the bind of the device? I don't understand the problem here that is being addressed, nor do I understand how this would be used. More explaination please.
Userspace doesn't know the order that the kernel will use when attempting to bind drivers, so punting binding out to userspace may result in different behaviour.
How can the order in which drivers are bound result in different behavior?
Quoted 2 lines
The kernel already has the code to do this, so we should just reuse it.
That's fine, but I don't understand the problem you are trying to solve, please explain better. What am I missing here?
thanks,
greg k-h
Quoted 7 lines · Greg Kroah-Hartman
Quoted 3 lines · Matthew Garrett
Userspace doesn't know the order that the kernel will use when attempting to bind drivers, so punting binding out to userspace may result in different behaviour.
How can the order in which drivers are bound result in different behavior?
If you have two loaded drivers that could bind to the device then the order you attempt to bind them in will matter.
Quoted 5 lines
Quoted 2 lines
The kernel already has the code to do this, so we should just reuse it.
That's fine, but I don't understand the problem you are trying to solve, please explain better. What am I missing here?
If you plug in a device while defer_all_probes is true, it won't be bound - that's the point. But if you have a USB keyboard and unplug it and plug it, you'd then end up with no keyboard. So you want userspace to be able to make an appropriate policy decision around which devices should be bound, and you need a mechanism to allow userspace to trigger that binding.
Greg Kroah-Hartman
maintainerQuoted 7 lines · Matthew Garrett
Quoted 5 lines · Greg Kroah-Hartman
Quoted 3 lines · Matthew Garrett
Userspace doesn't know the order that the kernel will use when attempting to bind drivers, so punting binding out to userspace may result in different behaviour.
Quoted 5 lines
Quoted 2 lines
How can the order in which drivers are bound result in different behavior?
If you have two loaded drivers that could bind to the device then the order you attempt to bind them in will matter.
If you have that, you are screwed no matter what. The driver model never guarantees any order in which a driver and device is matched up, sorry, and if that's the goal of this patch somehow, then I'll strongly object to it.
What in-kernel drivers do we have that bind to the same device? We shouldn't have that, because of this very issue.
Quoted 6 lines
Quoted 5 lines
Quoted 2 lines
The kernel already has the code to do this, so we should just reuse it.
That's fine, but I don't understand the problem you are trying to solve, please explain better. What am I missing here?
Quoted 6 lines
If you plug in a device while
defer_all_probesis true, it won't be bound - that's the point. But if you have a USB keyboard and unplug it and plug it, you'd then end up with no keyboard. So you want userspace to be able to make an appropriate policy decision around which devices should be bound, and you need a mechanism to allow userspace to trigger that binding.
Use the in-place mechanism for that, userspace gets notification that the device was plugged in, it can authorize it or not. That's what systems have been doing for a while now, and is what that api was created for.
I'm getting the impression that somehow these two different patches are a series and related to each other which is even more confusing...
thanks,
greg k-h
Quoted 8 lines · Greg Kroah-Hartman
Quoted 2 lines · Matthew Garrett
If you have two loaded drivers that could bind to the device then the order you attempt to bind them in will matter.
If you have that, you are screwed no matter what. The driver model never guarantees any order in which a driver and device is matched up, sorry, and if that's the goal of this patch somehow, then I'll strongly object to it.
It's not? The reason for implementing it this way is to avoid duplicating functionality the kernel already has, especially when it's impossible to precisely duplicate that behaviour in userspace.
Quoted 11 lines
Quoted 6 lines
If you plug in a device while
defer_all_probesis true, it won't be bound - that's the point. But if you have a USB keyboard and unplug it and plug it, you'd then end up with no keyboard. So you want userspace to be able to make an appropriate policy decision around which devices should be bound, and you need a mechanism to allow userspace to trigger that binding.Use the in-place mechanism for that, userspace gets notification that the device was plugged in, it can authorize it or not. That's what systems have been doing for a while now, and is what that api was created for.
That doesn't work - the USB authorisation code doesn't give you enough information to make that decision before driver binding occurs.
Quoted 2 lines
I'm getting the impression that somehow these two different patches are a series and related to each other which is even more confusing...
Yes, this is only relevant with the other patch.