From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-1.1 required=3.0 tests=DKIM_SIGNED,DKIM_VALID, DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,SPF_PASS autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id D9099C43381 for ; Mon, 25 Mar 2019 17:56:22 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 9A3F120823 for ; Mon, 25 Mar 2019 17:56:22 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (1024-bit key) header.d=chromium.org header.i=@chromium.org header.b="hlHXr/at" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1729716AbfCYR4V (ORCPT ); Mon, 25 Mar 2019 13:56:21 -0400 Received: from mail-lf1-f68.google.com ([209.85.167.68]:34596 "EHLO mail-lf1-f68.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1729036AbfCYR4U (ORCPT ); Mon, 25 Mar 2019 13:56:20 -0400 Received: by mail-lf1-f68.google.com with SMTP id y18so6760660lfe.1 for ; Mon, 25 Mar 2019 10:56:18 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=chromium.org; s=google; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=gxz4yoPt1q6OV76nKiFOODegQSe7rL5QL9V2EAX+Woo=; b=hlHXr/at2H4nErYZbKBI1q2uTyv9g0aaTySTLBtg2fh3UIeutwX4ysAJkcWWQPQLuU sgS3NrXbXlkxcjW1Y/jBm+/7LYo6volmPRiZxREkCSXGshOa00kS5t9u6sT1DEKAjcU3 S0Bw/Q3ROtGOnNS8vQ38KuW1BF2RINWpuSbkY= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=gxz4yoPt1q6OV76nKiFOODegQSe7rL5QL9V2EAX+Woo=; b=C4A8KdSRZIfQJrI4CaGd46VmrzCm8FA8JyL/ebyqf49VY7hJwqoROA9Q0KfeS3GQqD hHSgogew18EDI8lxth6XhQrcgOb0kwU4+Jf9Bmm68BJNk0Jcn1kGfYuZKuU7ndO8oPth uPUyeM65MFIdRnl+wIkOb/DmjMCeBqsZJvdieEWuQ9NPzU3E7FVHKJ+s93MbUUX76Vdt fzz+NCOX2hSXqiYkIOI9xUk4OY5I3oIdYdr38XDJN2DqB9Q3sSZcEdx0C1Ueewki4tll caGL8XQhtvt7n+LEBolHRMHu6ZtTMtMbEyJezGzDUUfLiYNkFv8yPyFq302Zr+Yl08Y+ 7xdg== X-Gm-Message-State: APjAAAWmG8I36vqeve3IANhsKTGQEgI3tGnTj5BE6pd4CoOZiWOKJ9sY 7Yi0kLGL2h/niB96wPZxnli5OESty0Y= X-Google-Smtp-Source: APXvYqwDcOTqWBdA1CPX5KwagShWn5wc/KZx80GoErTEUgk32bbm1FEd5TGH+S+hP6jcvkBzyZbQFg== X-Received: by 2002:a19:954b:: with SMTP id x72mr12968314lfd.67.1553536577474; Mon, 25 Mar 2019 10:56:17 -0700 (PDT) Received: from mail-lj1-f181.google.com (mail-lj1-f181.google.com. [209.85.208.181]) by smtp.gmail.com with ESMTPSA id s10sm131988lfc.20.2019.03.25.10.56.14 for (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Mon, 25 Mar 2019 10:56:14 -0700 (PDT) Received: by mail-lj1-f181.google.com with SMTP id v22so8663845lje.9 for ; Mon, 25 Mar 2019 10:56:14 -0700 (PDT) X-Received: by 2002:a2e:b058:: with SMTP id d24mr14870339ljl.42.1553536573991; Mon, 25 Mar 2019 10:56:13 -0700 (PDT) MIME-Version: 1.0 References: <20190306214730.6545-1-valleau@chromium.org> <44e151af-b6c4-d0af-3ef1-a5632d079915@kernel.org> In-Reply-To: From: Brian Norris Date: Mon, 25 Mar 2019 10:56:02 -0700 X-Gmail-Original-Message-ID: Message-ID: Subject: Re: [PATCH] tools: usb: usbip: adding support for older kernel versions To: shuah Cc: David Valleau , LKML , Linux USB Mailing List , Michael Grzeschik , Valentina Manea , Greg Kroah-Hartman , Sasha Levin Content-Type: text/plain; charset="UTF-8" Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi Shuah, On Mon, Mar 25, 2019 at 8:51 AM shuah wrote: > On 3/18/19 12:23 PM, Brian Norris wrote: > > On Sat, Mar 16, 2019 at 4:40 PM shuah 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 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. But this one is definitely a break: commit 1c9de5bf428612458427943b724bea51abde520a Author: Yuyang Du 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. 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 > +#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. > +#define V4_4_STATUS_HEADER "prt sta spd dev sockfd local_busid" > +#define V4_14_STATUS_HEADER "hub port sta spd dev sockfd local_busid" > > The difference here is the high speed support. Let's find a better > way to fix this than hard-coding kernel revisions in the tool. Yeah, I'm not in love with this particular patch, but it exposes the systemic problems you have already, so at least it serves to start the discussion. > > If stability is possible but you just don't care, then I guess we can > > fork our own version... > > > > Or even worse, we could build N copies of usbip for N kernels. But we > > don't do that for any other user space component. > > > > It might be easier to build N versions than maintaining the fork :) > > In any case, let's find ways to fix the problem with a constructive > approach. 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. If we can agree on that part, *then* we can talk about what (if anything) to do about patching usbip for compatibility with old (ABI-broken) kernels. Perhaps we will just throw out old kernels. Or maybe we'll do some minimal user space patching. Or maybe we patch our old kernels to match the latest ABI. That's not quite as big as a problem, as long as we agree on what ABI stability means going forward. Brian From mboxrd@z Thu Jan 1 00:00:00 1970 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: base64 Subject: tools: usb: usbip: adding support for older kernel versions From: Brian Norris Message-Id: Date: Mon, 25 Mar 2019 10:56:02 -0700 To: shuah Cc: David Valleau , LKML , Linux USB Mailing List , Michael Grzeschik , Valentina Manea , Greg Kroah-Hartman , Sasha Levin List-ID: SGkgU2h1YWgsCgpPbiBNb24sIE1hciAyNSwgMjAxOSBhdCA4OjUxIEFNIHNodWFoIDxzaHVhaEBr ZXJuZWwub3JnPiB3cm90ZToKPiBPbiAzLzE4LzE5IDEyOjIzIFBNLCBCcmlhbiBOb3JyaXMgd3Jv dGU6Cj4gPiBPbiBTYXQsIE1hciAxNiwgMjAxOSBhdCA0OjQwIFBNIHNodWFoIDxzaHVhaEBrZXJu ZWwub3JnPiB3cm90ZToKPiA+PiBJIHdvdWxkIGxpa2UgdG8gdW5kZXJzdGFuZCB0aGUgcmVhc29u cyBmb3Igd2FudGluZyB0byBydW4gbmV3IHRvb2wgb24KPiA+PiBvbGQga2VybmVscy4KPiA+Cj4g PiBPbiBDaHJvbWl1bSBPUywgd2Ugc2hpcCBtb3JlIG9yIGxlc3MgdGhlIHNhbWUgdXNlciBzcGFj ZSBmb3IgYSB2YXJpZXR5Cj4gPiBvZiBzeXN0ZW1zLCBidXQgbm90IGFsbCBvZiB0aG9zZSBydW4g dGhlIHNhbWUga2VybmVsLiBUaGF0J3Mgbm90Cj4gPiBleGFjdGx5IGEgbm92ZWwgY29uY2VwdCAt LSBtYW55IGdvb2QgdG9vbHMgYXJlIHdyaXR0ZW4gc3VjaCB0aGF0IHRoZXkKPiA+IGRlZ3JhZGUg Z3JhY2VmdWxseSB3aGVuIHJ1bm5pbmcgd2l0aCByZWR1Y2VkIGZlYXR1cmUgc2V0cyAoZS5nLiwg b2xkZXIKPiA+IGtlcm5lbHMpLiBXaGlsZSB3ZSBhcmUgd29ya2luZyBvbiByZWR1Y2luZyB0aGUg ZGl2ZXJnZW5jZSBhbmQgbnVtYmVyCj4gPiBvZiBrZXJuZWxzIHdlIHNoaXAsIGl0J3MgY3VycmVu dGx5IGEgZmFjdCBvZiBsaWZlIHRoYXQgd2UgaGF2ZSB0bwo+ID4gc3VwcG9ydCBtdWx0aXBsZSB0 YXJnZXQga2VybmVsIHZlcnNpb25zLgo+Cj4gVGhhbmtzIGZvciB0aGUgY29udGV4dCBmb3IgdGhp cyBjaGFuZ2UuCgpPbmUgdGhpbmcgdG8gYWRkOiB3ZSdyZSBjdXJyZW50bHkgb25seSB1c2luZyB1 c2JpcCBhcyBhIHRlc3QgdXRpbGl0eSwKYW5kIHdlJ3JlIG5vdCB5ZXQgZW5hYmxpbmcgaXQgb24g b2xkZXIga2VybmVscyAoYmVjYXVzZSBvZiB0aGlzCnByZWNpc2UgcHJvYmxlbSkuCgo+ID4gSXMg dGhlcmUgYSBmdW5kYW1lbnRhbCBwcm9ibGVtIHdpdGggVkhDSSBzdWNoIHRoYXQgaXQgZG9lc24n dCBoYXZlIGEKPiA+IHN0YWJsZSBBQkkgdGhhdCB0b29scyBjYW4gYmUgd3JpdHRlbiBhZ2FpbnN0 Pwo+ID4KPgo+IEluIGdlbmVyYWwgdGhlIEFCSSBpcyBzdGFibGUuCgpObywgaXQgcmVhbGx5IGlz bid0LiBUaGlzIGNvbW1pdCB3YXMgYSBicmVha2luZyBjaGFuZ2U6Cgpjb21taXQgMmYyZDAwODhl YjkzZGI1YzY0OWQyYTVlMzRhMzgwMGE4YTkzNWZjNQpBdXRob3I6IFNodWFoIEtoYW4gPHNodWFo a2hAb3NnLnNhbXN1bmcuY29tPgpEYXRlOiBUaHUgRGVjIDcgMTQ6MTY6NDkgMjAxNyAtMDcwMAoK ICB1c2JpcDogcHJldmVudCB2aGNpX2hjZCBkcml2ZXIgZnJvbSBsZWFraW5nIGEgc29ja2V0IHBv aW50ZXIgYWRkcmVzcwoKVGhhdCBvbmUgaXMgYXJndWFibHkgT0ssIHNpbmNlIG5vYm9keSB3YXMg YWN0dWFsbHkgdXNpbmcgdGhhdCBmaWVsZAooZXhjZXB0IGZvciBwYXJzaW5nIGl0IGFuZCB0aHJv d2luZyBpdCBhd2F5KSwgYW5kIGl0J3MgYSBzZWN1cml0eQpsZWFrLgoKQnV0IHRoaXMgb25lIGlz IGRlZmluaXRlbHkgYSBicmVhazoKCmNvbW1pdCAxYzlkZTViZjQyODYxMjQ1ODQyNzk0M2I3MjRi ZWE1MWFiZGU1MjBhCkF1dGhvcjogWXV5YW5nIER1IDx5dXlhbmcuZHVAaW50ZWwuY29tPgpEYXRl OiBUaHUgSnVuIDggMTM6MDQ6MTAgMjAxNyArMDgwMAoKICB1c2JpcDogdmhjaS1oY2Q6IEFkZCBV U0IzIFN1cGVyU3BlZWQgc3VwcG9ydAoKWW91IGNhbid0IGp1c3QgYXJiaXRyYXJpbHkgYWRkIGNv bHVtbnMgdG8gdGhlIGJlZ2lubmluZyBvZiBhIGZpbGUgbGlrZQp0aGF0IGFuZCBjbGFpbSB0aGF0 IHlvdSdyZSBub3QgYnJlYWtpbmcgQUJJLiBBbmQgSSBzaG91bGRuJ3QgbmVlZCB0bwpyZW1pbmQg eW91IHRoYXQgVGhvdSBTaGFsdCBOb3QgQnJlYWsgVXNlciBTcGFjZS4KCk5vdywgdGhpcyB3aG9s ZSBmaWxlIGhhcyB0b25zIG9mIHByb2JsZW1zOgogKiBpdCdzIG5vdCBkb2N1bWVudGVkIGF0IGFs bCBpbiBEb2N1bWVudGF0aW9uL0FCSS8gKHNvIGhleSwgeW91IGNhbgphcmd1ZSB0aGVyZSdzIG5v IEFCSSB0byBicmVhayEpCiAqIGl0IHZpb2xhdGVzIHRoZSAib25lIHBpZWNlIG9mIGluZm9ybWF0 aW9uIHBlciBhdHRyaWJ1dGUiIHJ1bGU7IHRoaXMKY29udHJpYnV0ZXMgdG8gdGhlIHByZXZpb3Vz IHBvaW50LCBzaW5jZSBpdCdzIGhhcmQgdG8gZXh0ZW5kIGFueXRoaW5nCndpdGhvdXQgYnJlYWtp bmcgdXNlciBzcGFjZQoKPiArI2RlZmluZSBWM18xOF9TVEFUVVNfSEVBREVSICJwcnQgc3RhIHNw ZCBidXMgZGV2IHNvY2tldAo+IGxvY2FsX2J1c2lkIgo+Cj4gV2hhdCdzIHlvdXIgMy4xOCBrZXJu ZWwgdmVyc2lvbj8gSSB0aGluayB5b3UgYXJlIG1pc3Npbmcgc2VjdXJpdHkKPiBmaXhlcyB0aGF0 IHByZXZlbnQgc29ja2V0IGFkZHJlc3MgbGVhayBpbiB0aGUgc3RhdHVzIGZpbGUuCgpXZSdyZSBu b3QgdXNpbmcgdGhlIGxhdGVzdCAzLjE4Lnkgc3RhYmxlIHNlcmllcywgaWYgdGhhdCdzIHdoYXQg eW91J3JlCmFza2luZy4gU28gbm8sIHdlIGRvbid0IGhhdmUgdGhhdCBzZWN1cml0eSBwYXRjaC4g QnV0IHdlIGFsc28gZG9uJ3QKY3VycmVudGx5IGVuYWJsZSBVU0JJUF9DT1JFIGFuZCBzaW1pbGFy LiBXZSBjYW4gYmFja3BvcnQgc3R1ZmYgaWYgd2UKbmVlZCAoYmFja3BvcnRpbmcgdGhhdCBzZWN1 cml0eSBmaXggaXMgb24gbXkvb3VyIHBsYXRlKSwgYnV0IHdlJ2QKcHJvYmFibHkgbGlrZSB0byB1 bmRlcnN0YW5kIHRoZSBtYWludGVuYW5jZSBsYW5kc2NhcGUgaGVyZSBmaXJzdC4KQ2xlYXJseSB1 c2JpcCBpcyBub3QgbWFpbnRhaW5lZCB3aXRoIEFCSSBjb21wYXRpYmlsaXR5IGluIG1pbmQuCgo+ ICsjZGVmaW5lIFY0XzRfU1RBVFVTX0hFQURFUiAicHJ0IHN0YSBzcGQgZGV2ICAgICAgc29ja2Zk IGxvY2FsX2J1c2lkIgo+ICsjZGVmaW5lIFY0XzE0X1NUQVRVU19IRUFERVIgImh1YiBwb3J0IHN0 YSBzcGQgZGV2ICAgICAgc29ja2ZkIGxvY2FsX2J1c2lkIgo+Cj4gVGhlIGRpZmZlcmVuY2UgaGVy ZSBpcyB0aGUgaGlnaCBzcGVlZCBzdXBwb3J0LiBMZXQncyBmaW5kIGEgYmV0dGVyCj4gd2F5IHRv IGZpeCB0aGlzIHRoYW4gaGFyZC1jb2Rpbmcga2VybmVsIHJldmlzaW9ucyBpbiB0aGUgdG9vbC4K ClllYWgsIEknbSBub3QgaW4gbG92ZSB3aXRoIHRoaXMgcGFydGljdWxhciBwYXRjaCwgYnV0IGl0 IGV4cG9zZXMgdGhlCnN5c3RlbWljIHByb2JsZW1zIHlvdSBoYXZlIGFscmVhZHksIHNvIGF0IGxl YXN0IGl0IHNlcnZlcyB0byBzdGFydCB0aGUKZGlzY3Vzc2lvbi4KCj4gPiBJZiBzdGFiaWxpdHkg aXMgcG9zc2libGUgYnV0IHlvdSBqdXN0IGRvbid0IGNhcmUsIHRoZW4gSSBndWVzcyB3ZSBjYW4K PiA+IGZvcmsgb3VyIG93biB2ZXJzaW9uLi4uCj4gPgo+ID4gT3IgZXZlbiB3b3JzZSwgd2UgY291 bGQgYnVpbGQgTiBjb3BpZXMgb2YgdXNiaXAgZm9yIE4ga2VybmVscy4gQnV0IHdlCj4gPiBkb24n dCBkbyB0aGF0IGZvciBhbnkgb3RoZXIgdXNlciBzcGFjZSBjb21wb25lbnQuCj4gPgo+Cj4gSXQg bWlnaHQgYmUgZWFzaWVyIHRvIGJ1aWxkIE4gdmVyc2lvbnMgdGhhbiBtYWludGFpbmluZyB0aGUg Zm9yayA6KQo+Cj4gSW4gYW55IGNhc2UsIGxldCdzIGZpbmQgd2F5cyB0byBmaXggdGhlIHByb2Js ZW0gd2l0aCBhIGNvbnN0cnVjdGl2ZQo+IGFwcHJvYWNoLgoKSSB0aGluayB3ZSBzaG91bGQgc2V0 dGxlIHRoZSBiaWdnZXN0IHF1ZXN0aW9ucyBmaXJzdDogZG9jdW1lbnRpbmcKKGFuZC9vciByZS1p bXBsZW1lbnRpbmcpIHRoZSBjdXJyZW50IEFCSSwgc28gdGhhdCB3ZSBoYXZlIGEgc3RhYmxlCnRh cmdldC4gR29pbmcgZm9yd2FyZCwgaWYgd2UgYXJlIHRvIHByZXRlbmQgdGhpcyBpcyBhIHJlYWwg a2VybmVsCmZlYXR1cmUsIGl0IHNob3VsZCBub3QgYmUgYnJlYWtpbmcgY29tcGF0aWJpbGl0eSBh cmJpdHJhcmlseSBsaWtlIGl0CmhhcyBhbHJlYWR5LiBJZiBJIHN0YXJ0ZWQgdXNpbmcgdXNiaXAg b24ga2VybmVsIDMuMTggKCsgdGhlCnBvaW50ZXItbGVha2luZyBzZWN1cml0eSBmaXgpLCBJIHNo b3VsZCBub3QgZXhwZWN0IHRvIHNlZSBteSB1c2VyCnNwYWNlIGJyZWFrIGp1c3QgYmVjYXVzZSBJ IHVwZ3JhZGVkIHRvIHRoZSBsYXRlc3Qga2VybmVsLgoKSWYgd2UgY2FuIGFncmVlIG9uIHRoYXQg cGFydCwgKnRoZW4qIHdlIGNhbiB0YWxrIGFib3V0IHdoYXQgKGlmCmFueXRoaW5nKSB0byBkbyBh Ym91dCBwYXRjaGluZyB1c2JpcCBmb3IgY29tcGF0aWJpbGl0eSB3aXRoIG9sZAooQUJJLWJyb2tl bikga2VybmVscy4gUGVyaGFwcyB3ZSB3aWxsIGp1c3QgdGhyb3cgb3V0IG9sZCBrZXJuZWxzLiBP cgptYXliZSB3ZSdsbCBkbyBzb21lIG1pbmltYWwgdXNlciBzcGFjZSBwYXRjaGluZy4gT3IgbWF5 YmUgd2UgcGF0Y2ggb3VyCm9sZCBrZXJuZWxzIHRvIG1hdGNoIHRoZSBsYXRlc3QgQUJJLiBUaGF0 J3Mgbm90IHF1aXRlIGFzIGJpZyBhcyBhCnByb2JsZW0sIGFzIGxvbmcgYXMgd2UgYWdyZWUgb24g d2hhdCBBQkkgc3RhYmlsaXR5IG1lYW5zIGdvaW5nCmZvcndhcmQuCgpCcmlhbgo=