linux-kernel - Re: [PATCH v2 1/2] iommu/amd: Enable swiotlb in all cases

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]

Date:   Thu, 7 Apr 2022 14:31:44 +0100
From:   Robin Murphy <robin.murphy@....com>
To:     Mario Limonciello <mario.limonciello@....com>,
        Joerg Roedel <joro@...tes.org>, Will Deacon <will@...nel.org>
Cc:     Christoph Hellwig <hch@...radead.org>,
        Marek Szyprowski <m.szyprowski@...sung.com>,
        "open list:IOMMU DRIVERS" <iommu@...ts.linux-foundation.org>,
        Suthikulpanit Suravee <Suravee.Suthikulpanit@....com>,
        Hegde Vasant <Vasant.Hegde@....com>,
        open list <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v2 1/2] iommu/amd: Enable swiotlb in all cases

On 2022-04-04 21:47, Mario Limonciello wrote:
> Previously the AMD IOMMU would only enable SWIOTLB in certain
> circumstances:
>   * IOMMU in passthrough mode
>   * SME enabled
> 
> This logic however doesn't work when an untrusted device is plugged in
> that doesn't do page aligned DMA transactions.  The expectation is
> that a bounce buffer is used for those transactions.
> 
> This fails like this:
> 
> swiotlb buffer is full (sz: 4096 bytes), total 0 (slots), used 0 (slots)
> 
> That happens because the bounce buffers have been allocated, followed by
> freed during startup but the bounce buffering code expects that all IOMMUs
> have left it enabled.
> 
> Remove the criteria to set up bounce buffers on AMD systems to ensure
> they're always available for supporting untrusted devices.

FWIW it's also broken for another niche case where 
iommu_default_passthrough() == false at init, but the user later changes 
a 32-bit device's default domain type to passthrough via sysfs, such 
that it starts needing regular dma-direct bouncing.

Reviewed-by: Robin Murphy <robin.murphy@....com>

> Fixes: 82612d66d51d ("iommu: Allow the dma-iommu api to use bounce buffers")
> Suggested-by: Christoph Hellwig <hch@...radead.org>
> Signed-off-by: Mario Limonciello <mario.limonciello@....com>
> ---
> v1->v2:
>   * Enable swiotlb for AMD instead of ignoring it for inactive
> 
>   drivers/iommu/amd/iommu.c | 7 -------
>   1 file changed, 7 deletions(-)
> 
> diff --git a/drivers/iommu/amd/iommu.c b/drivers/iommu/amd/iommu.c
> index a1ada7bff44e..079694f894b8 100644
> --- a/drivers/iommu/amd/iommu.c
> +++ b/drivers/iommu/amd/iommu.c
> @@ -1838,17 +1838,10 @@ void amd_iommu_domain_update(struct protection_domain *domain)
>   	amd_iommu_domain_flush_complete(domain);
>   }
>   
> -static void __init amd_iommu_init_dma_ops(void)
> -{
> -	swiotlb = (iommu_default_passthrough() || sme_me_mask) ? 1 : 0;
> -}
> -
>   int __init amd_iommu_init_api(void)
>   {
>   	int err;
>   
> -	amd_iommu_init_dma_ops();
> -
>   	err = bus_set_iommu(&pci_bus_type, &amd_iommu_ops);
>   	if (err)
>   		return err;