Re: [PATCH v2 0/5] add non-strict mode support for arm-smmu-v3

[Jean-Philippe Brucker]

Hi Zhen Lei,

On 10/06/18 12:07, Zhen Lei wrote:
> v1 -> v2:
> Use the lowest bit of the io_pgtable_ops.unmap's iova parameter to pass the strict mode:
> 0, IOMMU_STRICT;
> 1, IOMMU_NON_STRICT;
> Treat 0 as IOMMU_STRICT, so that the unmap operation can compatible with
> other IOMMUs which still use strict mode. In other words, this patch series
> will not impact other IOMMU drivers. I tried add a new quirk IO_PGTABLE_QUIRK_NON_STRICT
> in io_pgtable_cfg.quirks, but it can not pass the strict mode of the domain from SMMUv3
> driver to io-pgtable module. 
> 
> Add a new member domain_non_strict in struct iommu_dma_cookie, this member will only be
> initialized when the related domain and IOMMU driver support non-strict mode.

It's not obvious from the commit messages or comments what the
non-strict mode involves exactly. Could you add a description, and point
out the trade-off associated with it?

In this mode you don't send an invalidate commands when removing a leaf
entry, but instead send invalidate-all commands at regular interval.
This improves performance but introduces a vulnerability window, which
should be pointed out to users.

IOVA allocation isn't the only problem, I'm concerned about the page
allocator. If unmap() returns early, the TLB entry is still valid after
the kernel reallocate the page. The device can then perform a
use-after-free (instead of getting a translation fault), so a buggy
device will corrupt memory and an untrusted one will access arbitrary data.

Or is there a way in mm to ensure that the page isn't reallocated until
the invalidation succeeds? Could dma-iommu help with this? Having
support from the mm would also help consolidate ATS, mark a page stale
when an ATC invalidation times out. But last time I checked it seemed
quite difficult to implement, and ATS is inherently insecure so I didn't
bother.

At the very least I think it might be worth warning users in dmesg about
this pitfall (and add_taint?). Tell them that an IOMMU in this mode is
good for scatter-gather performance but lacks full isolation. The
"non-strict" name seems somewhat harmless, and people should know what
they're getting into before enabling this.

Thanks,
Jean