linux-kernel - Re: [PATCH] iommu: Fix def_domain_type interaction with untrusted devices

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: <20240418120224.GY223006@ziepe.ca>
Date: Thu, 18 Apr 2024 09:02:24 -0300
From: Jason Gunthorpe <jgg@...pe.ca>
To: Vasant Hegde <vashegde@....com>
Cc: Robin Murphy <robin.murphy@....com>, joro@...tes.org, will@...nel.org,
	ewagner12@...il.com, suravee.suthikulpanit@....com,
	iommu@...ts.linux.dev, linux-kernel@...r.kernel.org,
	regressions@...ts.linux.dev
Subject: Re: [PATCH] iommu: Fix def_domain_type interaction with untrusted
 devices

On Thu, Apr 18, 2024 at 05:14:59PM +0530, Vasant Hegde wrote:
> We return IDENTITY only if SNP and memory encryption is not enabled and device
> is SVA capable. Upstream has below code (v6.9-rc4)
> 
>         if (pdev_pasid_supported(dev_data) &&
>             !cc_platform_has(CC_ATTR_MEM_ENCRYPT) &&
>             !amd_iommu_snp_en) {
>                 return IOMMU_DOMAIN_IDENTITY;
>         }
> 
> 
> and during boot, we will enforce Paging domain when Encryption/SNP is enabled.

It is very confusing considering the comment says identity isn't supported:

        * Do not identity map IOMMUv2 capable devices when:
        *  - memory encryption is active, because some of those devices
        *    (AMD GPUs) don't have the encryption bit in their DMA-mask
        *    and require remapping.
        *  - SNP is enabled, because it prohibits DTE[Mode]=0.
        */

Then it goes on to return IDENTITY anyhow.

If the HW cannot identity map it needs to return DMA for those cases:

/* SNP is enabled, because it prohibits DTE[Mode]=0, IDENTITY is not
 * supported */
if (amd_snp_en)
   return IOMMU_DOMAIN_DMA;

/* memory encryption is active, because some of those devices
 * (AMD GPUs) don't have the encryption bit in their DMA-mask
 *  and require remapping. IDENTITY is not supported.
 */
if (cc_platform_has(CC_ATTR_MEM_ENCRYPT))
   return IOMMU_DOMAIN_DMA;

> So far the policy was core layer to handle untrusted device which got changed
> and hence we hit this bug.

Well I would say the AMD driver changed to mis-use def_domain for
policy and that causes problems.

> I don't see any issue with enforcing these checks inside AMD driver.. If we go
> with this approach then IMO core should just adhere to what driver returned
> rather than failing. Having policy decision at two level is inviting trouble
> like this one.

Again, no policy in drivers.

def_domain should return only the HW capability. If it returns
IDENTITY then the driver must fail attaches of PAGING.

You should send a patch fixing this function and remove the PASID test
entirely for now.

Jason