public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH] dmaengine: do not allow access outside of unmap_pool
@ 2016-05-17 12:00 Colin King
  2016-06-07  6:05 ` Vinod Koul
  0 siblings, 1 reply; 4+ messages in thread
From: Colin King @ 2016-05-17 12:00 UTC (permalink / raw)
  To: Dan Williams, Vinod Koul, dmaengine; +Cc: linux-kernel

From: Colin Ian King <colin.king@canonical.com>

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 <colin.king@canonical.com>
---
 drivers/dma/dmaengine.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/dma/dmaengine.c b/drivers/dma/dmaengine.c
index 8c9f45f..6027e66 100644
--- a/drivers/dma/dmaengine.c
+++ b/drivers/dma/dmaengine.c
@@ -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)
 	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

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

* Re: [PATCH] dmaengine: do not allow access outside of unmap_pool
  2016-05-17 12:00 [PATCH] dmaengine: do not allow access outside of unmap_pool Colin King
@ 2016-06-07  6:05 ` Vinod Koul
  0 siblings, 0 replies; 4+ messages in thread
From: Vinod Koul @ 2016-06-07  6:05 UTC (permalink / raw)
  To: Colin King; +Cc: Dan Williams, dmaengine, linux-kernel

On Tue, May 17, 2016 at 01:00:46PM +0100, Colin King wrote:
> From: Colin Ian King <colin.king@canonical.com>
> 
> 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 <colin.king@canonical.com>
> ---
>  drivers/dma/dmaengine.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/dma/dmaengine.c b/drivers/dma/dmaengine.c
> index 8c9f45f..6027e66 100644
> --- a/drivers/dma/dmaengine.c
> +++ b/drivers/dma/dmaengine.c
> @@ -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!

What did I miss...

>  	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
> 

-- 
~Vinod

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

* Re: [PATCH] dmaengine: do not allow access outside of unmap_pool
@ 2016-08-23  9:24 Xulin Sun
  2016-08-23  9:51 ` Colin Ian King
  0 siblings, 1 reply; 4+ messages in thread
From: Xulin Sun @ 2016-08-23  9:24 UTC (permalink / raw)
  To: vinod.koul, colin.king, dmaengine; +Cc: Xulin Sun, linux-kernel

 >On Tue, May 17, 2016 at 01:00:46PM +0100, Colin King wrote:
 >> From: Colin Ian King <colin.king@canonical.com>
 >>
 >> 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 <colin.king@canonical.com>
 >> ---
 >>  drivers/dma/dmaengine.c | 2 ++
 >>  1 file changed, 2 insertions(+)
 >>
 >> diff --git a/drivers/dma/dmaengine.c b/drivers/dma/dmaengine.c
 >> index 8c9f45f..6027e66 100644
 >> --- a/drivers/dma/dmaengine.c
 >> +++ b/drivers/dma/dmaengine.c
 >> @@ -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!

 >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
 >>      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
 >>

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

* Re: [PATCH] dmaengine: do not allow access outside of unmap_pool
  2016-08-23  9:24 Xulin Sun
@ 2016-08-23  9:51 ` Colin Ian King
  0 siblings, 0 replies; 4+ messages in thread
From: Colin Ian King @ 2016-08-23  9:51 UTC (permalink / raw)
  To: 1463486446-13890-1-git-send-email-colin.king, vinod.koul,
	dmaengine
  Cc: Xulin Sun, linux-kernel

On 23/08/16 10:24, Xulin Sun wrote:
>>On Tue, May 17, 2016 at 01:00:46PM +0100, Colin King wrote:
>>> From: Colin Ian King <colin.king@canonical.com>
>>>
>>> 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 <colin.king@canonical.com>
>>> ---
>>>  drivers/dma/dmaengine.c | 2 ++
>>>  1 file changed, 2 insertions(+)
>>>
>>> diff --git a/drivers/dma/dmaengine.c b/drivers/dma/dmaengine.c
>>> index 8c9f45f..6027e66 100644
>>> --- a/drivers/dma/dmaengine.c
>>> +++ b/drivers/dma/dmaengine.c
>>> @@ -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!
> 
>>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.

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

Colin
> 
> Thanks
> Xulin
>>>      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
>>>

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

end of thread, other threads:[~2016-08-23  9:53 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-05-17 12:00 [PATCH] dmaengine: do not allow access outside of unmap_pool Colin King
2016-06-07  6:05 ` Vinod Koul
  -- strict thread matches above, loose matches on Subject: below --
2016-08-23  9:24 Xulin Sun
2016-08-23  9:51 ` Colin Ian King

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox