Re: [PATCH 1/2] iommu: Fix race condition during default domain allocation

On Thu, Jun 17, 2021 at 11:21:39AM +0530, Ashish Mhetre wrote:

On 6/11/2021 6:19 PM, Robin Murphy wrote:

quoted

External email: Use caution opening links or attachments


On 2021-06-11 11:45, Will Deacon wrote:

quoted

On Thu, Jun 10, 2021 at 09:46:53AM +0530, Ashish Mhetre wrote:

quoted

Domain is getting created more than once during asynchronous multiple
display heads(devices) probe. All the display heads share same SID and
are expected to be in same domain. As iommu_alloc_default_domain() call
is not protected, the group->default_domain and group->domain are ending
up with different domains and leading to subsequent IOMMU faults.
Fix this by protecting iommu_alloc_default_domain() call with
group->mutex.

Can you provide some more information about exactly what the h/w
configuration is, and the callstack which exhibits the race, please?

It'll be basically the same as the issue reported long ago with PCI
groups in the absence of ACS not being constructed correctly. Triggering
the iommu_probe_device() replay in of_iommu_configure() off the back of
driver probe is way too late and allows calls to happen in the wrong
order, or indeed race in parallel as here. Fixing that is still on my
radar, but will not be simple, and will probably go hand-in-hand with
phasing out the bus ops (for the multiple-driver-coexistence problem).

For iommu group creation, the stack flow during race is like:
Display device 1:
iommu_probe_device -> iommu_group_get_for_dev -> arm_smmu_device_group
Display device 2:
iommu_probe_device -> iommu_group_get_for_dev -> arm_smmu_device_group

And this way it ends up in creating 2 groups for 2 display devices sharing
same SID.
Ideally for 2nd display device, iommu_group_get call from
iommu_group_get_for_dev should return same group as 1st display device. But
due to the race, it ends up with 2 groups.

For default domain, the stack flow during race is like:
Display device 1:
iommu_probe_device -> iommu_alloc_default_domain -> arm_smmu_domain_alloc
Display device 2:
iommu_probe_device -> iommu_alloc_default_domain -> arm_smmu_domain_alloc

Here also 2nd device should already have domain allocated and
'if(group->default_domain)' condition from iommu_alloc_default_domain should
be true for 2nd device.

Issue with this is IOVA accesses from 2nd device results in context faults.

Thanks for the explanation (also Robin and Krishna). Please put some of this
in the commit message for the next version.

Will

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel