Skip to contentSkip to conversation

dmaengine: do not allow access outside of unmap_pool

PATCH Xulin Sun 2 participants 2 messages Aug 23, 2016

lore
Share & tools
lore

Xulin Sun

author
#0 lore raw
Quoted 29 lines · Colin King
Quoted 18 lines

From: Colin Ian King <[email protected]>

When CONFIG_DMA_ENGINE_RAID is defined, unmap_pool[] is just 1 element in size, however, allows orders of 2..8 to access outside unmap_pool and returns an invalid address. Ensure we fall into the default path and report a BUG() when CONFIG_DMA_ENGINE_RAID is defined and order is out of range.

Signed-off-by Colin Ian King <[email protected]>
---
 drivers/dma/dmaengine.c | 2 ++
 1 file changed, 2 insertions(+)
drivers/dma/dmaengine.c +0 -0
index 8c9f45f..6027e66 100644

drivers/dma/dmaengine.c static struct dmaengine_unmap_pool *__get_unmap_pool(int nr) · line 1103

Quoted 4 lines
drivers/dma/dmaengine.c +1 -0
@@ -1100,12 +1100,14 @@ static struct dmaengine_unmap_pool *__get_unmap_pool(int nr)
     switch (order) {
     case 0 ... 1:
         return &unmap_pool[0];
+    #if IS_ENABLED(CONFIG_DMA_ENGINE_RAID)

Okay if CONFIG_DMA_ENGINE_RAID is enabled (m or y) then IS_ENABLED return 1, so we will go inside and not fall into default. And I though by changelog that you want it to go to default in CONFIG_DMA_ENGINE_RAID is defined!

Quoted 1 line

What did I miss...

Here it should be when CONFIG_DMA_ENGINE_RAID is NOT defined, unmap_pool[] is just 1 element in size, and the function "__get_unmap_pool" will access outside of the array unmap_pool[] in case orders of 2..8 and returns an invalid address, and I encountered the issue.

I think the patch is needed to avoid visiting outside of the array unmap_pool[] if CONFIG_DMA_ENGINE_RAID is NOT defined.

Thanks
Xulin

Quoted 13 lines
Quoted 13 lines
drivers/dma/dmaengine.c +1 -1
     case 2 ... 4:
         return &unmap_pool[1];
     case 5 ... 7:
         return &unmap_pool[2];
     case 8:
         return &unmap_pool[3];
+    #endif
     default:
         BUG();
         return NULL;
--

2.8.1

#1 lore raw
Quoted 39 lines · Xulin Sun
Quoted 14 lines · Colin King
Quoted 13 lines

From: Colin Ian King <[email protected]>

When CONFIG_DMA_ENGINE_RAID is defined, unmap_pool[] is just 1 element in size, however, allows orders of 2..8 to access outside unmap_pool and returns an invalid address. Ensure we fall into the default path and report a BUG() when CONFIG_DMA_ENGINE_RAID is defined and order is out of range.

Signed-off-by Colin Ian King <[email protected]>
---
 drivers/dma/dmaengine.c | 2 ++
 1 file changed, 2 insertions(+)
Quoted 5 lines

drivers/dma/dmaengine.c static struct dmaengine_unmap_pool

Quoted 5 lines
drivers/dma/dmaengine.c +0 -0
index 8c9f45f..6027e66 100644
@@ -1100,12 +1100,14 @@ static struct dmaengine_unmap_pool

*__get_unmap_pool(int nr)

Quoted 11 lines

drivers/dma/dmaengine.c static struct dmaengine_unmap_pool · line 1103

Quoted 4 lines
drivers/dma/dmaengine.c +1 -0
     switch (order) {
     case 0 ... 1:
         return &unmap_pool[0];
+    #if IS_ENABLED(CONFIG_DMA_ENGINE_RAID)

Okay if CONFIG_DMA_ENGINE_RAID is enabled (m or y) then IS_ENABLED return 1, so we will go inside and not fall into default. And I though by changelog that you want it to go to default in CONFIG_DMA_ENGINE_RAID is defined!

What did I miss...

Here it should be when CONFIG_DMA_ENGINE_RAID is NOT defined, unmap_pool[] is just 1
element in size, and the function "__get_unmap_pool" will access outside of the array unmap_pool[]
in case orders of 2..8 and returns an invalid address, and I encountered the issue.

Quoted 2 lines

I think the patch is needed to avoid visiting outside of the array unmap_pool[] if CONFIG_DMA_ENGINE_RAID is NOT defined.

Exactly. Thanks for explaining, I missed the original query, apologies for missing that.

Colin

Quoted 16 lines

Thanks
Xulin

Quoted 13 lines
Quoted 13 lines
drivers/dma/dmaengine.c +1 -1
     case 2 ... 4:
         return &unmap_pool[1];
     case 5 ... 7:
         return &unmap_pool[2];
     case 8:
         return &unmap_pool[3];
+    #endif
     default:
         BUG();
         return NULL;
--

2.8.1