[PATCH] arm: mm: fix DMA pool affiliation check

Subsystems: arm port, the rest

STALE5078d

2 messages, 2 authors, 2012-09-07 · open the first message on its own page

[PATCH] arm: mm: fix DMA pool affiliation check

From: Thomas Petazzoni <hidden>
Date: 2012-09-03 12:43:52

The __free_from_pool() function was changed in
e9da6e9905e639b0f842a244bc770b48ad0523e9. Unfortunately, the test that
checks whether the provided (start,size) is within the DMA pool has
been improperly modified. It used to be:

  if (start < coherent_head.vm_start || end > coherent_head.vm_end)

Where coherent_head.vm_end was non-inclusive (i.e, it did not include
the first byte after the pool). The test has been changed to:

  if (start < pool->vaddr || start > pool->vaddr + pool->size)

So now pool->vaddr + pool->size is inclusive (i.e, it includes the
first byte after the pool), so the test should be >= instead of >.

This bug causes the following message when freeing the *first* DMA
coherent buffer that has been allocated, because its virtual address
is exactly equal to pool->vaddr + pool->size :

WARNING: at /home/thomas/projets/linux-2.6/arch/arm/mm/dma-mapping.c:463 __free_from_pool+0xa4/0xc0()
freeing wrong coherent size from pool

Signed-off-by: Thomas Petazzoni <redacted>
Cc: Marek Szyprowski <m.szyprowski@samsung.com>
Cc: Russell King <redacted>
Cc: Lior Amsalem <redacted>
Cc: Maen Suleiman <redacted>
Cc: Tawfik Bayouk <redacted>
Cc: Shadi Ammouri <redacted>
Cc: Eran Ben-Avi <redacted>
Cc: Yehuda Yitschak <redacted>
Cc: Nadav Haklai <redacted>
---
 arch/arm/mm/dma-mapping.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/arm/mm/dma-mapping.c b/arch/arm/mm/dma-mapping.c
index 4e7d118..59d9062 100644
--- a/arch/arm/mm/dma-mapping.c
+++ b/arch/arm/mm/dma-mapping.c
@@ -456,7 +456,7 @@ static int __free_from_pool(void *start, size_t size)
 	unsigned long pageno, count;
 	unsigned long flags;
 
-	if (start < pool->vaddr || start > pool->vaddr + pool->size)
+	if (start < pool->vaddr || start >= pool->vaddr + pool->size)
 		return 0;
 
 	if (start + size > pool->vaddr + pool->size) {
-- 
1.7.9.5

[PATCH] arm: mm: fix DMA pool affiliation check

From: m.szyprowski@samsung.com (Marek Szyprowski)
Date: 2012-09-07 05:58:31

Hello,

On Monday, September 03, 2012 2:44 PM Thomas Petazzoni wrote:
The __free_from_pool() function was changed in
e9da6e9905e639b0f842a244bc770b48ad0523e9. Unfortunately, the test that
checks whether the provided (start,size) is within the DMA pool has
been improperly modified. It used to be:

  if (start < coherent_head.vm_start || end > coherent_head.vm_end)

Where coherent_head.vm_end was non-inclusive (i.e, it did not include
the first byte after the pool). The test has been changed to:

  if (start < pool->vaddr || start > pool->vaddr + pool->size)

So now pool->vaddr + pool->size is inclusive (i.e, it includes the
first byte after the pool), so the test should be >= instead of >.

This bug causes the following message when freeing the *first* DMA
coherent buffer that has been allocated, because its virtual address
is exactly equal to pool->vaddr + pool->size :

WARNING: at /home/thomas/projets/linux-2.6/arch/arm/mm/dma-mapping.c:463
__free_from_pool+0xa4/0xc0()
freeing wrong coherent size from pool

Signed-off-by: Thomas Petazzoni <redacted>
Cc: Marek Szyprowski <m.szyprowski@samsung.com>
Cc: Russell King <redacted>
Cc: Lior Amsalem <redacted>
Cc: Maen Suleiman <redacted>
Cc: Tawfik Bayouk <redacted>
Cc: Shadi Ammouri <redacted>
Cc: Eran Ben-Avi <redacted>
Cc: Yehuda Yitschak <redacted>
Cc: Nadav Haklai <redacted>
Thanks for spotting this issue and providing a fix. I will add it to my dma-mapping fixes branch for
v3.6 kernels.
quoted hunk
---
 arch/arm/mm/dma-mapping.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/arm/mm/dma-mapping.c b/arch/arm/mm/dma-mapping.c
index 4e7d118..59d9062 100644
--- a/arch/arm/mm/dma-mapping.c
+++ b/arch/arm/mm/dma-mapping.c
@@ -456,7 +456,7 @@ static int __free_from_pool(void *start, size_t size)
 	unsigned long pageno, count;
 	unsigned long flags;

-	if (start < pool->vaddr || start > pool->vaddr + pool->size)
+	if (start < pool->vaddr || start >= pool->vaddr + pool->size)
 		return 0;

 	if (start + size > pool->vaddr + pool->size) {
--
1.7.9.5
Best regards
-- 
Marek Szyprowski
Samsung Poland R&D Center
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help