[PATH 0/4] usbip: make vhci_hcd.* objects independent of vhci_hcd.0

[PATH 0/4] usbip: make vhci_hcd.* objects independent of vhci_hcd.0

[Salvador Fandino]

Let me start by explaining the problem that have motivated me to write
this patches:

I work on the QVD, a virtual desktop platform for Linux. This software
runs Linux desktops (i.e. XFCE, KDE) and their applications inside LXC
containers, and makes then available through the network to remote
users.

Supporting USB devices is a common feature customers have been
requesting us for a long time (in order to use, for instance, remote
signature pads, bar-code scanners, fingerprint readers, etc.). So, we
have been working on that feature using the USB/IP layer on the
kernel.

Connecting and disconnecting devices and transferring data works
seamless for the devices listed above. But we also want to make the
usbip operations private to the container where they are run.  For
instance, it is unacceptable for our product, that one user could list
the devices connected by other users or access them.

We can control how can access every device using cgroups once those
are attached, but the usbip layer is not providing any mechanism for
controlling who can attach, detach or list the devices.

So, we got the idea that in order to enforce that remote usbip devices
are only visible inside the container where they were imported, we
could conveniently mount-bind inside every container just one of the
vhci_hcd directories below /sys/devices/platform. So that it is as if
every container had a vhci_hcd just for itself (and also, we restrict
access to the matching USB ports in cgroups).

Unfortunately, all the vhci_hcd.* devices are controlled through
attributes in vhci_hcd.0 making our approach fail and so... well, that
is what this patch series changes. It makes every vhci_hcd device
controllable through attributes inside its own sysfs directory.

The first patch, does that in the kernel, and the second and third
patches change user space, adapting the libusbip and the usbip tools
code respectively.

Then there is a fourth patch, that allows to create much more USB
hubs per machine. It was limited to 64 and we are running thousands of
containers (every one requiring a hub) per host.

These changes are not completely backward compatible. In the sysfs
side, besides moving around the attribute files, now the port numbers
go from 0 to CONFIG_USBIP_VHCI_HC_PORTS * 2 - 1 and are reused for
every vhci_hcd device. I could have maintained the absolute numeration
but I think reusing the numbers is a better and simpler approach.

I have considered that until very recently, support for vhci_hcd
devices above the .0 was broken in the uspip tools (see
1ac7c8a78be85f84b019d3d2742d1a9f07255cc5 "usbip: fix usbip attach to
find a port that matches the requested speed") and that has been the
place where I have set the bar for backward compatibility: usage of
the tools must remain unchanged for accessing "vhci_hcd.0", but may
require changes otherwise. The same is true for the library functions
which now provides new functions for selecting the target vhci_hcd
device. The old functions now always target "vhci_hcd.0".

So, for instance, now, "usbip port" by default only shows "vhci_hcd.0"
ports but "usbip port -a" shows all of them, and, for instance, "usbip
port -i 4", shows the ports in "vhci_hcd.4".

Cheers.

[PATCH 1/4] usbip: make vhci_hcd.* objects independent of vhci_hcd.0

[Salvador Fandino]

From: Salvador Fandiño <salva@qindel.com>

The usbip VHCI layer supports multiple "vhci_hcd" devices, every one
emulating both a high speed and a super speed USB hub. These devices
are exposed in sysfs as "vhci_hcd.0", "vhci_hcd.1", etc.

But instead of controlling then by attributes inside their respective
directories, all of then were controlled through "vhci_hcd.0" where the
attributes "attach" and "detach" were used to connect and disconnect
respectively remote USB devices to any of the virtual hub ports. Also
port status was show but a series of "status.X" attributes, all inside
"vhci_hcd.0".

Besides the rather unusuality of this approach, it precludes users
from doing interesting things, as for instance, restricting the access
to vhci_hcd devices independently.

This patch adds "attach", "detach", "status" and "nports" attributes
inside every "vhci_hcd" object, besides "vhci_hcd.0" making then
independent.

Attribute "debug", allowing to control when debug messages for the
usbip subsystems are generated is, as before, only attached to
"vhci_hcd.0".

Signed-off-by: Salvador Fandiño <salva@qindel.com>
---
 drivers/usb/usbip/vhci.h       |  21 ++-
 drivers/usb/usbip/vhci_hcd.c   |  25 ++--
 drivers/usb/usbip/vhci_sysfs.c | 311 +++++++++++++----------------------------
 3 files changed, 132 insertions(+), 225 deletions(-)

diff --git a/drivers/usb/usbip/vhci.h b/drivers/usb/usbip/vhci.h
index 5659dce1526e..c1b848775be1 100644
--- a/drivers/usb/usbip/vhci.h
+++ b/drivers/usb/usbip/vhci.h
@@ -88,7 +88,14 @@ enum hub_speed {
 #define VHCI_NR_HCS 1
 #endif
 
-#define MAX_STATUS_NAME 16
+struct vhci_attrs {
+	struct dev_ext_attribute dev_attr_status;
+	struct dev_ext_attribute dev_attr_attach;
+	struct dev_ext_attribute dev_attr_detach;
+	struct dev_ext_attribute dev_attr_nports;
+
+	struct attribute_group attribute_group;
+};
 
 struct vhci {
 	spinlock_t lock;
@@ -97,6 +104,8 @@ struct vhci {
 
 	struct vhci_hcd *vhci_hcd_hs;
 	struct vhci_hcd *vhci_hcd_ss;
+
+	struct vhci_attrs *attrs;
 };
 
 /* for usb_hcd.hcd_priv[0] */
@@ -126,8 +135,8 @@ extern struct attribute_group vhci_attr_group;
 void rh_port_connect(struct vhci_device *vdev, enum usb_device_speed speed);
 
 /* vhci_sysfs.c */
-int vhci_init_attr_group(void);
-void vhci_finish_attr_group(void);
+int vhci_init_attr_group(struct vhci_hcd *vhci_hcd, int id);
+void vhci_finish_attr_group(struct vhci_hcd *vhci_hcd);
 
 /* vhci_rx.c */
 struct urb *pickup_urb_and_free_priv(struct vhci_device *vdev, __u32 seqnum);
@@ -171,4 +180,10 @@ static inline struct vhci_hcd *vdev_to_vhci_hcd(struct vhci_device *vdev)
 	return container_of((void *)(vdev - vdev->rhport), struct vhci_hcd, vdev);
 }
 
+static inline struct vhci *device_attribute_to_vhci(
+	struct device_attribute *attr)
+{
+	return (struct vhci *)((struct dev_ext_attribute *)attr)->var;
+}
+
 #endif /* __USBIP_VHCI_H */
diff --git a/drivers/usb/usbip/vhci_hcd.c b/drivers/usb/usbip/vhci_hcd.c
index c3e1008aa491..daabb06c9f46 100644
--- a/drivers/usb/usbip/vhci_hcd.c
+++ b/drivers/usb/usbip/vhci_hcd.c
@@ -1110,13 +1110,14 @@ static int vhci_setup(struct usb_hcd *hcd)
 static int vhci_start(struct usb_hcd *hcd)
 {
 	struct vhci_hcd *vhci_hcd = hcd_to_vhci_hcd(hcd);
+	struct vhci *vhci = vhci_hcd->vhci;
 	int id, rhport;
 	int err;
 
-	usbip_dbg_vhci_hc("enter vhci_start\n");
+	usbip_dbg_vhci_hc("enter vhci_start for %s\n", hcd_name(hcd));
 
 	if (usb_hcd_is_primary_hcd(hcd))
-		spin_lock_init(&vhci_hcd->vhci->lock);
+		spin_lock_init(&vhci->lock);
 
 	/* initialize private data of usb_hcd */
 
@@ -1143,16 +1144,17 @@ static int vhci_start(struct usb_hcd *hcd)
 	}
 
 	/* vhci_hcd is now ready to be controlled through sysfs */
-	if (id == 0 && usb_hcd_is_primary_hcd(hcd)) {
-		err = vhci_init_attr_group();
+	if (usb_hcd_is_primary_hcd(hcd)) {
+		err = vhci_init_attr_group(vhci_hcd, id);
 		if (err) {
 			pr_err("init attr group\n");
 			return err;
 		}
-		err = sysfs_create_group(&hcd_dev(hcd)->kobj, &vhci_attr_group);
+		err = sysfs_create_group(&hcd_dev(hcd)->kobj,
+					 &vhci->attrs->attribute_group);
 		if (err) {
 			pr_err("create sysfs files\n");
-			vhci_finish_attr_group();
+			vhci_finish_attr_group(vhci_hcd);
 			return err;
 		}
 		pr_info("created sysfs %s\n", hcd_name(hcd));
@@ -1164,15 +1166,16 @@ static int vhci_start(struct usb_hcd *hcd)
 static void vhci_stop(struct usb_hcd *hcd)
 {
 	struct vhci_hcd *vhci_hcd = hcd_to_vhci_hcd(hcd);
-	int id, rhport;
+	struct vhci *vhci = vhci_hcd->vhci;
+	int rhport;
 
 	usbip_dbg_vhci_hc("stop VHCI controller\n");
 
 	/* 1. remove the userland interface of vhci_hcd */
-	id = hcd_name_to_id(hcd_name(hcd));
-	if (id == 0 && usb_hcd_is_primary_hcd(hcd)) {
-		sysfs_remove_group(&hcd_dev(hcd)->kobj, &vhci_attr_group);
-		vhci_finish_attr_group();
+	if (usb_hcd_is_primary_hcd(hcd)) {
+		sysfs_remove_group(&hcd_dev(hcd)->kobj,
+				   &vhci->attrs->attribute_group);
+		vhci_finish_attr_group(vhci_hcd);
 	}
 
 	/* 2. shutdown all the ports of vhci_hcd */
diff --git a/drivers/usb/usbip/vhci_sysfs.c b/drivers/usb/usbip/vhci_sysfs.c
index 091f76b7196d..9e296ee9b9cb 100644
--- a/drivers/usb/usbip/vhci_sysfs.c
+++ b/drivers/usb/usbip/vhci_sysfs.c
@@ -57,24 +57,14 @@ static void port_show_vhci(char **out, int hub, int port, struct vhci_device *vd
 }
 
 /* Sysfs entry to show port status */
-static ssize_t status_show_vhci(int pdev_nr, char *out)
+static ssize_t status_show_vhci(struct vhci *vhci, char *out)
 {
-	struct platform_device *pdev = vhcis[pdev_nr].pdev;
-	struct vhci *vhci;
-	struct usb_hcd *hcd;
-	struct vhci_hcd *vhci_hcd;
 	char *s = out;
 	int i;
 	unsigned long flags;
 
-	if (!pdev || !out) {
-		usbip_dbg_vhci_sysfs("show status error\n");
+	if (WARN_ON(!vhci) || WARN_ON(!out))
 		return 0;
-	}
-
-	hcd = platform_get_drvdata(pdev);
-	vhci_hcd = hcd_to_vhci_hcd(hcd);
-	vhci = vhci_hcd->vhci;
 
 	spin_lock_irqsave(&vhci->lock, flags);
 
@@ -83,7 +73,7 @@ static ssize_t status_show_vhci(int pdev_nr, char *out)
 
 		spin_lock(&vdev->ud.lock);
 		port_show_vhci(&out, HUB_SPEED_HIGH,
-			       pdev_nr * VHCI_PORTS + i, vdev);
+			       i, vdev);
 		spin_unlock(&vdev->ud.lock);
 	}
 
@@ -92,7 +82,7 @@ static ssize_t status_show_vhci(int pdev_nr, char *out)
 
 		spin_lock(&vdev->ud.lock);
 		port_show_vhci(&out, HUB_SPEED_SUPER,
-			       pdev_nr * VHCI_PORTS + VHCI_HC_PORTS + i, vdev);
+			       VHCI_HC_PORTS + i, vdev);
 		spin_unlock(&vdev->ud.lock);
 	}
 
@@ -101,77 +91,21 @@ static ssize_t status_show_vhci(int pdev_nr, char *out)
 	return out - s;
 }
 
-static ssize_t status_show_not_ready(int pdev_nr, char *out)
-{
-	char *s = out;
-	int i = 0;
-
-	for (i = 0; i < VHCI_HC_PORTS; i++) {
-		out += sprintf(out, "hs  %04u %03u ",
-				    (pdev_nr * VHCI_PORTS) + i,
-				    VDEV_ST_NOTASSIGNED);
-		out += sprintf(out, "000 00000000 0000000000000000 0-0");
-		out += sprintf(out, "\n");
-	}
-
-	for (i = 0; i < VHCI_HC_PORTS; i++) {
-		out += sprintf(out, "ss  %04u %03u ",
-				    (pdev_nr * VHCI_PORTS) + VHCI_HC_PORTS + i,
-				    VDEV_ST_NOTASSIGNED);
-		out += sprintf(out, "000 00000000 0000000000000000 0-0");
-		out += sprintf(out, "\n");
-	}
-	return out - s;
-}
-
-static int status_name_to_id(const char *name)
-{
-	char *c;
-	long val;
-	int ret;
-
-	c = strchr(name, '.');
-	if (c == NULL)
-		return 0;
-
-	ret = kstrtol(c+1, 10, &val);
-	if (ret < 0)
-		return ret;
-
-	return val;
-}
-
 static ssize_t status_show(struct device *dev,
 			   struct device_attribute *attr, char *out)
 {
 	char *s = out;
-	int pdev_nr;
-
 	out += sprintf(out,
 		       "hub port sta spd dev      socket           local_busid\n");
-
-	pdev_nr = status_name_to_id(attr->attr.name);
-	if (pdev_nr < 0)
-		out += status_show_not_ready(pdev_nr, out);
-	else
-		out += status_show_vhci(pdev_nr, out);
-
+	out += status_show_vhci(device_attribute_to_vhci(attr), out);
 	return out - s;
 }
 
 static ssize_t nports_show(struct device *dev, struct device_attribute *attr,
 			   char *out)
 {
-	char *s = out;
-
-	/*
-	 * Half the ports are for SPEED_HIGH and half for SPEED_SUPER,
-	 * thus the * 2.
-	 */
-	out += sprintf(out, "%d\n", VHCI_PORTS * vhci_num_controllers);
-	return out - s;
+	return sprintf(out, "%d\n", VHCI_PORTS);
 }
-static DEVICE_ATTR_RO(nports);
 
 /* Sysfs entry to shutdown a virtual connection */
 static int vhci_port_disconnect(struct vhci_hcd *vhci_hcd, __u32 rhport)
@@ -205,14 +139,11 @@ static int vhci_port_disconnect(struct vhci_hcd *vhci_hcd, __u32 rhport)
 	return 0;
 }
 
-static int valid_port(__u32 pdev_nr, __u32 rhport)
+static int validate_port_in_range(__u32 port, __u32 base, __u32 top)
 {
-	if (pdev_nr >= vhci_num_controllers) {
-		pr_err("pdev %u\n", pdev_nr);
-		return 0;
-	}
-	if (rhport >= VHCI_HC_PORTS) {
-		pr_err("rhport %u\n", rhport);
+	if (port < base || port >= top) {
+		pr_err("Port number %u outside of range [%u-%u]\n",
+		       port, base, top - 1);
 		return 0;
 	}
 	return 1;
@@ -221,34 +152,24 @@ static int valid_port(__u32 pdev_nr, __u32 rhport)
 static ssize_t store_detach(struct device *dev, struct device_attribute *attr,
 			    const char *buf, size_t count)
 {
-	__u32 port = 0, pdev_nr = 0, rhport = 0;
-	struct usb_hcd *hcd;
-	struct vhci_hcd *vhci_hcd;
+	struct vhci *vhci = device_attribute_to_vhci(attr);
+	__u32 port = 0;
 	int ret;
 
 	if (kstrtoint(buf, 10, &port) < 0)
 		return -EINVAL;
 
-	pdev_nr = port_to_pdev_nr(port);
-	rhport = port_to_rhport(port);
+	usbip_dbg_vhci_sysfs("%s: detach port %d\n", dev_name(dev), port);
 
-	if (!valid_port(pdev_nr, rhport))
+	if (!validate_port_in_range(port, 0, VHCI_PORTS))
 		return -EINVAL;
 
-	hcd = platform_get_drvdata(vhcis[pdev_nr].pdev);
-	if (hcd == NULL) {
-		dev_err(dev, "port is not ready %u\n", port);
-		return -EAGAIN;
-	}
-
-	usbip_dbg_vhci_sysfs("rhport %d\n", rhport);
-
-	if ((port / VHCI_HC_PORTS) % 2)
-		vhci_hcd = hcd_to_vhci_hcd(hcd)->vhci->vhci_hcd_ss;
+	if (port >= VHCI_HC_PORTS)
+		ret = vhci_port_disconnect(vhci->vhci_hcd_ss,
+					   port - VHCI_HC_PORTS);
 	else
-		vhci_hcd = hcd_to_vhci_hcd(hcd)->vhci->vhci_hcd_hs;
+		ret = vhci_port_disconnect(vhci->vhci_hcd_hs, port);
 
-	ret = vhci_port_disconnect(vhci_hcd, rhport);
 	if (ret < 0)
 		return -EINVAL;
 
@@ -256,29 +177,6 @@ static ssize_t store_detach(struct device *dev, struct device_attribute *attr,
 
 	return count;
 }
-static DEVICE_ATTR(detach, S_IWUSR, NULL, store_detach);
-
-static int valid_args(__u32 pdev_nr, __u32 rhport, enum usb_device_speed speed)
-{
-	if (!valid_port(pdev_nr, rhport)) {
-		return 0;
-	}
-
-	switch (speed) {
-	case USB_SPEED_LOW:
-	case USB_SPEED_FULL:
-	case USB_SPEED_HIGH:
-	case USB_SPEED_WIRELESS:
-	case USB_SPEED_SUPER:
-		break;
-	default:
-		pr_err("Failed attach request for unsupported USB speed: %s\n",
-			usb_speed_string(speed));
-		return 0;
-	}
-
-	return 1;
-}
 
 /* Sysfs entry to establish a virtual connection */
 /*
@@ -295,50 +193,47 @@ static int valid_args(__u32 pdev_nr, __u32 rhport, enum usb_device_speed speed)
 static ssize_t store_attach(struct device *dev, struct device_attribute *attr,
 			    const char *buf, size_t count)
 {
+	struct vhci *vhci = device_attribute_to_vhci(attr);
 	struct socket *socket;
 	int sockfd = 0;
-	__u32 port = 0, pdev_nr = 0, rhport = 0, devid = 0, speed = 0;
-	struct usb_hcd *hcd;
-	struct vhci_hcd *vhci_hcd;
+	__u32 port = 0, devid = 0, speed = 0;
 	struct vhci_device *vdev;
-	struct vhci *vhci;
 	int err;
 	unsigned long flags;
 
 	/*
-	 * @rhport: port number of vhci_hcd
+	 * @port: port number of vhci_hcd
 	 * @sockfd: socket descriptor of an established TCP connection
 	 * @devid: unique device identifier in a remote host
 	 * @speed: usb device speed in a remote host
 	 */
 	if (sscanf(buf, "%u %u %u %u", &port, &sockfd, &devid, &speed) != 4)
 		return -EINVAL;
-	pdev_nr = port_to_pdev_nr(port);
-	rhport = port_to_rhport(port);
 
-	usbip_dbg_vhci_sysfs("port(%u) pdev(%d) rhport(%u)\n",
-			     port, pdev_nr, rhport);
-	usbip_dbg_vhci_sysfs("sockfd(%u) devid(%u) speed(%u)\n",
-			     sockfd, devid, speed);
+	usbip_dbg_vhci_sysfs("%s: attach port(%u) sockfd(%u) devid(%u) speed(%u)\n",
+			     dev_name(dev), port, sockfd, devid, speed);
 
 	/* check received parameters */
-	if (!valid_args(pdev_nr, rhport, speed))
+	switch (speed) {
+	case USB_SPEED_LOW:
+	case USB_SPEED_FULL:
+	case USB_SPEED_HIGH:
+	case USB_SPEED_WIRELESS:
+		if (!validate_port_in_range(port, 0, VHCI_HC_PORTS))
+			return -EINVAL;
+		vdev = &vhci->vhci_hcd_hs->vdev[port];
+		break;
+	case USB_SPEED_SUPER:
+		if (!validate_port_in_range(port, VHCI_HC_PORTS, VHCI_PORTS))
+			return -EINVAL;
+		vdev = &vhci->vhci_hcd_ss->vdev[port - VHCI_HC_PORTS];
+		break;
+	default:
+		pr_err("Failed attach request for unsupported USB speed: %s\n",
+		       usb_speed_string(speed));
 		return -EINVAL;
-
-	hcd = platform_get_drvdata(vhcis[pdev_nr].pdev);
-	if (hcd == NULL) {
-		dev_err(dev, "port %d is not ready\n", port);
-		return -EAGAIN;
 	}
 
-	vhci_hcd = hcd_to_vhci_hcd(hcd);
-	vhci = vhci_hcd->vhci;
-
-	if (speed == USB_SPEED_SUPER)
-		vdev = &vhci->vhci_hcd_ss->vdev[rhport];
-	else
-		vdev = &vhci->vhci_hcd_hs->vdev[rhport];
-
 	/* Extract socket from fd. */
 	socket = sockfd_lookup(sockfd, &err);
 	if (!socket)
@@ -357,7 +252,7 @@ static ssize_t store_attach(struct device *dev, struct device_attribute *attr,
 
 		sockfd_put(socket);
 
-		dev_err(dev, "port %d already used\n", rhport);
+		dev_err(dev, "port %u already used\n", port);
 		/*
 		 * Will be retried from userspace
 		 * if there's another free port.
@@ -365,8 +260,8 @@ static ssize_t store_attach(struct device *dev, struct device_attribute *attr,
 		return -EBUSY;
 	}
 
-	dev_info(dev, "pdev(%u) rhport(%u) sockfd(%d)\n",
-		 pdev_nr, rhport, sockfd);
+	dev_info(dev, "port(%u) sockfd(%d)\n",
+		 port, sockfd);
 	dev_info(dev, "devid(%u) speed(%u) speed_str(%s)\n",
 		 devid, speed, usb_speed_string(speed));
 
@@ -387,83 +282,77 @@ static ssize_t store_attach(struct device *dev, struct device_attribute *attr,
 
 	return count;
 }
-static DEVICE_ATTR(attach, S_IWUSR, NULL, store_attach);
 
-#define MAX_STATUS_NAME 16
-
-struct status_attr {
-	struct device_attribute attr;
-	char name[MAX_STATUS_NAME+1];
-};
-
-static struct status_attr *status_attrs;
-
-static void set_status_attr(int id)
+int vhci_init_attr_group(struct vhci_hcd *vhci_hcd, int id)
 {
-	struct status_attr *status;
+	struct vhci_attrs *vhci_attrs;
+	struct attribute **attrs;
+	struct vhci *vhci = vhci_hcd->vhci;
+	int nattrs = ((id == 0) ? 5 : 4);
 
-	status = status_attrs + id;
-	if (id == 0)
-		strcpy(status->name, "status");
-	else
-		snprintf(status->name, MAX_STATUS_NAME+1, "status.%d", id);
-	status->attr.attr.name = status->name;
-	status->attr.attr.mode = S_IRUGO;
-	status->attr.show = status_show;
-	sysfs_attr_init(&status->attr.attr);
-}
+	if (WARN_ON(vhci->attrs != NULL))
+		return -EADDRINUSE;
 
-static int init_status_attrs(void)
-{
-	int id;
+	vhci_attrs = kcalloc(1, sizeof(*vhci_attrs), GFP_KERNEL);
+	if (vhci_attrs == NULL)
+		return -ENOMEM;
 
-	status_attrs = kcalloc(vhci_num_controllers, sizeof(struct status_attr),
-			       GFP_KERNEL);
-	if (status_attrs == NULL)
+	attrs = kmalloc_array(nattrs + 1, sizeof(*attrs), GFP_KERNEL);
+	if (attrs == NULL) {
+		kfree(vhci_attrs);
 		return -ENOMEM;
+	}
 
-	for (id = 0; id < vhci_num_controllers; id++)
-		set_status_attr(id);
+	vhci->attrs = vhci_attrs;
+	vhci_attrs->attribute_group.attrs = attrs;
+
+	vhci_attrs->dev_attr_status.attr.attr.name = "status";
+	vhci_attrs->dev_attr_status.attr.attr.mode = 0444;
+	vhci_attrs->dev_attr_status.attr.show = status_show;
+	vhci_attrs->dev_attr_status.var = vhci;
+	sysfs_attr_init(&vhci_attrs->dev_attr_status.attr.attr);
+	attrs[0] = &vhci_attrs->dev_attr_status.attr.attr;
+
+	vhci_attrs->dev_attr_attach.attr.attr.name = "attach";
+	vhci_attrs->dev_attr_attach.attr.attr.mode = 0200;
+	vhci_attrs->dev_attr_attach.attr.store = store_attach;
+	vhci_attrs->dev_attr_attach.var = vhci;
+	sysfs_attr_init(&vhci_attrs->dev_attr_attach.attr.attr);
+	attrs[1] = &vhci_attrs->dev_attr_attach.attr.attr;
+
+	vhci_attrs->dev_attr_detach.attr.attr.name = "detach";
+	vhci_attrs->dev_attr_detach.attr.attr.mode = 0200;
+	vhci_attrs->dev_attr_detach.attr.store = store_detach;
+	vhci_attrs->dev_attr_detach.var = vhci;
+	sysfs_attr_init(&vhci_attrs->dev_attr_detach.attr.attr);
+	attrs[2] = &vhci_attrs->dev_attr_detach.attr.attr;
+
+	vhci_attrs->dev_attr_nports.attr.attr.name = "nports";
+	vhci_attrs->dev_attr_nports.attr.attr.mode = 0444;
+	vhci_attrs->dev_attr_nports.attr.show = nports_show;
+	vhci_attrs->dev_attr_nports.var = vhci;
+	sysfs_attr_init(&vhci_attrs->dev_attr_nports.attr.attr);
+	attrs[3] = &vhci_attrs->dev_attr_nports.attr.attr;
+
+	if (id == 0) {
+		attrs[4] = &dev_attr_usbip_debug.attr;
+		attrs[5] = NULL;
+	} else {
+		attrs[4] = NULL;
+	}
 
 	return 0;
 }
 
-static void finish_status_attrs(void)
+void vhci_finish_attr_group(struct vhci_hcd *vhci_hcd)
 {
-	kfree(status_attrs);
-}
-
-struct attribute_group vhci_attr_group = {
-	.attrs = NULL,
-};
+	struct vhci_attrs *vhci_attrs = vhci_hcd->vhci->attrs;
 
-int vhci_init_attr_group(void)
-{
-	struct attribute **attrs;
-	int ret, i;
+	if (vhci_attrs) {
+		struct attribute **attrs = vhci_attrs->attribute_group.attrs;
 
-	attrs = kcalloc((vhci_num_controllers + 5), sizeof(struct attribute *),
-			GFP_KERNEL);
-	if (attrs == NULL)
-		return -ENOMEM;
-
-	ret = init_status_attrs();
-	if (ret) {
 		kfree(attrs);
-		return ret;
+		kfree(vhci_attrs);
+		vhci_hcd->vhci->attrs = NULL;
 	}
-	*attrs = &dev_attr_nports.attr;
-	*(attrs + 1) = &dev_attr_detach.attr;
-	*(attrs + 2) = &dev_attr_attach.attr;
-	*(attrs + 3) = &dev_attr_usbip_debug.attr;
-	for (i = 0; i < vhci_num_controllers; i++)
-		*(attrs + i + 4) = &((status_attrs + i)->attr.attr);
-	vhci_attr_group.attrs = attrs;
-	return 0;
-}
-
-void vhci_finish_attr_group(void)
-{
-	finish_status_attrs();
-	kfree(vhci_attr_group.attrs);
 }
-- 
2.14.1

[PATCH 2/4] libusbip: use per vhci_hcd controller attributes

[Salvador Fandino]

From: Salvador Fandiño <salva@qindel.com>

Now every vhci_hcd device is controlled by its own sysfs attributes
(before all of them were controlled by vhci_hcd.0 attributes). This
patch addapts libusbip to this new interface.

Before this patch the library did not provide any mean to access a
specific vhci_hcd device. In order to keep backward compatibility
while at the same time allowing to access vhci_hcd devices
independently the following approach has been taken:

New functions accepting a vhci_hcd path or an index have been added
("usbip_vhci_driver_open_path" and "usbip_vhci_driver_open_ix"
respectively). The old function "usbip_vhci_driver_open" can only be
used to open "vhci_hcd.0"). That will make old programs fail to use
any other vhci_hcd device, but as that functionality was broken in the
kernel anyway until very recently (see
1ac7c8a78be85f84b019d3d2742d1a9f07255cc5) we consider it a good
compromise.

Signed-off-by: Salvador Fandiño <salva@qindel.com>
---
 tools/usb/usbip/libsrc/vhci_driver.c | 105 +++++++++++++++++++++--------------
 tools/usb/usbip/libsrc/vhci_driver.h |  12 +++-
 2 files changed, 74 insertions(+), 43 deletions(-)

diff --git a/tools/usb/usbip/libsrc/vhci_driver.c b/tools/usb/usbip/libsrc/vhci_driver.c
index c9c81614a66a..34d15167bab3 100644
--- a/tools/usb/usbip/libsrc/vhci_driver.c
+++ b/tools/usb/usbip/libsrc/vhci_driver.c
@@ -114,13 +114,16 @@ static int refresh_imported_device_list(void)
 	char status[MAX_STATUS_NAME+1] = "status";
 	int i, ret;
 
-	for (i = 0; i < vhci_driver->ncontrollers; i++) {
+	for (i = 0; ; i++) {
 		if (i > 0)
 			snprintf(status, sizeof(status), "status.%d", i);
 
 		attr_status = udev_device_get_sysattr_value(vhci_driver->hc_device,
 							    status);
 		if (!attr_status) {
+			if (i > 0)
+				break;
+
 			err("udev_device_get_sysattr_value failed");
 			return -1;
 		}
@@ -148,33 +151,6 @@ static int get_nports(void)
 	return (int)strtoul(attr_nports, NULL, 10);
 }
 
-static int vhci_hcd_filter(const struct dirent *dirent)
-{
-	return strcmp(dirent->d_name, "vhci_hcd") >= 0;
-}
-
-static int get_ncontrollers(void)
-{
-	struct dirent **namelist;
-	struct udev_device *platform;
-	int n;
-
-	platform = udev_device_get_parent(vhci_driver->hc_device);
-	if (platform == NULL)
-		return -1;
-
-	n = scandir(udev_device_get_syspath(platform), &namelist, vhci_hcd_filter, NULL);
-	if (n < 0)
-		err("scandir failed");
-	else {
-		for (int i = 0; i < n; i++)
-			free(namelist[i]);
-		free(namelist);
-	}
-
-	return n;
-}
-
 /*
  * Read the given port's record.
  *
@@ -199,7 +175,8 @@ static int read_record(int rhport, char *host, unsigned long host_len,
 	if (!buffer)
 		return -1;
 
-	snprintf(path, PATH_MAX, VHCI_STATE_PATH"/port%d", rhport);
+	snprintf(path, PATH_MAX, VHCI_STATE_PATH"/port%d-%d",
+		 vhci_driver->ix, rhport);
 
 	file = fopen(path, "r");
 	if (!file) {
@@ -238,9 +215,18 @@ static int read_record(int rhport, char *host, unsigned long host_len,
 	return 0;
 }
 
+static int vhci_path_to_ix(const char *path)
+{
+	int i = strlen(path);
+
+	while (--i >= 0 && isdigit(path[i]))
+		;
+	return atoi(path + i + 1);
+}
+
 /* ---------------------------------------------------------------------- */
 
-int usbip_vhci_driver_open(void)
+int usbip_vhci_driver_open_path(const char *path)
 {
 	udev_context = udev_new();
 	if (!udev_context) {
@@ -252,14 +238,15 @@ int usbip_vhci_driver_open(void)
 
 	/* will be freed in usbip_driver_close() */
 	vhci_driver->hc_device =
-		udev_device_new_from_subsystem_sysname(udev_context,
-						       USBIP_VHCI_BUS_TYPE,
-						       USBIP_VHCI_DEVICE_NAME);
+		udev_device_new_from_syspath(udev_context, path);
+
 	if (!vhci_driver->hc_device) {
 		err("udev_device_new_from_subsystem_sysname failed");
 		goto err;
 	}
 
+	vhci_driver->ix = vhci_path_to_ix(path);
+
 	vhci_driver->nports = get_nports();
 	dbg("available ports: %d", vhci_driver->nports);
 
@@ -271,14 +258,6 @@ int usbip_vhci_driver_open(void)
 		goto err;
 	}
 
-	vhci_driver->ncontrollers = get_ncontrollers();
-	dbg("available controllers: %d", vhci_driver->ncontrollers);
-
-	if (vhci_driver->ncontrollers <=0) {
-		err("no available usb controllers");
-		goto err;
-	}
-
 	if (refresh_imported_device_list())
 		goto err;
 
@@ -297,6 +276,45 @@ int usbip_vhci_driver_open(void)
 	return -1;
 }
 
+int usbip_vhci_driver_open(void)
+{
+	return usbip_vhci_driver_open_ix(0);
+}
+
+int usbip_vhci_driver_open_ix(int vhci_ix)
+{
+	struct udev *udev_context;
+	struct udev_device *dev = NULL;
+	char vhci_name[PATH_MAX + 1];
+	int len, rc = -1;
+
+	len = snprintf(vhci_name, PATH_MAX,
+		       "%s.%d", USBIP_VHCI_DEVICE_NAME_PREFIX, vhci_ix);
+	if (len >= PATH_MAX) {
+		err("vhci device name is too long");
+		return -1;
+	}
+
+	udev_context = udev_new();
+	if (!udev_context) {
+		err("udev_new failed");
+		return -1;
+	}
+
+	dev = udev_device_new_from_subsystem_sysname(udev_context,
+						     USBIP_VHCI_BUS_TYPE,
+						     vhci_name);
+	if (!dev) {
+		err("udev_device_from_subsystem_sysname failed to open %s",
+		    vhci_name);
+	} else {
+		rc = usbip_vhci_driver_open_path(udev_device_get_syspath(dev));
+		udev_device_unref(dev);
+	}
+
+	udev_unref(udev_context);
+	return rc;
+}
 
 void usbip_vhci_driver_close(void)
 {
@@ -459,3 +477,8 @@ int usbip_vhci_imported_device_dump(struct usbip_imported_device *idev)
 
 	return 0;
 }
+
+int usbip_vhci_driver_ix(void)
+{
+	return vhci_driver->ix;
+}
diff --git a/tools/usb/usbip/libsrc/vhci_driver.h b/tools/usb/usbip/libsrc/vhci_driver.h
index 418b404d5121..40537f4f8917 100644
--- a/tools/usb/usbip/libsrc/vhci_driver.h
+++ b/tools/usb/usbip/libsrc/vhci_driver.h
@@ -12,7 +12,9 @@
 #include "usbip_common.h"
 
 #define USBIP_VHCI_BUS_TYPE "platform"
-#define USBIP_VHCI_DEVICE_NAME "vhci_hcd.0"
+#define USBIP_VHCI_DEVICE_NAME_PREFIX "vhci_hcd"
+#define USBIP_VHCI_DEVICE_NAME_0 (USBIP_VHCI_DEVICE_NAME_PREFIX ".0")
+#define USBIP_VHCI_DEVICE_NAME_PATTERN (USBIP_VHCI_DEVICE_NAME_PREFIX ".*")
 #define MAXNPORT 128
 
 enum hub_speed {
@@ -39,7 +41,7 @@ struct usbip_vhci_driver {
 	/* /sys/devices/platform/vhci_hcd */
 	struct udev_device *hc_device;
 
-	int ncontrollers;
+	int ix;
 	int nports;
 	struct usbip_imported_device idev[MAXNPORT];
 };
@@ -47,7 +49,11 @@ struct usbip_vhci_driver {
 
 extern struct usbip_vhci_driver *vhci_driver;
 
+/* will be removed */
 int usbip_vhci_driver_open(void);
+
+int usbip_vhci_driver_open_path(const char *);
+int usbip_vhci_driver_open_ix(int vhci_ix);
 void usbip_vhci_driver_close(void);
 
 int  usbip_vhci_refresh_device_list(void);
@@ -65,4 +71,6 @@ int usbip_vhci_detach_device(uint8_t port);
 
 int usbip_vhci_imported_device_dump(struct usbip_imported_device *idev);
 
+int usbip_vhci_driver_ix(void);
+
 #endif /* __VHCI_DRIVER_H */
-- 
2.14.1

[PATCH 3/4] usbip tools: use per vhci_hcd controller attributes

[Salvador Fandino]

From: Salvador Fandiño <salva@qindel.com>

Now every vhci_hcd device is controlled by its own sysfs attributes
(before all of them were controlled by vhci_hcd.0 attributes). This
patch addapts the usbip tools to use the new interface through
libusbip.

The user visible changes are as follows:

- The files containing port information are named as "port$ix-$port"
(they were "port$port" before), as port numbers are local to the
vhci_hcd device now.

- Command "usbip port" by default only list ports attached to
"vhci_hcd.0", the new flag "--vhci-ix" can be used to list other
device. The flag "--all" is also available for listing all the devices
(it has not been made the default because it inserts per vhci_hcd
device headers, changing the output format).

Signed-off-by: Salvador Fandiño <salva@qindel.com>
---
 tools/usb/usbip/src/Makefile.am       |   3 +-
 tools/usb/usbip/src/usbip_attach.c    | 101 ++++++++++++++++--------
 tools/usb/usbip/src/usbip_detach.c    |  57 ++++++++------
 tools/usb/usbip/src/usbip_enumerate.c |  55 +++++++++++++
 tools/usb/usbip/src/usbip_enumerate.h |  25 ++++++
 tools/usb/usbip/src/usbip_port.c      | 143 +++++++++++++++++++++++++++++-----
 tools/usb/usbip/src/utils.c           |  16 ++++
 tools/usb/usbip/src/utils.h           |   1 +
 8 files changed, 322 insertions(+), 79 deletions(-)
 create mode 100644 tools/usb/usbip/src/usbip_enumerate.c
 create mode 100644 tools/usb/usbip/src/usbip_enumerate.h

diff --git a/tools/usb/usbip/src/Makefile.am b/tools/usb/usbip/src/Makefile.am
index e26f39e0579d..50436aaa3900 100644
--- a/tools/usb/usbip/src/Makefile.am
+++ b/tools/usb/usbip/src/Makefile.am
@@ -7,6 +7,7 @@ sbin_PROGRAMS := usbip usbipd
 
 usbip_SOURCES := usbip.h utils.h usbip.c utils.c usbip_network.c \
 		 usbip_attach.c usbip_detach.c usbip_list.c \
-		 usbip_bind.c usbip_unbind.c usbip_port.c
+		 usbip_bind.c usbip_unbind.c usbip_port.c \
+		 usbip_enumerate.c
 
 usbipd_SOURCES := usbip_network.h usbipd.c usbip_network.c
diff --git a/tools/usb/usbip/src/usbip_attach.c b/tools/usb/usbip/src/usbip_attach.c
index 7f07b2d50f59..3901dd507451 100644
--- a/tools/usb/usbip/src/usbip_attach.c
+++ b/tools/usb/usbip/src/usbip_attach.c
@@ -35,6 +35,7 @@
 #include "usbip_common.h"
 #include "usbip_network.h"
 #include "usbip.h"
+#include "usbip_enumerate.h"
 
 static const char usbip_attach_usage_string[] =
 	"usbip attach <args>\n"
@@ -48,7 +49,8 @@ void usbip_attach_usage(void)
 }
 
 #define MAX_BUFF 100
-static int record_connection(char *host, char *port, char *busid, int rhport)
+static int record_connection(char *host, char *port,
+			     char *busid, int vhci_ix, int rhport)
 {
 	int fd;
 	char path[PATH_MAX+1];
@@ -70,7 +72,7 @@ static int record_connection(char *host, char *port, char *busid, int rhport)
 			return -1;
 	}
 
-	snprintf(path, PATH_MAX, VHCI_STATE_PATH"/port%d", rhport);
+	snprintf(path, PATH_MAX, VHCI_STATE_PATH"/port%d-%d", vhci_ix, rhport);
 
 	fd = open(path, O_WRONLY|O_CREAT|O_TRUNC, S_IRWXU);
 	if (fd < 0)
@@ -90,46 +92,78 @@ static int record_connection(char *host, char *port, char *busid, int rhport)
 	return 0;
 }
 
-static int import_device(int sockfd, struct usbip_usb_device *udev)
+static int import_device(int sockfd, struct usbip_usb_device *udev,
+			 int *pvhci_ix, int *pport)
 {
-	int rc;
-	int port;
 	uint32_t speed = udev->speed;
+	int rc = -1;
 
-	rc = usbip_vhci_driver_open();
-	if (rc < 0) {
-		err("open vhci_driver");
-		goto err_out;
+	struct udev_enumerate *enumerate;
+	struct udev_list_entry *list, *entry;
+
+	enumerate = vhci_enumerate();
+	if (!enumerate) {
+		err("unable to list vhci_hcd drivers");
+		return -1;
+	}
+
+	list = udev_enumerate_get_list_entry(enumerate);
+	if (!list) {
+		err("unable to list vhci_hcd drivers");
+		return -1;
 	}
 
-	do {
-		port = usbip_vhci_get_free_port(speed);
-		if (port < 0) {
-			err("no free port");
-			goto err_driver_close;
+	udev_list_entry_foreach(entry, list) {
+		const char *path = udev_list_entry_get_name(entry);
+		int port, vhci_ix;
+
+		if (usbip_vhci_driver_open_path(path) < 0)
+			continue;
+
+		vhci_ix = usbip_vhci_driver_ix();
+
+		/* Between the moment we read and parse the status
+		 * files and the one we try to attach a socket to the
+		 * port, the later one may become occupied from some
+		 * other process. In order to avoid that race
+		 * condition, we retry on EBUSY errors. On any other
+		 * error we just jump to the next vhci_hcd device
+		 */
+		while (1) {
+			port = usbip_vhci_get_free_port(speed);
+			if (port < 0)
+				break;
+
+			dbg("got free port %d at %s", port, path);
+			rc = usbip_vhci_attach_device(port, sockfd,
+						      udev->busnum,
+						      udev->devnum,
+						      speed);
+
+			if (rc >= 0 || errno != EBUSY)
+				break;
+
+			usbip_vhci_refresh_device_list();
 		}
 
-		dbg("got free port %d", port);
+		usbip_vhci_driver_close();
 
-		rc = usbip_vhci_attach_device(port, sockfd, udev->busnum,
-					      udev->devnum, udev->speed);
-		if (rc < 0 && errno != EBUSY) {
-			err("import device");
-			goto err_driver_close;
+		if (rc >= 0) {
+			*pport = port;
+			*pvhci_ix = vhci_ix;
+			goto done;
 		}
-	} while (rc < 0);
-
-	usbip_vhci_driver_close();
+	}
+	err("import device failed");
 
-	return port;
+done:
+	udev_enumerate_unref(enumerate);
 
-err_driver_close:
-	usbip_vhci_driver_close();
-err_out:
-	return -1;
+	return rc;
 }
 
-static int query_import_device(int sockfd, char *busid)
+static int query_import_device(int sockfd, char *busid,
+			       int *pvhci_ix, int *pport)
 {
 	int rc;
 	struct op_import_request request;
@@ -178,7 +212,7 @@ static int query_import_device(int sockfd, char *busid)
 	}
 
 	/* import a device */
-	return import_device(sockfd, &reply.udev);
+	return import_device(sockfd, &reply.udev, pvhci_ix, pport);
 }
 
 static int attach_device(char *host, char *busid)
@@ -186,6 +220,7 @@ static int attach_device(char *host, char *busid)
 	int sockfd;
 	int rc;
 	int rhport;
+	int vhci_ix;
 
 	sockfd = usbip_net_tcp_connect(host, usbip_port_string);
 	if (sockfd < 0) {
@@ -193,15 +228,15 @@ static int attach_device(char *host, char *busid)
 		return -1;
 	}
 
-	rhport = query_import_device(sockfd, busid);
-	if (rhport < 0) {
+	rc = query_import_device(sockfd, busid, &vhci_ix, &rhport);
+	if (rc < 0) {
 		err("query");
 		return -1;
 	}
 
 	close(sockfd);
 
-	rc = record_connection(host, usbip_port_string, busid, rhport);
+	rc = record_connection(host, usbip_port_string, busid, vhci_ix, rhport);
 	if (rc < 0) {
 		err("record connection");
 		return -1;
diff --git a/tools/usb/usbip/src/usbip_detach.c b/tools/usb/usbip/src/usbip_detach.c
index 9db9d21bb2ec..06ad88d9959b 100644
--- a/tools/usb/usbip/src/usbip_detach.c
+++ b/tools/usb/usbip/src/usbip_detach.c
@@ -30,49 +30,39 @@
 #include "usbip_common.h"
 #include "usbip_network.h"
 #include "usbip.h"
+#include "utils.h"
 
 static const char usbip_detach_usage_string[] =
 	"usbip detach <args>\n"
-	"    -p, --port=<port>    " USBIP_VHCI_DRV_NAME
-	" port the device is on\n";
+	"    -i, --vhci-ix=<ix>   index of the "
+	USBIP_VHCI_DRV_NAME
+	" the device is on (defaults to 0)\n"
+	"    -p, --port=<port>    port the device is on\n";
 
 void usbip_detach_usage(void)
 {
 	printf("usage: %s", usbip_detach_usage_string);
 }
 
-static int detach_port(char *port)
+static int detach_port(int vhci_ix, int port)
 {
 	int ret;
-	uint8_t portnum;
 	char path[PATH_MAX+1];
 
-	unsigned int port_len = strlen(port);
-
-	for (unsigned int i = 0; i < port_len; i++)
-		if (!isdigit(port[i])) {
-			err("invalid port %s", port);
-			return -1;
-		}
-
-	/* check max port */
-
-	portnum = atoi(port);
-
 	/* remove the port state file */
 
-	snprintf(path, PATH_MAX, VHCI_STATE_PATH"/port%d", portnum);
+	snprintf(path, PATH_MAX, VHCI_STATE_PATH"/port%d-%d", vhci_ix, port);
 
 	remove(path);
 	rmdir(VHCI_STATE_PATH);
 
-	ret = usbip_vhci_driver_open();
+	ret = usbip_vhci_driver_open_ix(vhci_ix);
 	if (ret < 0) {
 		err("open vhci_driver");
 		return -1;
 	}
 
-	ret = usbip_vhci_detach_device(portnum);
+	ret = usbip_vhci_detach_device(port);
 	if (ret < 0)
 		return -1;
 
@@ -85,28 +75,45 @@ int usbip_detach(int argc, char *argv[])
 {
 	static const struct option opts[] = {
 		{ "port", required_argument, NULL, 'p' },
+		{ "vhci-ix", 0, NULL, 'i' },
 		{ NULL, 0, NULL, 0 }
 	};
 	int opt;
-	int ret = -1;
+	int port = -1;
+	int vhci_ix = 0;
 
 	for (;;) {
-		opt = getopt_long(argc, argv, "p:", opts, NULL);
+		opt = getopt_long(argc, argv, "p:i:", opts, NULL);
 
 		if (opt == -1)
 			break;
 
 		switch (opt) {
 		case 'p':
-			ret = detach_port(optarg);
-			goto out;
+			if (atoi_with_check(optarg, &port) < 0) {
+				err("bad port number");
+				return -1;
+			}
+			break;
+		case 'i':
+			if (atoi_with_check(optarg, &vhci_ix) < 0) {
+				err("bad vhci index");
+				return -1;
+			}
+			break;
 		default:
 			goto err_out;
 		}
 	}
+	if (optind < argc)
+		goto err_out;
+
+	if (port < 0)
+		goto err_out;
+
+	return detach_port(vhci_ix, port);
 
 err_out:
 	usbip_detach_usage();
-out:
-	return ret;
+	return -1;
 }
diff --git a/tools/usb/usbip/src/usbip_enumerate.c b/tools/usb/usbip/src/usbip_enumerate.c
new file mode 100644
index 000000000000..b6197435b476
--- /dev/null
+++ b/tools/usb/usbip/src/usbip_enumerate.c
@@ -0,0 +1,55 @@
+/*
+ * Copyright (C) 2018 Qindel Formación y Servicios SL
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program. If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include <libudev.h>
+
+#include "usbip_enumerate.h"
+#include "vhci_driver.h"
+
+struct udev_enumerate *vhci_enumerate(void)
+{
+	struct udev *udev_context = NULL;
+	struct udev_enumerate *enumerate = NULL;
+	int rc;
+
+	udev_context = udev_new();
+	if (!udev_context) {
+		err("udev_new failed");
+		return NULL;
+	}
+
+	enumerate = udev_enumerate_new(udev_context);
+	if (!enumerate) {
+		err("udev_enumerate_new failed");
+		goto err;
+	}
+
+	udev_enumerate_add_match_subsystem(enumerate, USBIP_VHCI_BUS_TYPE);
+	udev_enumerate_add_match_sysname(enumerate,
+					 USBIP_VHCI_DEVICE_NAME_PATTERN);
+	rc = udev_enumerate_scan_devices(enumerate);
+	if (rc < 0) {
+		err("udev_enumerate_scan_devices failed: %d", rc);
+		udev_enumerate_unref(enumerate);
+		enumerate = NULL;
+	}
+
+err:
+	udev_unref(udev_context);
+
+	return enumerate;
+}
diff --git a/tools/usb/usbip/src/usbip_enumerate.h b/tools/usb/usbip/src/usbip_enumerate.h
new file mode 100644
index 000000000000..ce1f4e6941d0
--- /dev/null
+++ b/tools/usb/usbip/src/usbip_enumerate.h
@@ -0,0 +1,25 @@
+/*
+ * Copyright (C) 2018 Qindel Formacion y Servicios SL
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program. If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#ifndef __USBIP_ENUMERATE_H
+#define __USBIP_ENUMERATE_H
+
+#include <libudev.h>
+
+struct udev_enumerate *vhci_enumerate(void);
+
+#endif /* __USBIP_ENUMERATE_H */
diff --git a/tools/usb/usbip/src/usbip_port.c b/tools/usb/usbip/src/usbip_port.c
index 7bd74fb3a9cd..a981a73b7dfb 100644
--- a/tools/usb/usbip/src/usbip_port.c
+++ b/tools/usb/usbip/src/usbip_port.c
@@ -13,54 +13,157 @@
  * GNU General Public License for more details.
  */
 
+#include <getopt.h>
+
 #include "vhci_driver.h"
 #include "usbip_common.h"
+#include "usbip_enumerate.h"
+#include "utils.h"
+
+static const char usbip_port_usage_string[] =
+	"usbip port <args>\n"
+	"    -i, --vhci-ix=<ix>   index of the "
+	USBIP_VHCI_DRV_NAME
+	" the device is on (defaults to 0)\n"
+	"    -a, --all            list the ports from all the available "
+	USBIP_VHCI_DRV_NAME "'s\n";
+
+void usbip_port_usage(void)
+{
+	printf("usage: %s", usbip_port_usage_string);
+}
 
 static int list_imported_devices(void)
 {
 	int i;
 	struct usbip_imported_device *idev;
-	int ret;
+	int ret = 0;
+
+	for (i = 0; i < vhci_driver->nports; i++) {
+		idev = &vhci_driver->idev[i];
 
+		if (usbip_vhci_imported_device_dump(idev) < 0) {
+			err("unable to list device %d", i);
+			ret = -1;
+		}
+	}
+	return ret;
+}
+
+static void list_imported_devices_header(void)
+{
+	printf("Imported USB devices\n");
+	printf("====================\n");
+}
+
+static int list_imported_devices_ix(int vhci_ix)
+{
+	int ret;
 	if (usbip_names_init(USBIDS_FILE))
 		err("failed to open %s", USBIDS_FILE);
 
-	ret = usbip_vhci_driver_open();
+	ret = usbip_vhci_driver_open_ix(vhci_ix);
 	if (ret < 0) {
 		err("open vhci_driver");
 		goto err_names_free;
 	}
 
-	printf("Imported USB devices\n");
-	printf("====================\n");
+	list_imported_devices_header();
+	ret = list_imported_devices();
+	usbip_vhci_driver_close();
+err_names_free:
+	usbip_names_free();
+	return ret;
+}
 
-	for (i = 0; i < vhci_driver->nports; i++) {
-		idev = &vhci_driver->idev[i];
+static int list_imported_devices_all(void)
+{
+	struct udev_enumerate *enumerate = NULL;
+	struct udev_list_entry *list, *entry;
+	int rc = 0;
+
+	if (usbip_names_init(USBIDS_FILE))
+		err("failed to open %s", USBIDS_FILE);
 
-		if (usbip_vhci_imported_device_dump(idev) < 0)
-			goto err_driver_close;
+	enumerate = vhci_enumerate();
+	if (!enumerate) {
+		err("Unable to list vhci_hcd drivers");
+		return -1;
 	}
 
-	usbip_vhci_driver_close();
-	usbip_names_free();
+	list = udev_enumerate_get_list_entry(enumerate);
+	if (!list) {
+		err("Unable to list vhci_hcd drivers");
+		return -1;
+	}
 
-	return ret;
+	list_imported_devices_header();
 
-err_driver_close:
-	usbip_vhci_driver_close();
-err_names_free:
+	udev_list_entry_foreach(entry, list) {
+		const char *path = udev_list_entry_get_name(entry);
+		int i;
+		int len = printf("VHCI: %s\n", path);
+
+		/* write a line of dashes */
+		for (i = 1; i < len; i++)
+			putchar('-');
+		putchar('\n');
+
+		if (usbip_vhci_driver_open_path(path) < 0) {
+			err("usbip_vhci_driver_open_path");
+			rc = -1;
+			continue;
+		}
+		if (list_imported_devices() < 0)
+			rc = -1;
+		usbip_vhci_driver_close();
+	}
 	usbip_names_free();
-	return -1;
+	udev_enumerate_unref(enumerate);
+
+	return rc;
 }
 
 int usbip_port_show(__attribute__((unused)) int argc,
 		    __attribute__((unused)) char *argv[])
 {
-	int ret;
+	int vhci_ix = 0;
+	int all = 0;
+	static const struct option opts[] = {
+		{ "vhci-ix", 0, NULL, 'i' },
+		{ "all", 0, NULL, 'a' },
+		{ NULL, 0, NULL, 0 }
+	};
 
-	ret = list_imported_devices();
-	if (ret < 0)
-		err("list imported devices");
+	for (;;) {
+		int opt = getopt_long(argc, argv, "i:a", opts, NULL);
 
-	return ret;
+		if (opt == -1)
+			break;
+		switch (opt) {
+		case 'i':
+			if (atoi_with_check(optarg, &vhci_ix) < 0) {
+				err("Bad vhci index");
+				return -1;
+			}
+			break;
+		case 'a':
+			all = 1;
+			break;
+		default:
+			goto err_out;
+		}
+	}
+
+	if (optind < argc)
+		goto err_out;
+
+	if (all)
+		return list_imported_devices_all();
+	else
+		return list_imported_devices_ix(vhci_ix);
+
+err_out:
+	usbip_port_usage();
+	return -1;
 }
diff --git a/tools/usb/usbip/src/utils.c b/tools/usb/usbip/src/utils.c
index 3d7b42e77299..0c55ab4f3d98 100644
--- a/tools/usb/usbip/src/utils.c
+++ b/tools/usb/usbip/src/utils.c
@@ -19,6 +19,8 @@
 #include <errno.h>
 #include <stdio.h>
 #include <string.h>
+#include <stdlib.h>
+#include <ctype.h>
 
 #include "usbip_common.h"
 #include "utils.h"
@@ -53,3 +55,17 @@ int modify_match_busid(char *busid, int add)
 
 	return 0;
 }
+
+int atoi_with_check(const char *str, int *pi)
+{
+	ssize_t len = strlen(str);
+
+	for (ssize_t i = 0; i < len; i++) {
+		if (!isdigit(str[i])) {
+			err("%s is not a number", str);
+			return -1;
+		}
+	}
+	*pi = atoi(str);
+	return 0;
+}
diff --git a/tools/usb/usbip/src/utils.h b/tools/usb/usbip/src/utils.h
index 5916fd3e02a6..a07d03c9bb12 100644
--- a/tools/usb/usbip/src/utils.h
+++ b/tools/usb/usbip/src/utils.h
@@ -20,6 +20,7 @@
 #define __UTILS_H
 
 int modify_match_busid(char *busid, int add);
+int atoi_with_check(const char *str, int *pi);
 
 #endif /* __UTILS_H */
 
-- 
2.14.1

[PATCH 4/4] config: make USB_MAXBUS configurable and adjust VHCI_NR_HCS top limit

[Salvador Fandino]

From: Salvador Fandiño <salva@qindel.com>

The maximum number of USB host controllers supported by the kernel had
a hard-coded limit of 64. In some scenarios that limit may be not
enough. For instance, in my particular case, I have systems with
thousands of containers running and would like to provide a VHCI
(USBIP host controller) device to every one of them but can't because
of this limit.

This patch adds a new configuration entry for USB_MAXBUS and also
increases the top limit for USBIP_VHCI_NR_HCS (number of VHCI
controllers) accordingly.

Signed-off-by: Salvador Fandiño <salva@qindel.com>
---
 drivers/usb/Kconfig       | 9 +++++++++
 drivers/usb/core/hcd.c    | 2 +-
 drivers/usb/usbip/Kconfig | 5 ++++-
 3 files changed, 14 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/Kconfig b/drivers/usb/Kconfig
index f699abab1787..78b3e2805d8f 100644
--- a/drivers/usb/Kconfig
+++ b/drivers/usb/Kconfig
@@ -72,6 +72,15 @@ config USB
 	  To compile this driver as a module, choose M here: the
 	  module will be called usbcore.
 
+config USB_MAXBUS
+	int "Maximum number of USB host controllers"
+	range 1 4096
+	default 64
+	---help---
+	  This defines the number of USB host controllers that would be
+	  available, both physical and virtual (for instance, those
+	  used by USB/IP).
+
 config USB_PCI
 	bool "PCI based USB host interface"
 	depends on PCI
diff --git a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c
index fc32391a34d5..e1589e950b86 100644
--- a/drivers/usb/core/hcd.c
+++ b/drivers/usb/core/hcd.c
@@ -83,7 +83,7 @@ DEFINE_IDR (usb_bus_idr);
 EXPORT_SYMBOL_GPL (usb_bus_idr);
 
 /* used when allocating bus numbers */
-#define USB_MAXBUS		64
+#define USB_MAXBUS		CONFIG_USB_MAXBUS
 
 /* used when updating list of hcds */
 DEFINE_MUTEX(usb_bus_idr_lock);	/* exported only for usbfs */
diff --git a/drivers/usb/usbip/Kconfig b/drivers/usb/usbip/Kconfig
index eeefa29f8aa2..cb87bec9dbbb 100644
--- a/drivers/usb/usbip/Kconfig
+++ b/drivers/usb/usbip/Kconfig
@@ -37,7 +37,7 @@ config USBIP_VHCI_HC_PORTS
 
 config USBIP_VHCI_NR_HCS
 	int "Number of USB/IP virtual host controllers"
-	range 1 128
+	range 1 2048
 	default 1
 	depends on USBIP_VHCI_HCD
 	---help---
@@ -46,6 +46,9 @@ config USBIP_VHCI_NR_HCS
 	  virtual host controllers as if adding physical host
 	  controllers.
 
+	  Note that the number of host controllers is also limited by
+	  USB_MAXBUS.
+
 config USBIP_HOST
 	tristate "Host driver"
 	depends on USBIP_CORE && USB
-- 
2.14.1

Re: [PATH 0/4] usbip: make vhci_hcd.* objects independent of vhci_hcd.0

[Shuah Khan]

On 01/30/2018 01:36 AM, Salvador Fandino wrote:
> Let me start by explaining the problem that have motivated me to write
> this patches:
> 
> I work on the QVD, a virtual desktop platform for Linux. This software
> runs Linux desktops (i.e. XFCE, KDE) and their applications inside LXC
> containers, and makes then available through the network to remote
> users.
> 
> Supporting USB devices is a common feature customers have been
> requesting us for a long time (in order to use, for instance, remote
> signature pads, bar-code scanners, fingerprint readers, etc.). So, we
> have been working on that feature using the USB/IP layer on the
> kernel.
> 
> Connecting and disconnecting devices and transferring data works
> seamless for the devices listed above. But we also want to make the
> usbip operations private to the container where they are run.  For
> instance, it is unacceptable for our product, that one user could list
> the devices connected by other users or access them.
> 
> We can control how can access every device using cgroups once those
> are attached, but the usbip layer is not providing any mechanism for
> controlling who can attach, detach or list the devices.
> 
> So, we got the idea that in order to enforce that remote usbip devices
> are only visible inside the container where they were imported, we
> could conveniently mount-bind inside every container just one of the
> vhci_hcd directories below /sys/devices/platform. So that it is as if
> every container had a vhci_hcd just for itself (and also, we restrict
> access to the matching USB ports in cgroups).
> 
> Unfortunately, all the vhci_hcd.* devices are controlled through
> attributes in vhci_hcd.0 making our approach fail and so... well, that
> is what this patch series changes. It makes every vhci_hcd device
> controllable through attributes inside its own sysfs directory.
> 
> The first patch, does that in the kernel, and the second and third
> patches change user space, adapting the libusbip and the usbip tools
> code respectively.
> 
> Then there is a fourth patch, that allows to create much more USB
> hubs per machine. It was limited to 64 and we are running thousands of
> containers (every one requiring a hub) per host.
> 
> These changes are not completely backward compatible. In the sysfs
> side, besides moving around the attribute files, now the port numbers
> go from 0 to CONFIG_USBIP_VHCI_HC_PORTS * 2 - 1 and are reused for
> every vhci_hcd device. I could have maintained the absolute numeration
> but I think reusing the numbers is a better and simpler approach.
> 
> I have considered that until very recently, support for vhci_hcd
> devices above the .0 was broken in the uspip tools (see
> 1ac7c8a78be85f84b019d3d2742d1a9f07255cc5 "usbip: fix usbip attach to
> find a port that matches the requested speed") and that has been the
> place where I have set the bar for backward compatibility: usage of
> the tools must remain unchanged for accessing "vhci_hcd.0", but may
> require changes otherwise. The same is true for the library functions
> which now provides new functions for selecting the target vhci_hcd
> device. The old functions now always target "vhci_hcd.0".
> 
> So, for instance, now, "usbip port" by default only shows "vhci_hcd.0"
> ports but "usbip port -a" shows all of them, and, for instance, "usbip
> port -i 4", shows the ports in "vhci_hcd.4".
> 
> Cheers.
> 

I will take a look at these in the next week or so.

thanks,
-- Shuah

Re: [PATH 0/4] usbip: make vhci_hcd.* objects independent of vhci_hcd.0

[Shuah Khan]

Hi Salvador,

On 01/30/2018 01:36 AM, Salvador Fandino wrote:
> Let me start by explaining the problem that have motivated me to write
> this patches:
> 
> I work on the QVD, a virtual desktop platform for Linux. This software
> runs Linux desktops (i.e. XFCE, KDE) and their applications inside LXC
> containers, and makes then available through the network to remote
> users.
> 
> Supporting USB devices is a common feature customers have been
> requesting us for a long time (in order to use, for instance, remote
> signature pads, bar-code scanners, fingerprint readers, etc.). So, we
> have been working on that feature using the USB/IP layer on the
> kernel.
> 
> Connecting and disconnecting devices and transferring data works
> seamless for the devices listed above. But we also want to make the
> usbip operations private to the container where they are run.  For
> instance, it is unacceptable for our product, that one user could list
> the devices connected by other users or access them.
> 
> We can control how can access every device using cgroups once those
> are attached, but the usbip layer is not providing any mechanism for
> controlling who can attach, detach or list the devices.

Did you explore a solution to add a mechanism for access control to
usbip?

> 
> So, we got the idea that in order to enforce that remote usbip devices
> are only visible inside the container where they were imported, we
> could conveniently mount-bind inside every container just one of the
> vhci_hcd directories below /sys/devices/platform. So that it is as if
> every container had a vhci_hcd just for itself (and also, we restrict
> access to the matching USB ports in cgroups).
> 
> Unfortunately, all the vhci_hcd.* devices are controlled through
> attributes in vhci_hcd.0 making our approach fail and so... well, that
> is what this patch series changes. It makes every vhci_hcd device
> controllable through attributes inside its own sysfs directory.> 
> The first patch, does that in the kernel, and the second and third
> patches change user space, adapting the libusbip and the usbip tools
> code respectively.
> 
> Then there is a fourth patch, that allows to create much more USB
> hubs per machine. It was limited to 64 and we are running thousands of
> containers (every one requiring a hub) per host.
> 
> These changes are not completely backward compatible. In the sysfs
> side, besides moving around the attribute files, now the port numbers
> go from 0 to CONFIG_USBIP_VHCI_HC_PORTS * 2 - 1 and are reused for
> every vhci_hcd device. I could have maintained the absolute numeration
> but I think reusing the numbers is a better and simpler approach.

Not being able to maintain backwards compatibility is an issue. This is
a considerable change to the user interface.

> 
> I have considered that until very recently, support for vhci_hcd
> devices above the .0 was broken in the uspip tools (see
> 1ac7c8a78be85f84b019d3d2742d1a9f07255cc5 "usbip: fix usbip attach to
> find a port that matches the requested speed") and that has been the
> place where I have set the bar for backward compatibility: usage of
> the tools must remain unchanged for accessing "vhci_hcd.0", but may
> require changes otherwise. The same is true for the library functions
> which now provides new functions for selecting the target vhci_hcd
> device. The old functions now always target "vhci_hcd.0".
> 
> So, for instance, now, "usbip port" by default only shows "vhci_hcd.0"
> ports but "usbip port -a" shows all of them, and, for instance, "usbip
> port -i 4", shows the ports in "vhci_hcd.4".
> 

I am going to play with these patches and how extensive the changes are
for users. In the meantime, maybe you can explore alternatives that don't
break backwards compatibility.

thanks,
-- Shuah

Re: [PATH 0/4] usbip: make vhci_hcd.* objects independent of vhci_hcd.0

[Salvador Fandiño]

On 02/21/2018 01:35 AM, Shuah Khan wrote:
> Hi Salvador,
> 
> On 01/30/2018 01:36 AM, Salvador Fandino wrote:
>> Let me start by explaining the problem that have motivated me to write
>> this patches:
>>
>> I work on the QVD, a virtual desktop platform for Linux. This software
>> runs Linux desktops (i.e. XFCE, KDE) and their applications inside LXC
>> containers, and makes then available through the network to remote
>> users.
>>
>> Supporting USB devices is a common feature customers have been
>> requesting us for a long time (in order to use, for instance, remote
>> signature pads, bar-code scanners, fingerprint readers, etc.). So, we
>> have been working on that feature using the USB/IP layer on the
>> kernel.
>>
>> Connecting and disconnecting devices and transferring data works
>> seamless for the devices listed above. But we also want to make the
>> usbip operations private to the container where they are run.  For
>> instance, it is unacceptable for our product, that one user could list
>> the devices connected by other users or access them.
>>
>> We can control how can access every device using cgroups once those
>> are attached, but the usbip layer is not providing any mechanism for
>> controlling who can attach, detach or list the devices.
> 
> Did you explore a solution to add a mechanism for access control to
> usbip?

Could you elaborate on that?

For "usbip", do you mean the user space tools? If that is the case, I 
don't think it would be enough. My aim is to limit vhci usage from 
containers and I have no control about what runs inside the containers. 
So, a mangled usbip tool-set could be used by a malicious user to 
circumvent any access control set there.

IMO, there is no other choice but to control access to VHCI at the 
kernel level.


> 
>>
>> So, we got the idea that in order to enforce that remote usbip devices
>> are only visible inside the container where they were imported, we
>> could conveniently mount-bind inside every container just one of the
>> vhci_hcd directories below /sys/devices/platform. So that it is as if
>> every container had a vhci_hcd just for itself (and also, we restrict
>> access to the matching USB ports in cgroups).
>>
>> Unfortunately, all the vhci_hcd.* devices are controlled through
>> attributes in vhci_hcd.0 making our approach fail and so... well, that
>> is what this patch series changes. It makes every vhci_hcd device
>> controllable through attributes inside its own sysfs directory.>
>> The first patch, does that in the kernel, and the second and third
>> patches change user space, adapting the libusbip and the usbip tools
>> code respectively.
>>
>> Then there is a fourth patch, that allows to create much more USB
>> hubs per machine. It was limited to 64 and we are running thousands of
>> containers (every one requiring a hub) per host.
>>
>> These changes are not completely backward compatible. In the sysfs
>> side, besides moving around the attribute files, now the port numbers
>> go from 0 to CONFIG_USBIP_VHCI_HC_PORTS * 2 - 1 and are reused for
>> every vhci_hcd device. I could have maintained the absolute numeration
>> but I think reusing the numbers is a better and simpler approach.
> 
> Not being able to maintain backwards compatibility is an issue. This is
> a considerable change to the user interface.

Well, it is true that it is a considerable change to the user interface 
breaking backward compatibility, but as I had already stated, that 
interface was broken until a couple of months ago, when my coworker, 
Juan Zea, reported it and nobody had noticed it before. So, I don't 
think we are going to affect too many people.

Note also that the user interface does not change when only vhci_hcd.0 
is used.


Regards

Re: [PATH 0/4] usbip: make vhci_hcd.* objects independent of vhci_hcd.0

[Shuah Khan]

On 03/05/2018 02:00 AM, Salvador Fandiño wrote:
> On 02/21/2018 01:35 AM, Shuah Khan wrote:
>> Hi Salvador,
>>
>> On 01/30/2018 01:36 AM, Salvador Fandino wrote:
>>> Let me start by explaining the problem that have motivated me to write
>>> this patches:
>>>
>>> I work on the QVD, a virtual desktop platform for Linux. This software
>>> runs Linux desktops (i.e. XFCE, KDE) and their applications inside LXC
>>> containers, and makes then available through the network to remote
>>> users.
>>>
>>> Supporting USB devices is a common feature customers have been
>>> requesting us for a long time (in order to use, for instance, remote
>>> signature pads, bar-code scanners, fingerprint readers, etc.). So, we
>>> have been working on that feature using the USB/IP layer on the
>>> kernel.
>>>
>>> Connecting and disconnecting devices and transferring data works
>>> seamless for the devices listed above. But we also want to make the
>>> usbip operations private to the container where they are run.  For
>>> instance, it is unacceptable for our product, that one user could list
>>> the devices connected by other users or access them.
>>>
>>> We can control how can access every device using cgroups once those
>>> are attached, but the usbip layer is not providing any mechanism for
>>> controlling who can attach, detach or list the devices.

In this use-case:

- does a container act as usbip client and attach devices from their
  host?
- do containers attach remote devices from other systems?

Is the core of the problem really that any remote system can import without
a provision for being able to restrict export to a set of remote machines?
If so, this is a generic problem even without containers and I would like
to solve this with a generic solution that works in all cases, not just for
containers.

The approach in this patch series appears to solve the problem just for
containers.

>>
>> Did you explore a solution to add a mechanism for access control to
>> usbip?
> 
> Could you elaborate on that?
> 
> For "usbip", do you mean the user space tools? 
> If that is the case, I don't think it would be enough.
> My aim is to limit vhci usage from containers and I have no control about what runs inside the containers. So, a mangled usbip tool-set could > > be used by a malicious user to circumvent any access control set there.> 

I mean the driver. There might be changes necessary in the user-space
as well depending on how the access controls are implemented. I am not
proposing implementing access controls in the user-space.


> IMO, there is no other choice but to control access to VHCI at the kernel level.
> 

Probably. Please give as many details as possible on your environment
for me to make a call on if this problem can be solved in a different
way.

thanks,
-- Shuah

Re: [PATH 0/4] usbip: make vhci_hcd.* objects independent of vhci_hcd.0

[Salvador Fandiño]

On 03/06/2018 01:03 AM, Shuah Khan wrote:
> On 03/05/2018 02:00 AM, Salvador Fandiño wrote:
>> On 02/21/2018 01:35 AM, Shuah Khan wrote:
>>> Hi Salvador,
>>>
>>> On 01/30/2018 01:36 AM, Salvador Fandino wrote:
>>>> Let me start by explaining the problem that have motivated me to write
>>>> this patches:
>>>>
>>>> I work on the QVD, a virtual desktop platform for Linux. This software
>>>> runs Linux desktops (i.e. XFCE, KDE) and their applications inside LXC
>>>> containers, and makes then available through the network to remote
>>>> users.
>>>>
>>>> Supporting USB devices is a common feature customers have been
>>>> requesting us for a long time (in order to use, for instance, remote
>>>> signature pads, bar-code scanners, fingerprint readers, etc.). So, we
>>>> have been working on that feature using the USB/IP layer on the
>>>> kernel.
>>>>
>>>> Connecting and disconnecting devices and transferring data works
>>>> seamless for the devices listed above. But we also want to make the
>>>> usbip operations private to the container where they are run.  For
>>>> instance, it is unacceptable for our product, that one user could list
>>>> the devices connected by other users or access them.
>>>>
>>>> We can control how can access every device using cgroups once those
>>>> are attached, but the usbip layer is not providing any mechanism for
>>>> controlling who can attach, detach or list the devices.
> In this use-case:
>
> - does a container act as usbip client and attach devices from their
>    host?
> - do containers attach remote devices from other systems?
In my particular case devices are imported from remote machines. But 
well, the thing is that I don't care where the connections come from, 
they could even be devices emulated in user space.

> Is the core of the problem really that any remote system can import without
> a provision for being able to restrict export to a set of remote machines?
> If so, this is a generic problem even without containers and I would like
> to solve this with a generic solution that works in all cases, not just for
> containers.
No, that is a different issue. You are talking about controlling which 
devices can be connected, from which hosts, etc. That is an interesting 
problem but not the one I am trying to tackle here.

I don't mind which every user does inside its container as far as it 
does not interfere which other users. In practice that means:

1- Not being able to attach/detach devices in other containers
2- Not being able to list devices attached in other containers
3- Not being able to access devices attached in other containers.

Point 3 is already enforceable using the devices hierarchy in cgroups. 
For points 1 and 2, my proposition is making every vhci_hcd device have 
its own fully independent sysfs directory (instead of all of them going 
through vhci_hcd.0) so that they can be selectively exposed with rw 
permissions inside the containers.



> The approach in this patch series appears to solve the problem just for
> containers.
>
>>> Did you explore a solution to add a mechanism for access control to
>>> usbip?
>> Could you elaborate on that?
>>
>> For "usbip", do you mean the user space tools?
>> If that is the case, I don't think it would be enough.
>> My aim is to limit vhci usage from containers and I have no control about what runs inside the containers. So, a mangled usbip tool-set could > > be used by a malicious user to circumvent any access control set there.>
> I mean the driver. There might be changes necessary in the user-space
> as well depending on how the access controls are implemented. I am not
> proposing implementing access controls in the user-space.
>
>
>> IMO, there is no other choice but to control access to VHCI at the kernel level.
>>
> Probably. Please give as many details as possible on your environment
> for me to make a call on if this problem can be solved in a different
> way.

In our particular real life application, we are targeting the kernel 
interface directly, we don't use the usbip tools at all. It is that way 
because we have our own* transport layer, authentication and 
authorization mechanisms. And once all the handshaking is done we end 
with a socket we can directly pass to the kernel in order to get it 
attached to a vhci_hcd port. We don't like having an extra application 
listening on some TCP port which can be accessed by third parties on the 
client side either.

The imported USB devices used are mostly devices which do not require 
kernel modules and that are accessed though libusb by the applications 
(i.e., id card readers, barcode scanners, signing pads, etc.).

* Just in case you want to know, USBIP data goes through a channel in a 
nx (https://github.com/ArcticaProject/nx-libs) connection running over a 
websocket over TLS. Authentication is performed by the broker (a proxy 
which knows where a user containers are running). Authorization is 
performed following policies configured by the administrator (currently 
it is just an all or nothing policy: USPIP is allowed or not) by the 
control application at container creation time.

Re: [PATH 0/4] usbip: make vhci_hcd.* objects independent of vhci_hcd.0

[Shuah Khan]

On 03/06/2018 01:35 AM, Salvador Fandiño wrote:
> 
> 
> On 03/06/2018 01:03 AM, Shuah Khan wrote:
>> On 03/05/2018 02:00 AM, Salvador Fandiño wrote:
>>> On 02/21/2018 01:35 AM, Shuah Khan wrote:
>>>> Hi Salvador,
>>>>
>>>> On 01/30/2018 01:36 AM, Salvador Fandino wrote:
>>>>> Let me start by explaining the problem that have motivated me to write
>>>>> this patches:
>>>>>
>>>>> I work on the QVD, a virtual desktop platform for Linux. This software
>>>>> runs Linux desktops (i.e. XFCE, KDE) and their applications inside LXC
>>>>> containers, and makes then available through the network to remote
>>>>> users.
>>>>>
>>>>> Supporting USB devices is a common feature customers have been
>>>>> requesting us for a long time (in order to use, for instance, remote
>>>>> signature pads, bar-code scanners, fingerprint readers, etc.). So, we
>>>>> have been working on that feature using the USB/IP layer on the
>>>>> kernel.
>>>>>
>>>>> Connecting and disconnecting devices and transferring data works
>>>>> seamless for the devices listed above. But we also want to make the
>>>>> usbip operations private to the container where they are run.  For
>>>>> instance, it is unacceptable for our product, that one user could list
>>>>> the devices connected by other users or access them.
>>>>>
>>>>> We can control how can access every device using cgroups once those
>>>>> are attached, but the usbip layer is not providing any mechanism for
>>>>> controlling who can attach, detach or list the devices.
>> In this use-case:
>>
>> - does a container act as usbip client and attach devices from their
>>    host?
>> - do containers attach remote devices from other systems?
> In my particular case devices are imported from remote machines. But well, the thing is that I don't care where the connections come from, they could even be devices emulated in user space.
> 
>> Is the core of the problem really that any remote system can import without
>> a provision for being able to restrict export to a set of remote machines?
>> If so, this is a generic problem even without containers and I would like
>> to solve this with a generic solution that works in all cases, not just for
>> containers.
> No, that is a different issue. You are talking about controlling which devices can be connected, from which hosts, etc. That is an interesting problem but not the one I am trying to tackle here.
> 

Not entirely. These problems are linked if you use usbip driver and usbip
tools. USBIp driver is intended to be used in conjunction with the usbip
tools.

> I don't mind which every user does inside its container as far as it does not interfere which other users. In practice that means:
> 
> 1- Not being able to attach/detach devices in other container

How do container attach/detach in other containers in your setup?

> 2- Not being able to list devices attached in other containers

How do container list devices in other containers in your setup?

> 3- Not being able to access devices attached in other containers.
> 
> Point 3 is already enforceable using the devices hierarchy in cgroups. For points 1 and 2, my proposition is making every vhci_hcd device have its own fully independent sysfs directory (instead of all of them going through vhci_hcd.0) so that they can be selectively exposed with rw permissions inside the containers.
> 
> 
> 
>> The approach in this patch series appears to solve the problem just for
>> containers.
>>
>>>> Did you explore a solution to add a mechanism for access control to
>>>> usbip?
>>> Could you elaborate on that?
>>>
>>> For "usbip", do you mean the user space tools?
>>> If that is the case, I don't think it would be enough.
>>> My aim is to limit vhci usage from containers and I have no control about what runs inside the containers. So, a mangled usbip tool-set could > > be used by a malicious user to circumvent any access control set there.>
>> I mean the driver. There might be changes necessary in the user-space
>> as well depending on how the access controls are implemented. I am not
>> proposing implementing access controls in the user-space.
>>
>>
>>> IMO, there is no other choice but to control access to VHCI at the kernel level.
>>>
>> Probably. Please give as many details as possible on your environment
>> for me to make a call on if this problem can be solved in a different
>> way.
> 
> In our particular real life application, we are targeting the kernel interface directly, we don't use the usbip tools at all. It is that way because we have our own* transport layer, authentication and authorization mechanisms. And once all the handshaking is done we end with a socket we can directly pass to the kernel in order to get it attached to a vhci_hcd port. 

How do you do that? Can you elaborate on how do you pass the socket to the USBIP
host? The way you are using is unsupported and just not the way it is designed
to be used.


We don't like having an extra application listening on some TCP port which can be accessed by third parties on the client side either.
> 

USBIP is a server/client protocol is intended to work that way. You can specify
a port to use.

> The imported USB devices used are mostly devices which do not require kernel modules and that are accessed though libusb by the applications (i.e., id card readers, barcode scanners, signing pads, etc.).

This is just not they was USBIP driver in the kernel is intended to be used.
I am beginning to think that USBIP isn't the right solution for your application.
You are talking about not using the protocol the way it is designed and finding
custom ways to use it.

I am all for making the USBIP more secure for container environment by adding
features to restrict:

- Remote machines that can import (attach) - this can be per device.
- Make sure detach is done only by the remote that imported the device
- Restrict listing of imported devices to the remote that imported them
- Enhance current version match to a stricter version match and add checksum
  match between kernel and user-space.

Let me know if you would like to explore the above options that are generic as
opposed to custom solution based on a setup that doesn't use the USBIP driver
the way it is designed to work.

thanks,
-- Shuah

Re: [PATH 0/4] usbip: make vhci_hcd.* objects independent of vhci_hcd.0

[Salvador Fandiño]

On 03/06/2018 09:58 PM, Shuah Khan wrote:
> On 03/06/2018 01:35 AM, Salvador Fandiño wrote:
>>
>> On 03/06/2018 01:03 AM, Shuah Khan wrote:
>>> On 03/05/2018 02:00 AM, Salvador Fandiño wrote:
>>>> On 02/21/2018 01:35 AM, Shuah Khan wrote:
>>>>> Hi Salvador,
>>>>>
>>>>> On 01/30/2018 01:36 AM, Salvador Fandino wrote:
>>>>>> Let me start by explaining the problem that have motivated me to write
>>>>>> this patches:
>>>>>>
>>>>>> I work on the QVD, a virtual desktop platform for Linux. This software
>>>>>> runs Linux desktops (i.e. XFCE, KDE) and their applications inside LXC
>>>>>> containers, and makes then available through the network to remote
>>>>>> users.
>>>>>>
>>>>>> Supporting USB devices is a common feature customers have been
>>>>>> requesting us for a long time (in order to use, for instance, remote
>>>>>> signature pads, bar-code scanners, fingerprint readers, etc.). So, we
>>>>>> have been working on that feature using the USB/IP layer on the
>>>>>> kernel.
>>>>>>
>>>>>> Connecting and disconnecting devices and transferring data works
>>>>>> seamless for the devices listed above. But we also want to make the
>>>>>> usbip operations private to the container where they are run.  For
>>>>>> instance, it is unacceptable for our product, that one user could list
>>>>>> the devices connected by other users or access them.
>>>>>>
>>>>>> We can control how can access every device using cgroups once those
>>>>>> are attached, but the usbip layer is not providing any mechanism for
>>>>>> controlling who can attach, detach or list the devices.
>>> In this use-case:
>>>
>>> - does a container act as usbip client and attach devices from their
>>>     host?
>>> - do containers attach remote devices from other systems?
>> In my particular case devices are imported from remote machines. But well, the thing is that I don't care where the connections come from, they could even be devices emulated in user space.
>>
>>> Is the core of the problem really that any remote system can import without
>>> a provision for being able to restrict export to a set of remote machines?
>>> If so, this is a generic problem even without containers and I would like
>>> to solve this with a generic solution that works in all cases, not just for
>>> containers.
>> No, that is a different issue. You are talking about controlling which devices can be connected, from which hosts, etc. That is an interesting problem but not the one I am trying to tackle here.
>>
> Not entirely. These problems are linked if you use usbip driver and usbip
> tools. USBIp driver is intended to be used in conjunction with the usbip
> tools.

I will come back to that later.



>> I don't mind which every user does inside its container as far as it does not interfere which other users. In practice that means:
>>
>> 1- Not being able to attach/detach devices in other container
> How do container attach/detach in other containers in your setup?

(let's for a moment forget I am using my own user-space USBIP layer, 
because it doesn't do anything different than the USBIP tools other than 
not using TCP sockets as the transport layer)

The sysfs is bind-mounted inside the container. I have also enabled 
access to USBIP devices through the cgroups device hierarchy.

In that scenario, I can just use the "usbip" tool to attach remote USBIP 
devices.

Limiting what devices can be attached or detached inside the "usbip" 
tool is not enough because, in general, the container's root can install 
any software he likes inside the container including for instance, a 
mangled "usbip" tool with any don't-touch-others-usbip-devices policy we 
could implement there disabled.


>> 2- Not being able to list devices attached in other containers
> How do container list devices in other containers in your setup?

Which the current implementation, running "usbip port" inside the 
container shows all the devices attached from the host or from any 
container.

The information is available from the "status*" entries in vhci_hcd.0, 
so, again, even if some 
see-only-the-usbip-devices-attached-from-this-container policy where 
enforced by the usbip tool, the container's root would still be able to 
read the status files directly and see all the devices connected host-wise.

In the end, that means that any enforcement of container-to-container or 
container-to-host USBIP access restriction must be made at the kernel 
level and that's just what my proposed patch allows to do: It makes the 
vhci_hcd devices fully independent. Every one having its own attach, 
detach and status objects. In that way, every container can be limited 
to accessing just one of them (well, or some subset) by selectively 
hidden vhci_hcd.* entries on the container sysfs when it is started. 
Then, from inside the container it is impossible to attach/detach 
devices in other containers vhci_hcd or to list them.

Coming back to where you said "Not entirely. These problems are linked 
if you use usbip driver and usbip tools. USBIp driver is intended to be 
used in conjunction with the usbip tools."

Well, as I have already explained, the usbip user space tools can not be 
used to limit access from the container to the host or to other 
containers because the container's root may mangle it in order to 
override any protection. So, I don't see how using or not using the 
usbip has any importance here.



>
>> 3- Not being able to access devices attached in other containers.
>>
>> Point 3 is already enforceable using the devices hierarchy in cgroups. For points 1 and 2, my proposition is making every vhci_hcd device have its own fully independent sysfs directory (instead of all of them going through vhci_hcd.0) so that they can be selectively exposed with rw permissions inside the containers.
>>
>>
>>
>>> The approach in this patch series appears to solve the problem just for
>>> containers.
>>>
>>>>> Did you explore a solution to add a mechanism for access control to
>>>>> usbip?
>>>> Could you elaborate on that?
>>>>
>>>> For "usbip", do you mean the user space tools?
>>>> If that is the case, I don't think it would be enough.
>>>> My aim is to limit vhci usage from containers and I have no control about what runs inside the containers. So, a mangled usbip tool-set could > > be used by a malicious user to circumvent any access control set there.>
>>> I mean the driver. There might be changes necessary in the user-space
>>> as well depending on how the access controls are implemented. I am not
>>> proposing implementing access controls in the user-space.
>>>
>>>
>>>> IMO, there is no other choice but to control access to VHCI at the kernel level.
>>>>
>>> Probably. Please give as many details as possible on your environment
>>> for me to make a call on if this problem can be solved in a different
>>> way.
>> In our particular real life application, we are targeting the kernel interface directly, we don't use the usbip tools at all. It is that way because we have our own* transport layer, authentication and authorization mechanisms. And once all the handshaking is done we end with a socket we can directly pass to the kernel in order to get it attached to a vhci_hcd port.
> How do you do that? Can you elaborate on how do you pass the socket to the USBIP
> host?
My userspace application creates a socketpair and passes one side to the 
kernel writing to the "attach" file. It does exactly the same the usbip 
tool does, the only difference is that instead of a TCP socket it uses a 
UNIX one.


>   The way you are using is unsupported and just not the way it is designed
> to be used.

Yea, that's the magic of Open Source. You intended your application to 
be used some way, somebody comes with a slightly different use case and 
submits a patch so that his use case is also supported...

In any case, the discussion here should not be affected by if I am using 
the standard user space tools or some custom ones. It is only related to 
how to effectively allow compartmentalization of the USBIP devices in a 
secure manner.


>
> We don't like having an extra application listening on some TCP port which can be accessed by third parties on the client side either.
> USBIP is a server/client protocol is intended to work that way. You can specify
> a port to use.
>
>> The imported USB devices used are mostly devices which do not require kernel modules and that are accessed though libusb by the applications (i.e., id card readers, barcode scanners, signing pads, etc.).
> This is just not they was USBIP driver in the kernel is intended to be used.
> I am beginning to think that USBIP isn't the right solution for your application.
> You are talking about not using the protocol the way it is designed and finding
> custom ways to use it.
>
> I am all for making the USBIP more secure for container environment by adding
> features to restrict:
>
> - Remote machines that can import (attach) - this can be per device.
> - Make sure detach is done only by the remote that imported the device
> - Restrict listing of imported devices to the remote that imported them
> - Enhance current version match to a stricter version match and add checksum
>    match between kernel and user-space.
>
> Let me know if you would like to explore the above options that are generic as
> opposed to custom solution based on a setup that doesn't use the USBIP driver
> the way it is designed to work.
>
> thanks,
> -- Shuah
>
>
>

Re: [PATH 0/4] usbip: make vhci_hcd.* objects independent of vhci_hcd.0

[Salvador Fandino]

Ping!

In March, I submitted a patch-set to get USBIP to play nicely inside 
containers, but the discussion died after a couple of mails, I got 
abducted into some unrelated project and mostly forgot about it...

But now, I have some time available to put into this and would like to 
push it further.

So, can we pick the discussion up again?

Regards



On 03/08/2018 01:45 PM, Salvador Fandiño wrote:
> On 03/06/2018 09:58 PM, Shuah Khan wrote:
>> On 03/06/2018 01:35 AM, Salvador Fandiño wrote:
>>>
>>> On 03/06/2018 01:03 AM, Shuah Khan wrote:
>>>> On 03/05/2018 02:00 AM, Salvador Fandiño wrote:
>>>>> On 02/21/2018 01:35 AM, Shuah Khan wrote:
>>>>>> Hi Salvador,
>>>>>>
>>>>>> On 01/30/2018 01:36 AM, Salvador Fandino wrote:
>>>>>>> Let me start by explaining the problem that have motivated me to 
>>>>>>> write
>>>>>>> this patches:
>>>>>>>
>>>>>>> I work on the QVD, a virtual desktop platform for Linux. This 
>>>>>>> software
>>>>>>> runs Linux desktops (i.e. XFCE, KDE) and their applications 
>>>>>>> inside LXC
>>>>>>> containers, and makes then available through the network to remote
>>>>>>> users.
>>>>>>>
>>>>>>> Supporting USB devices is a common feature customers have been
>>>>>>> requesting us for a long time (in order to use, for instance, remote
>>>>>>> signature pads, bar-code scanners, fingerprint readers, etc.). 
>>>>>>> So, we
>>>>>>> have been working on that feature using the USB/IP layer on the
>>>>>>> kernel.
>>>>>>>
>>>>>>> Connecting and disconnecting devices and transferring data works
>>>>>>> seamless for the devices listed above. But we also want to make the
>>>>>>> usbip operations private to the container where they are run.  For
>>>>>>> instance, it is unacceptable for our product, that one user could 
>>>>>>> list
>>>>>>> the devices connected by other users or access them.
>>>>>>>
>>>>>>> We can control how can access every device using cgroups once those
>>>>>>> are attached, but the usbip layer is not providing any mechanism for
>>>>>>> controlling who can attach, detach or list the devices.
>>>> In this use-case:
>>>>
>>>> - does a container act as usbip client and attach devices from their
>>>>     host?
>>>> - do containers attach remote devices from other systems?
>>> In my particular case devices are imported from remote machines. But 
>>> well, the thing is that I don't care where the connections come from, 
>>> they could even be devices emulated in user space.
>>>
>>>> Is the core of the problem really that any remote system can import 
>>>> without
>>>> a provision for being able to restrict export to a set of remote 
>>>> machines?
>>>> If so, this is a generic problem even without containers and I would 
>>>> like
>>>> to solve this with a generic solution that works in all cases, not 
>>>> just for
>>>> containers.
>>> No, that is a different issue. You are talking about controlling 
>>> which devices can be connected, from which hosts, etc. That is an 
>>> interesting problem but not the one I am trying to tackle here.
>>>
>> Not entirely. These problems are linked if you use usbip driver and usbip
>> tools. USBIp driver is intended to be used in conjunction with the usbip
>> tools.
> 
> I will come back to that later.
> 
> 
> 
>>> I don't mind which every user does inside its container as far as it 
>>> does not interfere which other users. In practice that means:
>>>
>>> 1- Not being able to attach/detach devices in other container
>> How do container attach/detach in other containers in your setup?
> 
> (let's for a moment forget I am using my own user-space USBIP layer, 
> because it doesn't do anything different than the USBIP tools other than 
> not using TCP sockets as the transport layer)
> 
> The sysfs is bind-mounted inside the container. I have also enabled 
> access to USBIP devices through the cgroups device hierarchy.
> 
> In that scenario, I can just use the "usbip" tool to attach remote USBIP 
> devices.
> 
> Limiting what devices can be attached or detached inside the "usbip" 
> tool is not enough because, in general, the container's root can install 
> any software he likes inside the container including for instance, a 
> mangled "usbip" tool with any don't-touch-others-usbip-devices policy we 
> could implement there disabled.
> 
> 
>>> 2- Not being able to list devices attached in other containers
>> How do container list devices in other containers in your setup?
> 
> Which the current implementation, running "usbip port" inside the 
> container shows all the devices attached from the host or from any 
> container.
> 
> The information is available from the "status*" entries in vhci_hcd.0, 
> so, again, even if some 
> see-only-the-usbip-devices-attached-from-this-container policy where 
> enforced by the usbip tool, the container's root would still be able to 
> read the status files directly and see all the devices connected host-wise.
> 
> In the end, that means that any enforcement of container-to-container or 
> container-to-host USBIP access restriction must be made at the kernel 
> level and that's just what my proposed patch allows to do: It makes the 
> vhci_hcd devices fully independent. Every one having its own attach, 
> detach and status objects. In that way, every container can be limited 
> to accessing just one of them (well, or some subset) by selectively 
> hidden vhci_hcd.* entries on the container sysfs when it is started. 
> Then, from inside the container it is impossible to attach/detach 
> devices in other containers vhci_hcd or to list them.
> 
> Coming back to where you said "Not entirely. These problems are linked 
> if you use usbip driver and usbip tools. USBIp driver is intended to be 
> used in conjunction with the usbip tools."
> 
> Well, as I have already explained, the usbip user space tools can not be 
> used to limit access from the container to the host or to other 
> containers because the container's root may mangle it in order to 
> override any protection. So, I don't see how using or not using the 
> usbip has any importance here.
> 
> 
> 
>>
>>> 3- Not being able to access devices attached in other containers.
>>>
>>> Point 3 is already enforceable using the devices hierarchy in 
>>> cgroups. For points 1 and 2, my proposition is making every vhci_hcd 
>>> device have its own fully independent sysfs directory (instead of all 
>>> of them going through vhci_hcd.0) so that they can be selectively 
>>> exposed with rw permissions inside the containers.
>>>
>>>
>>>
>>>> The approach in this patch series appears to solve the problem just for
>>>> containers.
>>>>
>>>>>> Did you explore a solution to add a mechanism for access control to
>>>>>> usbip?
>>>>> Could you elaborate on that?
>>>>>
>>>>> For "usbip", do you mean the user space tools?
>>>>> If that is the case, I don't think it would be enough.
>>>>> My aim is to limit vhci usage from containers and I have no control 
>>>>> about what runs inside the containers. So, a mangled usbip tool-set 
>>>>> could > > be used by a malicious user to circumvent any access 
>>>>> control set there.>
>>>> I mean the driver. There might be changes necessary in the user-space
>>>> as well depending on how the access controls are implemented. I am not
>>>> proposing implementing access controls in the user-space.
>>>>
>>>>
>>>>> IMO, there is no other choice but to control access to VHCI at the 
>>>>> kernel level.
>>>>>
>>>> Probably. Please give as many details as possible on your environment
>>>> for me to make a call on if this problem can be solved in a different
>>>> way.
>>> In our particular real life application, we are targeting the kernel 
>>> interface directly, we don't use the usbip tools at all. It is that 
>>> way because we have our own* transport layer, authentication and 
>>> authorization mechanisms. And once all the handshaking is done we end 
>>> with a socket we can directly pass to the kernel in order to get it 
>>> attached to a vhci_hcd port.
>> How do you do that? Can you elaborate on how do you pass the socket to 
>> the USBIP
>> host?
> My userspace application creates a socketpair and passes one side to the 
> kernel writing to the "attach" file. It does exactly the same the usbip 
> tool does, the only difference is that instead of a TCP socket it uses a 
> UNIX one.
> 
> 
>>   The way you are using is unsupported and just not the way it is 
>> designed
>> to be used.
> 
> Yea, that's the magic of Open Source. You intended your application to 
> be used some way, somebody comes with a slightly different use case and 
> submits a patch so that his use case is also supported...
> 
> In any case, the discussion here should not be affected by if I am using 
> the standard user space tools or some custom ones. It is only related to 
> how to effectively allow compartmentalization of the USBIP devices in a 
> secure manner.
> 
> 
>>
>> We don't like having an extra application listening on some TCP port 
>> which can be accessed by third parties on the client side either.
>> USBIP is a server/client protocol is intended to work that way. You 
>> can specify
>> a port to use.
>>
>>> The imported USB devices used are mostly devices which do not require 
>>> kernel modules and that are accessed though libusb by the 
>>> applications (i.e., id card readers, barcode scanners, signing pads, 
>>> etc.).
>> This is just not they was USBIP driver in the kernel is intended to be 
>> used.
>> I am beginning to think that USBIP isn't the right solution for your 
>> application.
>> You are talking about not using the protocol the way it is designed 
>> and finding
>> custom ways to use it.
>>
>> I am all for making the USBIP more secure for container environment by 
>> adding
>> features to restrict:
>>
>> - Remote machines that can import (attach) - this can be per device.
>> - Make sure detach is done only by the remote that imported the device
>> - Restrict listing of imported devices to the remote that imported them
>> - Enhance current version match to a stricter version match and add 
>> checksum
>>    match between kernel and user-space.
>>
>> Let me know if you would like to explore the above options that are 
>> generic as
>> opposed to custom solution based on a setup that doesn't use the USBIP 
>> driver
>> the way it is designed to work.
>>
>> thanks,
>> -- Shuah
>>
>>
>>
> 
> 
>