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
| ||
|
Date: Wed, 22 Jun 2022 14:27:57 +0100 From: Robin Murphy <robin.murphy@....com> To: Joerg Roedel <joro@...tes.org>, yf.wang@...iatek.com Cc: Miles Chen <miles.chen@...iatek.com>, wsd_upstream@...iatek.com, open list <linux-kernel@...r.kernel.org>, Libo Kang <Libo.Kang@...iatek.com>, "open list:IOMMU DRIVERS" <iommu@...ts.linux-foundation.org>, "moderated list:ARM/Mediatek SoC support" <linux-mediatek@...ts.infradead.org>, Ning Li <ning.li@...iatek.com>, Matthias Brugger <matthias.bgg@...il.com>, Will Deacon <will@...nel.org>, "moderated list:ARM/Mediatek SoC support" <linux-arm-kernel@...ts.infradead.org> Subject: Re: [PATCH] iommu/dma: Fix race condition during iova_domain initialization On 2022-06-22 13:46, Joerg Roedel wrote: > Please re-send with > > Robin Murphy <robin.murphy@....com> > > in Cc. Apologies, I did spot this before, I've just been tied up with other things and dropping everything non-critical on the floor, so didn't get round to replying before it slipped my mind again. In summary, I hate it, but mostly because the whole situation of calling iommu_probe_device off the back of driver probe is fundamentally broken. I'm still a few steps away from fixing that properly, at which point I can just as well rip all these little bodges out again. If it really does need mitigating in the meantime (i.e. this is real-world async probe, not just some contrived testcase), then I can't easily think of any cleaner hack, so, Acked-by: Robin Murphy <robin.murphy@....com> (somewhat reluctantly) Cheers, Robin. > On Mon, May 30, 2022 at 08:07:45PM +0800, yf.wang@...iatek.com wrote: >> From: Yunfei Wang <yf.wang@...iatek.com> >> >> When many devices share the same iova domain, iommu_dma_init_domain() >> may be called at the same time. The checking of iovad->start_pfn will >> all get false in iommu_dma_init_domain() and both enter init_iova_domain() >> to do iovad initialization. >> >> Fix this by protecting init_iova_domain() with iommu_dma_cookie->mutex. >> >> Exception backtrace: >> rb_insert_color(param1=0xFFFFFF80CD2BDB40, param3=1) + 64 >> init_iova_domain() + 180 >> iommu_setup_dma_ops() + 260 >> arch_setup_dma_ops() + 132 >> of_dma_configure_id() + 468 >> platform_dma_configure() + 32 >> really_probe() + 1168 >> driver_probe_device() + 268 >> __device_attach_driver() + 524 >> __device_attach() + 524 >> bus_probe_device() + 64 >> deferred_probe_work_func() + 260 >> process_one_work() + 580 >> worker_thread() + 1076 >> kthread() + 332 >> ret_from_fork() + 16 >> >> Signed-off-by: Ning Li <ning.li@...iatek.com> >> Signed-off-by: Yunfei Wang <yf.wang@...iatek.com> >> --- >> drivers/iommu/dma-iommu.c | 17 +++++++++++++---- >> 1 file changed, 13 insertions(+), 4 deletions(-) >> >> diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c >> index 09f6e1c0f9c0..b38c5041eeab 100644 >> --- a/drivers/iommu/dma-iommu.c >> +++ b/drivers/iommu/dma-iommu.c >> @@ -63,6 +63,7 @@ struct iommu_dma_cookie { >> >> /* Domain for flush queue callback; NULL if flush queue not in use */ >> struct iommu_domain *fq_domain; >> + struct mutex mutex; >> }; >> >> static DEFINE_STATIC_KEY_FALSE(iommu_deferred_attach_enabled); >> @@ -309,6 +310,7 @@ int iommu_get_dma_cookie(struct iommu_domain *domain) >> if (!domain->iova_cookie) >> return -ENOMEM; >> >> + mutex_init(&domain->iova_cookie->mutex); >> return 0; >> } >> >> @@ -549,26 +551,33 @@ static int iommu_dma_init_domain(struct iommu_domain *domain, dma_addr_t base, >> } >> >> /* start_pfn is always nonzero for an already-initialised domain */ >> + mutex_lock(&cookie->mutex); >> if (iovad->start_pfn) { >> if (1UL << order != iovad->granule || >> base_pfn != iovad->start_pfn) { >> pr_warn("Incompatible range for DMA domain\n"); >> - return -EFAULT; >> + ret = -EFAULT; >> + goto done_unlock; >> } >> >> - return 0; >> + ret = 0; >> + goto done_unlock; >> } >> >> init_iova_domain(iovad, 1UL << order, base_pfn); >> ret = iova_domain_init_rcaches(iovad); >> if (ret) >> - return ret; >> + goto done_unlock; >> >> /* If the FQ fails we can simply fall back to strict mode */ >> if (domain->type == IOMMU_DOMAIN_DMA_FQ && iommu_dma_init_fq(domain)) >> domain->type = IOMMU_DOMAIN_DMA; >> >> - return iova_reserve_iommu_regions(dev, domain); >> + ret = iova_reserve_iommu_regions(dev, domain); >> + >> +done_unlock: >> + mutex_unlock(&cookie->mutex); >> + return ret; >> } >> >> /** >> -- >> 2.18.0 > _______________________________________________ > iommu mailing list > iommu@...ts.linux-foundation.org > https://lists.linuxfoundation.org/mailman/listinfo/iommu
Powered by blists - more mailing lists