linux-kernel - Re: [PATCH v4 rc] iommu: Skip PASID validation for devices without PASID capability

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]

Message-ID: <b8ab3c01-0602-4980-8c31-0d16c5de2545@linux.intel.com>
Date: Tue, 20 May 2025 17:12:46 +0800
From: Baolu Lu <baolu.lu@...ux.intel.com>
To: Tushar Dave <tdave@...dia.com>, joro@...tes.org, will@...nel.org,
 robin.murphy@....com, kevin.tian@...el.com, jgg@...dia.com,
 yi.l.liu@...el.com, iommu@...ts.linux.dev, linux-kernel@...r.kernel.org
Cc: baolu.lu@...ux.intel.com, linux-pci@...r.kernel.org,
 stable@...r.kernel.org
Subject: Re: [PATCH v4 rc] iommu: Skip PASID validation for devices without
 PASID capability

On 5/20/2025 9:19 AM, Tushar Dave wrote:
> Generally PASID support requires ACS settings that usually create
> single device groups, but there are some niche cases where we can get
> multi-device groups and still have working PASID support. The primary
> issue is that PCI switches are not required to treat PASID tagged TLPs
> specially so appropriate ACS settings are required to route all TLPs to
> the host bridge if PASID is going to work properly.
> 
> pci_enable_pasid() does check that each device that will use PASID has
> the proper ACS settings to achieve this routing.
> 
> However, no-PASID devices can be combined with PASID capable devices
> within the same topology using non-uniform ACS settings. In this case
> the no-PASID devices may not have strict route to host ACS flags and
> end up being grouped with the PASID devices.
> 
> This configuration fails to allow use of the PASID within the iommu
> core code which wrongly checks if the no-PASID device supports PASID.
> 
> Fix this by ignoring no-PASID devices during the PASID validation. They
> will never issue a PASID TLP anyhow so they can be ignored.
> 
> Fixes: c404f55c26fc ("iommu: Validate the PASID in iommu_attach_device_pasid()")
> Cc:stable@...r.kernel.org
> Signed-off-by: Tushar Dave<tdave@...dia.com>
> ---
> 
> changes in v4:
> - rebase to 6.15-rc7
> 
>   drivers/iommu/iommu.c | 43 ++++++++++++++++++++++++++++---------------
>   1 file changed, 28 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
> index 4f91a740c15f..9d728800a862 100644
> --- a/drivers/iommu/iommu.c
> +++ b/drivers/iommu/iommu.c
> @@ -3366,10 +3366,12 @@ static int __iommu_set_group_pasid(struct iommu_domain *domain,
>   	int ret;
>   
>   	for_each_group_device(group, device) {
> -		ret = domain->ops->set_dev_pasid(domain, device->dev,
> -						 pasid, old);
> -		if (ret)
> -			goto err_revert;
> +		if (device->dev->iommu->max_pasids > 0) {
> +			ret = domain->ops->set_dev_pasid(domain, device->dev,
> +							 pasid, old);
> +			if (ret)
> +				goto err_revert;
> +		}

You can save an indent by making it like this,

for_each_group_device(group, device) {
	if (device->dev->iommu->max_pasids == 0)
		continue;

	ret = domain->ops->set_dev_pasid(domain, device->dev, pasid, old);
	if (ret)
		goto err_revert;
}

Anyway,

Reviewed-by: Lu Baolu <baolu.lu@...ux.intel.com>