Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Reply-To: shuah@kernel.org
Subject: Re: [PATH 0/4] usbip: make vhci_hcd.* objects independent of
 vhci_hcd.0
To:     Salvador Fandino <salva@qindel.com>, linux-usb@vger.kernel.org
Cc:     gregkh@linuxfoundation.org, valentina.manea.m@gmail.com,
        linux-kernel@vger.kernel.org, Shuah Khan <shuah@kernel.org>,
        Shuah Khan <shuahkh@osg.samsung.com>
References: <20180130083630.26501-1-salva@qindel.com>
From:   Shuah Khan <shuah@kernel.org>
Message-ID: <391a54d5-73ac-3bd5-49ca-114639ea69fd@kernel.org>
Date:   Tue, 20 Feb 2018 17:35:51 -0700
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101
 Thunderbird/52.6.0
MIME-Version: 1.0
In-Reply-To: <20180130083630.26501-1-salva@qindel.com>
Content-Type: text/plain; charset=utf-8
Content-Language: en-US
Content-Transfer-Encoding: 8bit
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

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