Re: [PATCH v3] PCI: ACPI: Support Microsoft's "DmaProperty"

[Greg Kroah-Hartman <gregkh@linuxfoundation.org>]

On Thu, Feb 17, 2022 at 10:26:39AM -0800, Rajat Jain wrote:
> Hello,
> 
> On Wed, Feb 16, 2022 at 10:16 PM Greg Kroah-Hartman
> <gregkh@linuxfoundation.org> wrote:
> >
> > On Wed, Feb 16, 2022 at 02:05:41PM -0800, Rajat Jain wrote:
> > > The "DmaProperty" is supported and documented by Microsoft here:
> > > https://docs.microsoft.com/en-us/windows-hardware/drivers/pci/dsd-for-pcie-root-ports
> > > They use this property for DMA protection:
> > > https://docs.microsoft.com/en-us/windows/security/information-protection/kernel-dma-protection-for-thunderbolt
> > >
> > > Support the "DmaProperty" with the same semantics. Windows documents the
> > > property to apply to PCIe root ports only. Extend it to apply to any
> > > PCI device. This is useful for internal PCI devices that do not hang off
> > > a PCIe rootport, but offer an attack surface for DMA attacks (e.g.
> > > internal network devices).
> > >
> > > Signed-off-by: Rajat Jain <rajatja@google.com>
> > > ---
> > > v3: * Use Microsoft's documented property "DmaProperty"
> > >     * Resctrict to ACPI only
> > >
> > >  drivers/pci/pci-acpi.c | 18 ++++++++++++++++++
> > >  1 file changed, 18 insertions(+)
> > >
> > > diff --git a/drivers/pci/pci-acpi.c b/drivers/pci/pci-acpi.c
> > > index a42dbf448860..660baa60c040 100644
> > > --- a/drivers/pci/pci-acpi.c
> > > +++ b/drivers/pci/pci-acpi.c
> > > @@ -1350,12 +1350,30 @@ static void pci_acpi_set_external_facing(struct pci_dev *dev)
> > >               dev->external_facing = 1;
> > >  }
> > >
> > > +static void pci_acpi_check_for_dma_protection(struct pci_dev *dev)
> > > +{
> > > +     u8 val;
> > > +
> > > +     /*
> > > +      * Microsoft Windows uses this property, and is documented here:
> > > +      * https://docs.microsoft.com/en-us/windows-hardware/drivers/pci/dsd-for-pcie-root-ports
> > > +      * While Microsoft documents this property as only applicable to PCIe
> > > +      * root ports, we expand it to be applicable to any PCI device.

Web pages have a tendancy to die over time (will it be here in 20+
years?)  Please describe how Windows uses this attribute and what it
uses it for in the comment.


> > > +      */
> > > +     if (device_property_read_u8(&dev->dev, "DmaProperty", &val))
> > > +             return;
> >
> > Why not continue to only do this for PCIe devices like it is actually
> > being used for?  Why expand it?
> 
> Because devices hanging off of PCIe root ports are not the only ones
> that may need DMA protection. There may be internal PCI devices (that
> don't hang off a PCIe root port) that may need DMA protection.
> Examples include internal network controllers that may offer an attack
> surface by handling network data or running vendor firmware.

And why does Microsoft not do the same for them?  What attribute do they
use for that?

And again, this is for "dma protection" not "trusted / untrusted".
That name here is getting very confusing and as I have stated in the
past, is probably incorrect and needs to be changed.  Also userspace
policy decisions need to be made here which would define the
trust/untrusted value.

So how about just passing this on as what Windows does, and have a new
attribute for the device called "platform wants to protect dma accesses
for this device" or something like that?

naming is hard,

greg k-h