Received-SPF: pass (google.com: domain of linux-kernel+bounces-157036-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.80.249 as permitted sender) client-ip=147.75.80.249;
Message-ID: <d43d54bc-e51d-4483-80ab-c0d52572de24@linux.intel.com>
Date: Wed, 24 Apr 2024 22:05:27 +0800
Precedence: bulk
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Cc: baolu.lu@linux.intel.com, joro@8bytes.org, will@kernel.org,
 ewagner12@gmail.com, suravee.suthikulpanit@amd.com, vashegde@amd.com,
 iommu@lists.linux.dev, linux-kernel@vger.kernel.org,
 regressions@lists.linux.dev
Subject: Re: [PATCH] iommu: Fix def_domain_type interaction with untrusted
 devices
To: Jason Gunthorpe <jgg@ziepe.ca>, Robin Murphy <robin.murphy@arm.com>
References: <fa14583e94cbf540b60a6be94b41bb24d0037e75.1713272443.git.robin.murphy@arm.com>
 <20240416152943.GU223006@ziepe.ca>
 <3cf97e3c-c8d9-4282-a8ca-e4c1ea383dbd@arm.com>
 <20240424130457.GF231144@ziepe.ca>
Content-Language: en-US
From: Baolu Lu <baolu.lu@linux.intel.com>
In-Reply-To: <20240424130457.GF231144@ziepe.ca>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit

On 2024/4/24 21:04, Jason Gunthorpe wrote:
>> Furthermore I'm pretty sure we're still letting an
>> untrusted device be hotplugged into an existing group without any
>> checks at all.
> Yes, combining trusted and untrusted in the same group, regardless of
> domain type, is fundamentally wrong and we don't check it.

I suppose this should be fixed in the PCI layer. The current algorithm
is setting external_facing to the pci port which has been marked as an
external facing port.

static void pci_acpi_set_external_facing(struct pci_dev *dev)
{
         u8 val;

         if (pci_pcie_type(dev) != PCI_EXP_TYPE_ROOT_PORT)
                 return;
         if (device_property_read_u8(&dev->dev, "ExternalFacingPort", &val))
                 return;

         /*
          * These root ports expose PCIe (including DMA) outside of the
          * system.  Everything downstream from them is external.
          */
         if (val)
                 dev->external_facing = 1;
}

Then, all devices connected to this port are marked as untrusted.

static void set_pcie_untrusted(struct pci_dev *dev)
{
         struct pci_dev *parent;

         /*
          * If the upstream bridge is untrusted we treat this device
          * untrusted as well.
          */
         parent = pci_upstream_bridge(dev);
         if (parent && (parent->untrusted || parent->external_facing))
                 dev->untrusted = true;
}

The above algorithms don't consider the ACS. Hence, an untrusted device
could possibly gain P2P access to other devices that are treated as
trusted. This appears to be already broken.

Thus, the impact has already beyond iommu group if I didn't overlook
anything.

Best regards,
baolu