Re: [PATCH] Disable USB bus probes
On Mon, Apr 27, 2015 at 11:18:55AM +0200, Martin Pieuchot wrote: On 22/04/15(Wed) 10:29, Dimitris Papastamos wrote: [...] On a side note, What do you think of using a sysctl instead of an ioctl? I guess an ioctl will do but it would make it easy for people to disable this at boot time by simply adding the relevant knob in /etc/sysctl.conf. Otherwise I imagine people adding usbdevs -p off in rc.local. I think it makes sense. Note that we're currently not using sysctls for USB so I wonder where the node should be placed. hw.usb I guess. OK, should I have a look at adding the top-level sysctl node and the knob for controlling bus probing? I can then re-work my patch on top of that.
Re: [PATCH] Disable USB bus probes
On 22/04/15(Wed) 10:29, Dimitris Papastamos wrote: [...] On a side note, What do you think of using a sysctl instead of an ioctl? I guess an ioctl will do but it would make it easy for people to disable this at boot time by simply adding the relevant knob in /etc/sysctl.conf. Otherwise I imagine people adding usbdevs -p off in rc.local. I think it makes sense. Note that we're currently not using sysctls for USB so I wonder where the node should be placed. hw.usb I guess.
Re: [PATCH] Disable USB bus probes
On 17/04/15(Fri) 16:47, Dimitris Papastamos wrote:
Hi,
This patch adds an option to usbdevs(8) to disable USB bus probing
at runtime. The operation is restricted to the root user.
It would be nice to show if probing is on or off, for example
# usbdevs -p
bus probing: on
# usbdevs -p off
But other people might have better suggestions.
I am not sure if this approach is sensible or even correct. Some
pointers would be much appreciated.
Setting a variable per hub (and here roothub) is overkill, a global
would be enough.
This was started as part of a reply by mpi on tech@
http://marc.info/?l=openbsd-techm=142917883126679w=2
I guess the reasoning behind this is to add some protection against
things like badusb?
It can have multiple usages :)
How did you try it? What happen if you plug a hub with multiple
devices, turn bus probing off then detach the hub?
What happen if you plug a device like a phone that use the power to
charge its battery after turning probing off. If I read your diff
correctly you still allow the device to be charged which is fine. Did
you try that? I think it's worth a documentation note.
if (!dev-self_powered dev-powersrc-parent != NULL
!dev-powersrc-parent-self_powered) {
@@ -494,6 +495,9 @@ uhub_explore(struct usbd_device *dev)
*/
if (speed sc-sc_hub-speed)
speed = sc-sc_hub-speed;
+
+ if (dev-hub-noprobe)
+ return (0);
I believe you can move that before the Figure out device speed.
Re: [PATCH] Disable USB bus probes
Below is an updated diff. I will add the documentation bits
soonish once I've tested all possible configurations as you
requested.
I am also planning to rework the structure of usbdevs.c as it is
a bit confusing currently but that will happen incrementally after
this diff.
Index: sys/dev/usb/uhub.c
===
RCS file: /cvs/src/sys/dev/usb/uhub.c,v
retrieving revision 1.83
diff -u -p -r1.83 uhub.c
--- sys/dev/usb/uhub.c 12 Feb 2015 05:07:52 - 1.83
+++ sys/dev/usb/uhub.c 22 Apr 2015 11:10:46 -
@@ -53,6 +53,9 @@
#define DPRINTF(x...)
#endif
+/* controls enabling/disabling of USB bus probing */
+int busprobe = 1;
+
struct uhub_softc {
struct device sc_dev; /* base device */
struct usbd_device *sc_hub;/* USB device */
@@ -487,6 +490,9 @@ uhub_explore(struct usbd_device *dev)
else
speed = sc-sc_hub-speed;
}
+
+ if (!busprobe)
+ return (0);
/*
* Reduce the speed, otherwise we won't setup the proper
Index: sys/dev/usb/usb.c
===
RCS file: /cvs/src/sys/dev/usb/usb.c,v
retrieving revision 1.107
diff -u -p -r1.107 usb.c
--- sys/dev/usb/usb.c 14 Mar 2015 03:38:50 - 1.107
+++ sys/dev/usb/usb.c 22 Apr 2015 11:10:46 -
@@ -87,6 +87,8 @@ int usb_noexplore = 0;
#define DPRINTFN(n,x)
#endif
+extern int busprobe;
+
struct usb_softc {
struct devicesc_dev;/* base device */
struct usbd_bus *sc_bus; /* USB controller */
@@ -607,6 +609,14 @@ usbioctl(dev_t devt, u_long cmd, caddr_t
#endif
break;
#endif /* USB_DEBUG */
+ case USB_GET_BUS_PROBE:
+ *(unsigned int *)data = busprobe;
+ break;
+ case USB_SET_BUS_PROBE:
+ if ((error = suser(curproc, 0)) != 0)
+ return (error);
+ busprobe = !!*(unsigned int *)data;
+ break;
case USB_REQUEST:
{
struct usb_ctl_request *ur = (void *)data;
Index: sys/dev/usb/usb.h
===
RCS file: /cvs/src/sys/dev/usb/usb.h,v
retrieving revision 1.50
diff -u -p -r1.50 usb.h
--- sys/dev/usb/usb.h 14 Feb 2015 06:18:58 - 1.50
+++ sys/dev/usb/usb.h 22 Apr 2015 11:10:46 -
@@ -720,14 +720,16 @@ struct usb_device_stats {
};
/* USB controller */
-#define USB_REQUEST_IOWR('U', 1, struct usb_ctl_request)
-#define USB_SETDEBUG _IOW ('U', 2, unsigned int)
+#define USB_REQUEST_IOWR('U', 1, struct usb_ctl_request)
+#define USB_SETDEBUG _IOW ('U', 2, unsigned int)
#define USB_DISCOVER _IO ('U', 3)
-#define USB_DEVICEINFO _IOWR('U', 4, struct usb_device_info)
-#define USB_DEVICESTATS_IOR ('U', 5, struct usb_device_stats)
-#define USB_DEVICE_GET_CDESC _IOWR('U', 6, struct usb_device_cdesc)
-#define USB_DEVICE_GET_FDESC _IOWR('U', 7, struct usb_device_fdesc)
-#define USB_DEVICE_GET_DDESC _IOWR('U', 8, struct usb_device_ddesc)
+#define USB_DEVICEINFO _IOWR('U', 4, struct usb_device_info)
+#define USB_DEVICESTATS_IOR ('U', 5, struct usb_device_stats)
+#define USB_DEVICE_GET_CDESC _IOWR('U', 6, struct usb_device_cdesc)
+#define USB_DEVICE_GET_FDESC _IOWR('U', 7, struct usb_device_fdesc)
+#define USB_DEVICE_GET_DDESC _IOWR('U', 8, struct usb_device_ddesc)
+#define USB_GET_BUS_PROBE _IOR ('U', 9, unsigned int)
+#define USB_SET_BUS_PROBE _IOW ('U', 10, unsigned int)
/* Generic HID device */
#define USB_GET_REPORT_DESC_IOR ('U', 21, struct usb_ctl_report_desc)
Index: usr.sbin/usbdevs/usbdevs.8
===
RCS file: /cvs/src/usr.sbin/usbdevs/usbdevs.8,v
retrieving revision 1.9
diff -u -p -r1.9 usbdevs.8
--- usr.sbin/usbdevs/usbdevs.8 26 Jun 2008 05:42:21 - 1.9
+++ usr.sbin/usbdevs/usbdevs.8 22 Apr 2015 11:10:46 -
@@ -39,6 +39,7 @@
.Op Fl dv
.Op Fl a Ar addr
.Op Fl f Ar dev
+.Op Fl p Ns Op Ar on | off
.Sh DESCRIPTION
.Nm
prints a listing of all USB devices connected to the system
@@ -53,6 +54,10 @@ Only print information about the device
Show the device drivers associated with each device.
.It Fl f Ar dev
Only print information for the given USB controller.
+.It Fl p Ns Op Ar on | off
+Enable or disable USB bus probing. The default
+is
+.Ar on .
.It Fl v
Be verbose.
.El
Index: usr.sbin/usbdevs/usbdevs.c
===
RCS file: /cvs/src/usr.sbin/usbdevs/usbdevs.c,v
retrieving revision 1.24
diff -u -p -r1.24 usbdevs.c
--- usr.sbin/usbdevs/usbdevs.c 31 Mar 2015 13:38:27 - 1.24
+++ usr.sbin/usbdevs/usbdevs.c 22 Apr 2015
Re: [PATCH] Disable USB bus probes
On Wed, Apr 22, 2015 at 08:54:34AM +0200, Martin Pieuchot wrote: On 17/04/15(Fri) 16:47, Dimitris Papastamos wrote: Hi, This patch adds an option to usbdevs(8) to disable USB bus probing at runtime. The operation is restricted to the root user. It would be nice to show if probing is on or off, for example # usbdevs -p bus probing: on # usbdevs -p off But other people might have better suggestions. Yeah I thought about that, I will add it. I am not sure if this approach is sensible or even correct. Some pointers would be much appreciated. Setting a variable per hub (and here roothub) is overkill, a global would be enough. Will fix. How did you try it? What happen if you plug a hub with multiple devices, turn bus probing off then detach the hub? I have not tried connecting a hub, but connecting a device with probing on, disabling probing and then removing works as expected by first detaching the device and disallowing any further probes after disabling them. I will try with a hub too. What happen if you plug a device like a phone that use the power to charge its battery after turning probing off. If I read your diff correctly you still allow the device to be charged which is fine. Did you try that? I think it's worth a documentation note. Not tried this yet, but will do. On a side note, What do you think of using a sysctl instead of an ioctl? I guess an ioctl will do but it would make it easy for people to disable this at boot time by simply adding the relevant knob in /etc/sysctl.conf. Otherwise I imagine people adding usbdevs -p off in rc.local. Cheers, Dimitris
