All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH RESEND] iommu/dma: Don't touch invalid iova_domain members
@ 2017-05-15 15:01 Robin Murphy
       [not found] ` <122f47aa5596dc8ef0a1ee76e4425c7b597479a4.1494859975.git.robin.murphy-5wv7dgnIgG8@public.gmane.org>
  0 siblings, 1 reply; 4+ messages in thread
From: Robin Murphy @ 2017-05-15 15:01 UTC (permalink / raw)
  To: joro-zLv9SwRftAIdnm+yROfE0A
  Cc: iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA,
	shankerd-sgV2jX0FEOL9JmXXK+q4OQ

When __iommu_dma_map() and iommu_dma_free_iova() are called from
iommu_dma_get_msi_page(), various iova_*() helpers are still invoked in
the process, whcih is unwise since they access a different member of the
union (the iova_domain) from that which was last written, and there's no
guarantee that sensible values will result anyway.

CLean up the code paths that are valid for an MSI cookie to ensure we
only do iova_domain-specific things when we're actually dealing with one.

Fixes: a44e6657585b ("iommu/dma: Clean up MSI IOVA allocation")
Reported-by: Nate Watterson <nwatters-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
Tested-by: Shanker Donthineni <shankerd-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
Tested-by: Bharat Bhushan <bharat.bhushan-3arQi8VN3Tc@public.gmane.org>
Signed-off-by: Robin Murphy <robin.murphy-5wv7dgnIgG8@public.gmane.org>
---

Resending rebased to 4.12-rc1 with fixes tag.

 drivers/iommu/dma-iommu.c | 13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c
index 8348f366ddd1..62618e77bedc 100644
--- a/drivers/iommu/dma-iommu.c
+++ b/drivers/iommu/dma-iommu.c
@@ -396,13 +396,13 @@ static void iommu_dma_free_iova(struct iommu_dma_cookie *cookie,
 		dma_addr_t iova, size_t size)
 {
 	struct iova_domain *iovad = &cookie->iovad;
-	unsigned long shift = iova_shift(iovad);
 
 	/* The MSI case is only ever cleaning up its most recent allocation */
 	if (cookie->type == IOMMU_DMA_MSI_COOKIE)
 		cookie->msi_iova -= size;
 	else
-		free_iova_fast(iovad, iova >> shift, size >> shift);
+		free_iova_fast(iovad, iova_pfn(iovad, iova),
+				size >> iova_shift(iovad));
 }
 
 static void __iommu_dma_unmap(struct iommu_domain *domain, dma_addr_t dma_addr,
@@ -617,11 +617,14 @@ static dma_addr_t __iommu_dma_map(struct device *dev, phys_addr_t phys,
 {
 	struct iommu_domain *domain = iommu_get_domain_for_dev(dev);
 	struct iommu_dma_cookie *cookie = domain->iova_cookie;
-	struct iova_domain *iovad = &cookie->iovad;
-	size_t iova_off = iova_offset(iovad, phys);
+	size_t iova_off = 0;
 	dma_addr_t iova;
 
-	size = iova_align(iovad, size + iova_off);
+	if (cookie->type == IOMMU_DMA_IOVA_COOKIE) {
+		iova_off = iova_offset(&cookie->iovad, phys);
+		size = iova_align(&cookie->iovad, size + iova_off);
+	}
+
 	iova = iommu_dma_alloc_iova(domain, size, dma_get_mask(dev), dev);
 	if (!iova)
 		return DMA_ERROR_CODE;
-- 
2.12.2.dirty

^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH RESEND] iommu/dma: Don't touch invalid iova_domain members
       [not found] ` <122f47aa5596dc8ef0a1ee76e4425c7b597479a4.1494859975.git.robin.murphy-5wv7dgnIgG8@public.gmane.org>
@ 2017-05-17 12:36   ` Joerg Roedel
       [not found]     ` <20170517123631.GB11068-zLv9SwRftAIdnm+yROfE0A@public.gmane.org>
  0 siblings, 1 reply; 4+ messages in thread
From: Joerg Roedel @ 2017-05-17 12:36 UTC (permalink / raw)
  To: Robin Murphy
  Cc: iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA,
	shankerd-sgV2jX0FEOL9JmXXK+q4OQ

On Mon, May 15, 2017 at 04:01:30PM +0100, Robin Murphy wrote:
> When __iommu_dma_map() and iommu_dma_free_iova() are called from
> iommu_dma_get_msi_page(), various iova_*() helpers are still invoked in
> the process, whcih is unwise since they access a different member of the
> union (the iova_domain) from that which was last written, and there's no
> guarantee that sensible values will result anyway.
> 
> CLean up the code paths that are valid for an MSI cookie to ensure we
> only do iova_domain-specific things when we're actually dealing with one.
> 
> Fixes: a44e6657585b ("iommu/dma: Clean up MSI IOVA allocation")
> Reported-by: Nate Watterson <nwatters-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
> Tested-by: Shanker Donthineni <shankerd-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
> Tested-by: Bharat Bhushan <bharat.bhushan-3arQi8VN3Tc@public.gmane.org>
> Signed-off-by: Robin Murphy <robin.murphy-5wv7dgnIgG8@public.gmane.org>

Applied to iommu/fixes. Why didn't you add the Tested-by from Eric in
you resend?


	Joerg

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH RESEND] iommu/dma: Don't touch invalid iova_domain members
       [not found]     ` <20170517123631.GB11068-zLv9SwRftAIdnm+yROfE0A@public.gmane.org>
@ 2017-05-17 14:32       ` Robin Murphy
       [not found]         ` <dfe27cdf-9c9c-8e8c-f753-3a0289eb5c30-5wv7dgnIgG8@public.gmane.org>
  0 siblings, 1 reply; 4+ messages in thread
From: Robin Murphy @ 2017-05-17 14:32 UTC (permalink / raw)
  To: Joerg Roedel
  Cc: iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA,
	shankerd-sgV2jX0FEOL9JmXXK+q4OQ

On 17/05/17 13:36, Joerg Roedel wrote:
> On Mon, May 15, 2017 at 04:01:30PM +0100, Robin Murphy wrote:
>> When __iommu_dma_map() and iommu_dma_free_iova() are called from
>> iommu_dma_get_msi_page(), various iova_*() helpers are still invoked in
>> the process, whcih is unwise since they access a different member of the
>> union (the iova_domain) from that which was last written, and there's no
>> guarantee that sensible values will result anyway.
>>
>> CLean up the code paths that are valid for an MSI cookie to ensure we
>> only do iova_domain-specific things when we're actually dealing with one.
>>
>> Fixes: a44e6657585b ("iommu/dma: Clean up MSI IOVA allocation")
>> Reported-by: Nate Watterson <nwatters-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
>> Tested-by: Shanker Donthineni <shankerd-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
>> Tested-by: Bharat Bhushan <bharat.bhushan-3arQi8VN3Tc@public.gmane.org>
>> Signed-off-by: Robin Murphy <robin.murphy-5wv7dgnIgG8@public.gmane.org>
> 
> Applied to iommu/fixes.

Thanks!

> Why didn't you add the Tested-by from Eric in
> you resend?

Mostly due to the relative timing of those two events ;)

Robin.

> 
> 
> 	Joerg
> 

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH RESEND] iommu/dma: Don't touch invalid iova_domain members
       [not found]         ` <dfe27cdf-9c9c-8e8c-f753-3a0289eb5c30-5wv7dgnIgG8@public.gmane.org>
@ 2017-05-17 14:43           ` Auger Eric
  0 siblings, 0 replies; 4+ messages in thread
From: Auger Eric @ 2017-05-17 14:43 UTC (permalink / raw)
  To: Robin Murphy, Joerg Roedel
  Cc: iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA,
	shankerd-sgV2jX0FEOL9JmXXK+q4OQ

Hi,

On 17/05/2017 16:32, Robin Murphy wrote:
> On 17/05/17 13:36, Joerg Roedel wrote:
>> On Mon, May 15, 2017 at 04:01:30PM +0100, Robin Murphy wrote:
>>> When __iommu_dma_map() and iommu_dma_free_iova() are called from
>>> iommu_dma_get_msi_page(), various iova_*() helpers are still invoked in
>>> the process, whcih is unwise since they access a different member of the
>>> union (the iova_domain) from that which was last written, and there's no
>>> guarantee that sensible values will result anyway.
>>>
>>> CLean up the code paths that are valid for an MSI cookie to ensure we
>>> only do iova_domain-specific things when we're actually dealing with one.
>>>
>>> Fixes: a44e6657585b ("iommu/dma: Clean up MSI IOVA allocation")
>>> Reported-by: Nate Watterson <nwatters-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
>>> Tested-by: Shanker Donthineni <shankerd-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
>>> Tested-by: Bharat Bhushan <bharat.bhushan-3arQi8VN3Tc@public.gmane.org>
>>> Signed-off-by: Robin Murphy <robin.murphy-5wv7dgnIgG8@public.gmane.org>
>>
>> Applied to iommu/fixes.
> 
> Thanks!
> 
>> Why didn't you add the Tested-by from Eric in
>> you resend?
> 
> Mostly due to the relative timing of those two events ;)

Yes no worries. I showed up later!

Thanks

Eric
> 
> Robin.
> 
>>
>>
>> 	Joerg
>>
> 
> _______________________________________________
> iommu mailing list
> iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org
> https://lists.linuxfoundation.org/mailman/listinfo/iommu
> 

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2017-05-17 14:43 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-05-15 15:01 [PATCH RESEND] iommu/dma: Don't touch invalid iova_domain members Robin Murphy
     [not found] ` <122f47aa5596dc8ef0a1ee76e4425c7b597479a4.1494859975.git.robin.murphy-5wv7dgnIgG8@public.gmane.org>
2017-05-17 12:36   ` Joerg Roedel
     [not found]     ` <20170517123631.GB11068-zLv9SwRftAIdnm+yROfE0A@public.gmane.org>
2017-05-17 14:32       ` Robin Murphy
     [not found]         ` <dfe27cdf-9c9c-8e8c-f753-3a0289eb5c30-5wv7dgnIgG8@public.gmane.org>
2017-05-17 14:43           ` Auger Eric

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.