linux-kernel - Re: [PATCH v2 4/6] virtio: Initialize authorized attribute for confidential guest

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 for Android: free password hash cracker in your pocket

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

Date:   Wed, 6 Oct 2021 07:45:12 +0200
From:   Greg Kroah-Hartman <gregkh@...uxfoundation.org>
To:     Dan Williams <dan.j.williams@...el.com>
Cc:     Mika Westerberg <mika.westerberg@...ux.intel.com>,
        Alan Stern <stern@...land.harvard.edu>,
        "Kuppuswamy, Sathyanarayanan" 
        <sathyanarayanan.kuppuswamy@...ux.intel.com>,
        "Michael S. Tsirkin" <mst@...hat.com>,
        Borislav Petkov <bp@...en8.de>, X86 ML <x86@...nel.org>,
        Bjorn Helgaas <bhelgaas@...gle.com>,
        Thomas Gleixner <tglx@...utronix.de>,
        Ingo Molnar <mingo@...hat.com>,
        Andreas Noever <andreas.noever@...il.com>,
        Michael Jamet <michael.jamet@...el.com>,
        Yehezkel Bernat <YehezkelShB@...il.com>,
        "Rafael J . Wysocki" <rafael@...nel.org>,
        Jonathan Corbet <corbet@....net>,
        Jason Wang <jasowang@...hat.com>,
        Andi Kleen <ak@...ux.intel.com>,
        Kuppuswamy Sathyanarayanan <knsathya@...nel.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        Linux PCI <linux-pci@...r.kernel.org>,
        USB list <linux-usb@...r.kernel.org>,
        virtualization@...ts.linux-foundation.org
Subject: Re: [PATCH v2 4/6] virtio: Initialize authorized attribute for
 confidential guest

On Tue, Oct 05, 2021 at 03:33:29PM -0700, Dan Williams wrote:
> On Sun, Oct 3, 2021 at 10:16 PM Mika Westerberg
> <mika.westerberg@...ux.intel.com> wrote:
> >
> > Hi,
> >
> > On Fri, Oct 01, 2021 at 12:57:18PM -0700, Dan Williams wrote:
> > > > > Ah, so are you saying that it would be sufficient for USB if the
> > > > > generic authorized implementation did something like:
> > > > >
> > > > > dev->authorized = 1;
> > > > > device_attach(dev);
> > > > >
> > > > > ...for the authorize case, and:
> > > > >
> > > > > dev->authorize = 0;
> > > > > device_release_driver(dev);
> > > > >
> > > > > ...for the deauthorize case?
> > > >
> > > > Yes, I think so.  But I haven't tried making this change to test and
> > > > see what really happens.
> > >
> > > Sounds like a useful path for this effort to explore. Especially as
> > > Greg seems to want the proposed "has_probe_authorization" flag in the
> > > bus_type to disappear and make this all generic. It just seems that
> > > Thunderbolt would need deeper surgery to move what it does in the
> > > authorization toggle path into the probe and remove paths.
> > >
> > > Mika, do you see a path for Thunderbolt to align its authorization
> > > paths behind bus ->probe() ->remove() events similar to what USB might
> > > be able to support for a generic authorization path?
> >
> > In Thunderbolt "authorization" actually means whether there is a PCIe
> > tunnel to the device or not. There is no driver bind/unbind happening
> > when authorization toggles (well on Thunderbolt bus, there can be on PCI
> > bus after the tunnel is established) so I'm not entirely sure how we
> > could use the bus ->probe() or ->remove for that to be honest.
> 
> Greg, per your comment:
> 
> "... which was to move the way that busses are allowed to authorize
> the devices they wish to control into a generic way instead of being
> bus-specific logic."
> 
> We have USB and TB that have already diverged on the ABI here. The USB
> behavior is more in line with the "probe authorization" concept, while
> TB is about tunnel establishment and not cleanly tied to probe
> authorization. So while I see a path to a common authorization
> implementation for USB and other buses (per the insight from Alan), TB
> needs to retain the ability to record the authorization state as an
> enum rather than a bool, and emit a uevent on authorization status
> change.
> 
> So how about something like the following that moves the attribute
> into the core, but still calls back to TB and USB to perform their
> legacy authorization work. This new authorized attribute only shows up
> when devices default to not authorized, i.e. when userspace owns the
> allow list past critical-boot built-in drivers, or if the bus (USB /
> TB) implements ->authorize().

At quick glance, this looks better, but it would be good to see someone
test it :)

thanks,

greg k-h