linux-kernel - Re: [PATCH v2 18/24] iommu: Express DMA strictness via the domain type

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: <f2858a7f-e665-ff6a-38d6-b9d591287fac@arm.com>
Date:   Thu, 29 Jul 2021 10:36:56 +0100
From:   Robin Murphy <robin.murphy@....com>
To:     Lu Baolu <baolu.lu@...ux.intel.com>, joro@...tes.org,
        will@...nel.org
Cc:     iommu@...ts.linux-foundation.org,
        linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org,
        suravee.suthikulpanit@....com, john.garry@...wei.com,
        dianders@...omium.org
Subject: Re: [PATCH v2 18/24] iommu: Express DMA strictness via the domain
 type

On 2021-07-29 08:13, Lu Baolu wrote:
> Hi Robin,
> 
> On 7/28/21 11:58 PM, Robin Murphy wrote:
>> diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
>> index 982545234cf3..eecb5657de69 100644
>> --- a/drivers/iommu/iommu.c
>> +++ b/drivers/iommu/iommu.c
>> @@ -136,6 +136,9 @@ static int __init iommu_subsys_init(void)
>>           }
>>       }
>> +    if (!iommu_default_passthrough() && !iommu_dma_strict)
>> +        iommu_def_domain_type = IOMMU_DOMAIN_DMA_FQ;
> 
> iommu_dma_strict could be changed later by the vendor iommu driver via
> iommu_set_dma_strict(). This seems not to be the right place to set
> iommu_def_domain_type.

Ah yes, good catch once again, thanks!

I think this *is* the right place to initially set it to honour the 
command-line option, since that matches what we do for passthrough. 
However also like passthrough we'll need to keep things in sync if it's 
updated later, like this:


diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index 87d7b299436e..593d4555bc57 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -359,6 +359,8 @@ early_param("iommu.strict", iommu_dma_setup);
  void iommu_set_dma_strict(void)
  {
         iommu_dma_strict = true;
+       if (iommu_def_domain_type == IOMMU_DOMAIN_DMA_FQ)
+               iommu_def_domain_type = IOMMU_DOMAIN_DMA;
  }

  static ssize_t iommu_group_attr_show(struct kobject *kobj,


Does that seem reasonable? I'm not sure there's any cleaner way to do it 
since we don't want to inadvertently clobber the default type if the 
user has given us something funky like "intel_iommu=strict 
iommu.passthrough=1".

Cheers,
Robin.

> 
>> +
>>       pr_info("Default domain type: %s %s\n",
>>           iommu_domain_type_str(iommu_def_domain_type),
>>           (iommu_cmd_line & IOMMU_CMD_LINE_DMA_API) ?
> 
> Best regards,
> baolu