Hi Brian,
On 3/25/19 11:56 AM, Brian Norris wrote:
Hi Shuah,
On Mon, Mar 25, 2019 at 8:51 AM shuah <shuah@xxxxxxxxxx> wrote:
On 3/18/19 12:23 PM, Brian Norris wrote:
On Sat, Mar 16, 2019 at 4:40 PM shuah <shuah@xxxxxxxxxx> wrote:
I would like to understand the reasons for wanting to run new tool on
old kernels.
On Chromium OS, we ship more or less the same user space for a variety
of systems, but not all of those run the same kernel. That's not
exactly a novel concept -- many good tools are written such that they
degrade gracefully when running with reduced feature sets (e.g., older
kernels). While we are working on reducing the divergence and number
of kernels we ship, it's currently a fact of life that we have to
support multiple target kernel versions.
Thanks for the context for this change.
One thing to add: we're currently only using usbip as a test utility,
and we're not yet enabling it on older kernels (because of this
precise problem).
Is there a fundamental problem with VHCI such that it doesn't have a
stable ABI that tools can be written against?
In general the ABI is stable.
No, it really isn't. This commit was a breaking change:
commit 2f2d0088eb93db5c649d2a5e34a3800a8a935fc5
Author: Shuah Khan <shuahkh@xxxxxxxxxxxxxxx>
Date: Thu Dec 7 14:16:49 2017 -0700
usbip: prevent vhci_hcd driver from leaking a socket pointer address
That one is arguably OK, since nobody was actually using that field
(except for parsing it and throwing it away), and it's a security
leak.
The ABI change and tool change went in the same security fix in my
back-port. I understand you are seeing it because you are using an
older 3.18 kernel. It was a judgment call to break ABI to fix the
leak. Looks like we are on the same page on this one.
But this one is definitely a break:
commit 1c9de5bf428612458427943b724bea51abde520a
Author: Yuyang Du <yuyang.du@xxxxxxxxx>
Date: Thu Jun 8 13:04:10 2017 +0800
usbip: vhci-hcd: Add USB3 SuperSpeed support
You can't just arbitrarily add columns to the beginning of a file like
that and claim that you're not breaking ABI. And I shouldn't need to
remind you that Thou Shalt Not Break User Space.
USB 3.0 driver and tool support went in, I would say it was oversight to
not make sure the tool continues to work on older kernels.
Now, this whole file has tons of problems:
* it's not documented at all in Documentation/ABI/ (so hey, you can
argue there's no ABI to break!)
* it violates the "one piece of information per attribute" rule; this
contributes to the previous point, since it's hard to extend anything
without breaking user space
Agreed. The documentation could use work.
+#define V3_18_STATUS_HEADER "prt sta spd bus dev socket
local_busid"
What's your 3.18 kernel version? I think you are missing security
fixes that prevent socket address leak in the status file.
We're not using the latest 3.18.y stable series, if that's what you're
asking. So no, we don't have that security patch. But we also don't
currently enable USBIP_CORE and similar. We can backport stuff if we
need (backporting that security fix is on my/our plate), but we'd
probably like to understand the maintenance landscape here first.
Clearly usbip is not maintained with ABI compatibility in mind.
I wouldn't characterize it is as "Clearly usbip is not maintained
with ABI compatibility in mind." It is not the intent to break ABI.
I back-ported the security to fix to all the stable releases.
I think we should settle the biggest questions first: documenting
(and/or re-implementing) the current ABI, so that we have a stable
target. Going forward, if we are to pretend this is a real kernel
feature, it should not be breaking compatibility arbitrarily like it
has already. If I started using usbip on kernel 3.18 (+ the
pointer-leaking security fix), I should not expect to see my user
space break just because I upgraded to the latest kernel.
So what's your ability to upgrade to 3.18 latest to get the security
fix. You would want to pick this up anyway.
Now the second issue is "supporting the latest tool on older kernels".
Guess I didn't think about the possibility of tools from 5.1 being run
on 3.18 :)
I am willing to guarantee that going forward the latest usbip tool
will not break on the supported stable releases.
I am going to take a look at fixing the tool to run on older kernels.
thanks,
-- Shuah
