netdev - Re: [PATCH 4/4] thunderbolt: Get rid of E2E workaround

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CA+CmpXtRZ4JMe2V2-kWiYWR0pnnzLQMbXQESni6ne8eFeDCCXg@mail.gmail.com>
Date:   Mon, 15 Jun 2020 18:41:32 +0300
From:   Yehezkel Bernat <yehezkelshb@...il.com>
To:     Mika Westerberg <mika.westerberg@...ux.intel.com>
Cc:     Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        linux-usb@...r.kernel.org, Michael Jamet <michael.jamet@...el.com>,
        "David S . Miller" <davem@...emloft.net>,
        Jakub Kicinski <kuba@...nel.org>,
        Andreas Noever <andreas.noever@...il.com>,
        Lukas Wunner <lukas@...ner.de>, netdev@...r.kernel.org
Subject: Re: [PATCH 4/4] thunderbolt: Get rid of E2E workaround

On Mon, Jun 15, 2020 at 6:32 PM Mika Westerberg
<mika.westerberg@...ux.intel.com> wrote:
>
> On Mon, Jun 15, 2020 at 06:15:47PM +0300, Yehezkel Bernat wrote:
> > On Mon, Jun 15, 2020 at 5:22 PM Mika Westerberg
> > <mika.westerberg@...ux.intel.com> wrote:
> > >
> > > On Mon, Jun 15, 2020 at 05:18:38PM +0300, Yehezkel Bernat wrote:
> > > > On Mon, Jun 15, 2020 at 4:51 PM Greg Kroah-Hartman
> > > > <gregkh@...uxfoundation.org> wrote:
> > > > >
> > > > > On Mon, Jun 15, 2020 at 04:45:22PM +0300, Yehezkel Bernat wrote:
> > > > > > On Mon, Jun 15, 2020 at 4:02 PM Mika Westerberg
> > > > > > <mika.westerberg@...ux.intel.com> wrote:
> > > > > > >
> > > > > > > diff --git a/include/linux/thunderbolt.h b/include/linux/thunderbolt.h
> > > > > > > index ff397c0d5c07..5db2b11ab085 100644
> > > > > > > --- a/include/linux/thunderbolt.h
> > > > > > > +++ b/include/linux/thunderbolt.h
> > > > > > > @@ -504,8 +504,6 @@ struct tb_ring {
> > > > > > >  #define RING_FLAG_NO_SUSPEND   BIT(0)
> > > > > > >  /* Configure the ring to be in frame mode */
> > > > > > >  #define RING_FLAG_FRAME                BIT(1)
> > > > > > > -/* Enable end-to-end flow control */
> > > > > > > -#define RING_FLAG_E2E          BIT(2)
> > > > > > >
> > > > > >
> > > > > > Isn't it better to keep it (or mark it as reserved) so it'll not cause
> > > > > > compatibility issues with older versions of the driver or with Windows?
> > > > >
> > > > >
> > > > > How can you have "older versions of the driver"?  All drivers are in the
> > > > > kernel tree at the same time, you can't ever mix-and-match drivers and
> > > > > kernels.
> > > > >
> > > > > And how does Windows come into play here?
> > > > >
> > > >
> > > > As much as I remember, this flag is sent as part of creating the
> > > > interdomain connection.
> > > > If we reuse this bit to something else, and the other host runs an
> > > > older kernel or
> > > > Windows, this seems to be an issue.
> > > > But maybe I don't remember it correctly.
> > >
> > > We never send this flag anywhere. At the moment we do not announce
> > > support the "full E2E" in the network driver. Basically this is dead
> > > code what we remove.
> >
> > OK, maybe we never sent it, but Windows driver does send such a flag somewhere.
>
> It does yes but that is optional and we chose not to support it in
> Linux TBT network driver.
>
> > This is the only way both sides can know both of them support it and that they
> > should start using it. I'd prefer at least leaving a comment here that mentions
> > this, so if someone goes to add flags in the future, they will know to
> > take it into consideration.
>
> This flag here (RING_FLAG_E2E) is not exposed anywhere outside of
> thunderbolt driver.
>
> I think you are talking about the "prtstns" property in the network
> driver. There we only set TBNET_MATCH_FRAGS_ID (bit 1). This is the
> thing that get exposed to the other side of the connection and we never
> announced support for full E2E.


Ah, yes, this one, Thanks!
As Windows driver uses it for flagging full-E2E, and we completely drop E2E
support here, it may worth to mention there that this is what bit 2 is used in
Windows so any reuse should consider the possible compatibility issue.