linux-kernel - Re: [PATCH 1/4] PCI / ACPI: Identify external PCI devices

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]

Date:   Thu, 15 Nov 2018 19:58:13 +0200
From:   Yehezkel Bernat <yehezkelshb@...il.com>
To:     lorenzo.pieralisi@....com
Cc:     Mika Westerberg <mika.westerberg@...ux.intel.com>, lukas@...ner.de,
        iommu@...ts.linux-foundation.org, joro@...tes.org,
        David Woodhouse <dwmw2@...radead.org>,
        baolu.lu@...ux.intel.com, ashok.raj@...el.com,
        Bjorn Helgaas <bhelgaas@...gle.com>, rjw@...ysocki.net,
        jacob.jun.pan@...el.com, Andreas Noever <andreas.noever@...il.com>,
        michael.jamet@...el.com, Christian Kellner <ckellner@...hat.com>,
        Mario Limonciello <Mario.Limonciello@...l.com>,
        Anthony Wong <anthony.wong@...onical.com>,
        linux-acpi@...r.kernel.org, linux-pci@...r.kernel.org,
        LKML <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 1/4] PCI / ACPI: Identify external PCI devices

On Thu, Nov 15, 2018 at 7:46 PM Lorenzo Pieralisi
<lorenzo.pieralisi@....com> wrote:
>
> On Thu, Nov 15, 2018 at 02:16:27PM +0200, Mika Westerberg wrote:
> > On Thu, Nov 15, 2018 at 01:07:36PM +0100, Lukas Wunner wrote:
> > > On Thu, Nov 15, 2018 at 01:37:37PM +0200, Mika Westerberg wrote:
> > > > On Thu, Nov 15, 2018 at 11:13:56AM +0000, Lorenzo Pieralisi wrote:
> > > > > I have strong objections to the way these bindings have been forced upon
> > > > > everybody; if that's the way *generic* ACPI bindings are specified I
> > > > > wonder why there still exists an ACPI specification and related working
> > > > > group.
> > > > >
> > > > > I personally (but that's Bjorn and Rafael choice) think that this is
> > > > > not a change that belongs in PCI core, ACPI bindings are ill-defined
> > > > > and device tree bindings are non-existing.
> > > >
> > > > Any idea where should I put it then? These systems are already out there
> > > > and we need to support them one way or another.
> > >
> > > I suppose those are all Thunderbolt, so could be handled by the
> > > existing ->is_thunderbolt bit?
> > >
> > > It was said in this thread that ->is_external is more generic in
> > > that it could also be used on PCIe slots, however that use case
> > > doesn't appear to lend itself to the "plug in while laptop owner
> > > is getting coffee" attack.  To access PCIe slots on a server you
> > > normally need access to a data center.  On a desktop, you usually
> > > have to open the case, by which time the coffee may already have
> > > been fetched.  So frankly the binding seems a bit over-engineered
> > > to me and yet another thing that BIOS writers may get wrong.
> >
> > I would not say it should include PCIe slots but there are other cables
> > that carry PCIe and I was thinking we could make it to support those as
> > well.
> >
> > I have no problem using is_thunderbolt here, though if we don't want to
> > support non-Thunderbolt external devices this way.
> >
> > However, the question here is more that where I should put the _DSD
> > parsing code if it is not suitable to be placed inside PCI/ACPI core as
> > I've done in this patch? ;-)
>
> Do you really need to parse it if the dev->is_thunderbolt check is enough ?

>From what I know, there are more devices that suffer from similar security
issues like Thunderbolt, e.g. FireWire [1].
My assumption is that the same protection may be applied to such devices too,
even if currently it sounds like vendors care mostly about Thunderbolt (probably
because it removes the need for user approval for device connection; it becames
a simple plug-and-play experience).

Thus, I don't think binding it with dev->is_thunderbolt is the correct
thing to do.

[1] https://github.com/carmaa/inception