lack of permission checking with intel-iommu?

lack of permission checking with intel-iommu?

[Christoph Hellwig]

Hi all,

we've recently had an interesting case where a device unexpectedly
modified a dma buffer passed to it.  On AMD systems this caused iommu
faults because the buffer was mapped read only (DMA_FROM_DEVICE), but
on an Intel system everything worked fine as the IOMMU did not trip up
on the device writes to that buffer.  Any idea why that could sneak
through?

Re: lack of permission checking with intel-iommu?

[Christoph Hellwig]

On Tue, Jul 02, 2024 at 12:46:59AM -0700, Christoph Hellwig wrote:
> Hi all,
> 
> we've recently had an interesting case where a device unexpectedly
> modified a dma buffer passed to it.  On AMD systems this caused iommu
> faults because the buffer was mapped read only (DMA_FROM_DEVICE), but
> on an Intel system everything worked fine as the IOMMU did not trip up
> on the device writes to that buffer.  Any idea why that could sneak
> through?

Sorry if this was confusing, the permission checking is of course about
the device reading from a buffer mapped read-only for the device
(as that is what DMA_FROM_DEVICE is), which on intel still allows it
to read.

Re: lack of permission checking with intel-iommu?

[Baolu Lu]

On 2024/7/2 16:02, Christoph Hellwig wrote:
> On Tue, Jul 02, 2024 at 12:46:59AM -0700, Christoph Hellwig wrote:
>> Hi all,
Hi Christoph,

>>
>> we've recently had an interesting case where a device unexpectedly
>> modified a dma buffer passed to it.  On AMD systems this caused iommu
>> faults because the buffer was mapped read only (DMA_FROM_DEVICE), but
>> on an Intel system everything worked fine as the IOMMU did not trip up
>> on the device writes to that buffer.  Any idea why that could sneak
>> through?
> Sorry if this was confusing, the permission checking is of course about
> the device reading from a buffer mapped read-only for the device
> (as that is what DMA_FROM_DEVICE is), which on intel still allows it
> to read.

Is it about IOMMU blocking "device reading from buffer" or "device
writing to buffer"?

The intel iommu driver might use first-stage translation for kernel DMA.
It seems not to block the "device reading from buffer" if I remember it
correctly.

Best regards,
baolu

Re: lack of permission checking with intel-iommu?

[Christoph Hellwig]

On Tue, Jul 02, 2024 at 04:22:25PM +0800, Baolu Lu wrote:
> Is it about IOMMU blocking "device reading from buffer" or "device
> writing to buffer"?
> 
> The intel iommu driver might use first-stage translation for kernel DMA.
> It seems not to block the "device reading from buffer" if I remember it
> correctly.

This is about device reading from buffer it is only supposed to write
to (but doesn't).

Re: lack of permission checking with intel-iommu?

[Yi Liu]

Hi Baolu,

On 2024/7/2 16:22, Baolu Lu wrote:
> On 2024/7/2 16:02, Christoph Hellwig wrote:
>> On Tue, Jul 02, 2024 at 12:46:59AM -0700, Christoph Hellwig wrote:
>>> Hi all,
> Hi Christoph,
> 
>>>
>>> we've recently had an interesting case where a device unexpectedly
>>> modified a dma buffer passed to it.  On AMD systems this caused iommu
>>> faults because the buffer was mapped read only (DMA_FROM_DEVICE), but
>>> on an Intel system everything worked fine as the IOMMU did not trip up
>>> on the device writes to that buffer.  Any idea why that could sneak
>>> through?
>> Sorry if this was confusing, the permission checking is of course about
>> the device reading from a buffer mapped read-only for the device
>> (as that is what DMA_FROM_DEVICE is), which on intel still allows it
>> to read.
> 
> Is it about IOMMU blocking "device reading from buffer" or "device
> writing to buffer"?
> 
> The intel iommu driver might use first-stage translation for kernel DMA.
> It seems not to block the "device reading from buffer" if I remember it
> correctly.

does the current kernel use the first-stage page table by default?

-- 
Regards,
Yi Liu

Re: lack of permission checking with intel-iommu?

[Baolu Lu]

On 2024/7/2 16:47, Yi Liu wrote:
>>>>
>>>> we've recently had an interesting case where a device unexpectedly
>>>> modified a dma buffer passed to it.  On AMD systems this caused iommu
>>>> faults because the buffer was mapped read only (DMA_FROM_DEVICE), but
>>>> on an Intel system everything worked fine as the IOMMU did not trip up
>>>> on the device writes to that buffer.  Any idea why that could sneak
>>>> through?
>>> Sorry if this was confusing, the permission checking is of course about
>>> the device reading from a buffer mapped read-only for the device
>>> (as that is what DMA_FROM_DEVICE is), which on intel still allows it
>>> to read.
>>
>> Is it about IOMMU blocking "device reading from buffer" or "device
>> writing to buffer"?
>>
>> The intel iommu driver might use first-stage translation for kernel DMA.
>> It seems not to block the "device reading from buffer" if I remember it
>> correctly.
> 
> does the current kernel use the first-stage page table by default?

Use first-stage page table for kernel DMA by default.

Best regards,
baolu

Re: lack of permission checking with intel-iommu?

[Baolu Lu]

On 2024/7/2 16:25, Christoph Hellwig wrote:
> On Tue, Jul 02, 2024 at 04:22:25PM +0800, Baolu Lu wrote:
>> Is it about IOMMU blocking "device reading from buffer" or "device
>> writing to buffer"?
>>
>> The intel iommu driver might use first-stage translation for kernel DMA.
>> It seems not to block the "device reading from buffer" if I remember it
>> correctly.
> This is about device reading from buffer it is only supposed to write
> to (but doesn't).

In this case, Intel IOMMU doesn't block device reading as the first-
stage page table only grants two types of permissions: Read-only or
Read-write. Write-only permission is not available.

Do you have a functional requirement for Write-only permission? If so,
we can choose the second-stage page table.

Best regards,
baolu

Re: lack of permission checking with intel-iommu?

[Christoph Hellwig]

On Tue, Jul 02, 2024 at 05:12:32PM +0800, Baolu Lu wrote:
> In this case, Intel IOMMU doesn't block device reading as the first-
> stage page table only grants two types of permissions: Read-only or
> Read-write. Write-only permission is not available.
> 
> Do you have a functional requirement for Write-only permission? If so,
> we can choose the second-stage page table.

No functional need, but it would have found a device bug.

RE: lack of permission checking with intel-iommu?

[Tian, Kevin]

> From: Baolu Lu <baolu.lu@linux.intel.com>
> Sent: Tuesday, July 2, 2024 5:13 PM
> 
> On 2024/7/2 16:25, Christoph Hellwig wrote:
> > On Tue, Jul 02, 2024 at 04:22:25PM +0800, Baolu Lu wrote:
> >> Is it about IOMMU blocking "device reading from buffer" or "device
> >> writing to buffer"?
> >>
> >> The intel iommu driver might use first-stage translation for kernel DMA.
> >> It seems not to block the "device reading from buffer" if I remember it
> >> correctly.
> > This is about device reading from buffer it is only supposed to write
> > to (but doesn't).
> 
> In this case, Intel IOMMU doesn't block device reading as the first-
> stage page table only grants two types of permissions: Read-only or
> Read-write. Write-only permission is not available.
> 
> Do you have a functional requirement for Write-only permission? If so,
> we can choose the second-stage page table.
> 

Note there could be configurations (e.g. vIOMMU) which only supports
1st-stage. So there is no guarantee that write-only permission can be
always enforced on Intel.

btw according to current code looks write-only even doesn't work on
2nd-stage:

__domain_mapping()
{
	...
	attr = prot & (DMA_PTE_READ | DMA_PTE_WRITE | DMA_PTE_SNP);
	attr |= DMA_FL_PTE_PRESENT;
	if (domain->use_first_level) {
                		...
	}
	...
}

DMA_FL_PTE_PRESENT and DMA_PTE_READ are both bit-0, so
READ is always set in 2nd-stage. That line should be done only
for domain->use_first_level==true

Re: lack of permission checking with intel-iommu?

[Baolu Lu]

On 7/3/24 8:56 AM, Tian, Kevin wrote:
>> From: Baolu Lu <baolu.lu@linux.intel.com>
>> Sent: Tuesday, July 2, 2024 5:13 PM
>>
>> On 2024/7/2 16:25, Christoph Hellwig wrote:
>>> On Tue, Jul 02, 2024 at 04:22:25PM +0800, Baolu Lu wrote:
>>>> Is it about IOMMU blocking "device reading from buffer" or "device
>>>> writing to buffer"?
>>>>
>>>> The intel iommu driver might use first-stage translation for kernel DMA.
>>>> It seems not to block the "device reading from buffer" if I remember it
>>>> correctly.
>>> This is about device reading from buffer it is only supposed to write
>>> to (but doesn't).
>>
>> In this case, Intel IOMMU doesn't block device reading as the first-
>> stage page table only grants two types of permissions: Read-only or
>> Read-write. Write-only permission is not available.
>>
>> Do you have a functional requirement for Write-only permission? If so,
>> we can choose the second-stage page table.
>>
> 
> Note there could be configurations (e.g. vIOMMU) which only supports
> 1st-stage. So there is no guarantee that write-only permission can be
> always enforced on Intel.
> 
> btw according to current code looks write-only even doesn't work on
> 2nd-stage:
> 
> __domain_mapping()
> {
> 	...
> 	attr = prot & (DMA_PTE_READ | DMA_PTE_WRITE | DMA_PTE_SNP);
> 	attr |= DMA_FL_PTE_PRESENT;
> 	if (domain->use_first_level) {
>                  		...
> 	}
> 	...
> }
> 
> DMA_FL_PTE_PRESENT and DMA_PTE_READ are both bit-0, so
> READ is always set in 2nd-stage. That line should be done only
> for domain->use_first_level==true

This is intended to make the permission consistent between first and
second stage.

Thanks,
baolu