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

[Kuppuswamy, Sathyanarayanan]

On 10/1/21 12:00 PM, Alan Stern wrote:
> On Fri, Oct 01, 2021 at 11:09:52AM -0700, Dan Williams wrote:
> > On Fri, Oct 1, 2021 at 9:47 AM Alan Stern <stern@xxxxxxxxxxxxxxxxxxx> wrote:
> > > On Fri, Oct 01, 2021 at 09:13:54AM -0700, Dan Williams wrote:
> > > > Bear with me, and perhaps it's a lack of imagination on my part, but I
> > > > don't see how to get to a globally generic "authorized" sysfs ABI
> > > > given that USB and Thunderbolt want to do bus specific actions on
> > > > authorization toggle events. Certainly a default generic authorized
> > > > attribute can be defined for all the other buses that don't have
> > > > legacy here, but Thunderbolt will still require support for '2' as an
> > > > authorized value, and USB will still want to base probe decisions on
> > > > the authorization state of both the usb_device and the usb_interface.
> > >
> > > The USB part isn't really accurate (I can't speak for Thunderbolt).
> > > When a usb_device is deauthorized, the device will be unconfigured,
> > > deleting all its interfaces and removing the need for any probe
> > > decisions about them.  In other words, the probe decision for a
> > > usb_device or usb_interface depends only on the device's/interface's
> > > own authorization state.
> > >
> > > True, the interface binding code does contain a test of the device's
> > > authorization setting.  That test is redundant and can be removed.
> > >
> > > The actions that USB wants to take on authorization toggle events for
> > > usb_devices are: for authorize, select and install a configuration;
> > > for deauthorize, unconfigure the device.  Each of these could be
> > > handled simply enough just by binding/unbinding the device.  (There
> > > is some special code for handling wireless USB devices, but wireless
> > > USB is now defunct.)
> >
> > 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.

For thunderbolt driver, it looks much more complicated. Unless you
define some callbacks in struct bus_type, we cannot easily generalize
it (but such callbacks are not recommended because it brings bus specific
operations to core layer).

sysfs_read()
-> simple read

sysfs_write()
-> tb_switch_set_authorized()
  -> disapprove_switch()
     -> tb_domain_disapprove_switch()
        -> tb->cm_ops->disapprove_switch() (product specific call)
  -> tb_domain_approve_switch_key()
     -> tb->cm_ops->add_switch_key
     -> tb->cm_ops->approve_switch() (product specific call)
  -> tb_domain_approve_switch()
     -> tb->cm_ops->approve_switch() (product specific call)
  -> tb_domain_challenge_switch_key()
     -> tb->cm_ops->challenge_switch_key()
     -> crypto_alloc_shash()
     -> crypto_shash_setkey()
     -> crypto_shash_digest()
     -> tb->cm_ops->approve_switch() (product specific call)


> Alan Stern

--
Sathyanarayanan Kuppuswamy
Linux Kernel Developer