Re: [PATCH] dmaengine: do not allow access outside of unmap_pool

[Colin Ian King]

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 
>>>
>>> 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 
>>> ---
>>>  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 _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 _pool[1];
>>>  case 5 ... 7:
>>>  return _pool[2];
>>>  case 8:
>>>  return _pool[3];
>>> +#endif
>>>  default:
>>>  BUG();
>>>  return NULL;
>>> --
>>> 2.8.1
>>>

Re: [PATCH] dmaengine: do not allow access outside of unmap_pool

[Colin Ian King]

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 
>>>
>>> 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 
>>> ---
>>>  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 _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 _pool[1];
>>>  case 5 ... 7:
>>>  return _pool[2];
>>>  case 8:
>>>  return _pool[3];
>>> +#endif
>>>  default:
>>>  BUG();
>>>  return NULL;
>>> --
>>> 2.8.1
>>>

Re: [PATCH] dmaengine: do not allow access outside of unmap_pool

[Xulin Sun]

>On Tue, May 17, 2016 at 01:00:46PM +0100, Colin King wrote:
>> From: Colin Ian King 
>>
>> 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 
>> ---
>>  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 _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 _pool[1];
>>  case 5 ... 7:
>>  return _pool[2];
>>  case 8:
>>  return _pool[3];
>> +#endif
>>  default:
>>  BUG();
>>  return NULL;
>> --
>> 2.8.1
>>

Re: [PATCH] dmaengine: do not allow access outside of unmap_pool

[Xulin Sun]

>On Tue, May 17, 2016 at 01:00:46PM +0100, Colin King wrote:
>> From: Colin Ian King 
>>
>> 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 
>> ---
>>  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 _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 _pool[1];
>>  case 5 ... 7:
>>  return _pool[2];
>>  case 8:
>>  return _pool[3];
>> +#endif
>>  default:
>>  BUG();
>>  return NULL;
>> --
>> 2.8.1
>>

Re: [PATCH] dmaengine: do not allow access outside of unmap_pool

[Vinod Koul]

On Tue, May 17, 2016 at 01:00:46PM +0100, Colin King wrote:
> From: Colin Ian King 
> 
> 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 
> ---
>  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 _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 _pool[1];
>   case 5 ... 7:
>   return _pool[2];
>   case 8:
>   return _pool[3];
> + #endif
>   default:
>   BUG();
>   return NULL;
> -- 
> 2.8.1
> 

-- 
~Vinod

Re: [PATCH] dmaengine: do not allow access outside of unmap_pool

[Vinod Koul]

On Tue, May 17, 2016 at 01:00:46PM +0100, Colin King wrote:
> From: Colin Ian King 
> 
> 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 
> ---
>  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 _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 _pool[1];
>   case 5 ... 7:
>   return _pool[2];
>   case 8:
>   return _pool[3];
> + #endif
>   default:
>   BUG();
>   return NULL;
> -- 
> 2.8.1
> 

-- 
~Vinod

[PATCH] dmaengine: do not allow access outside of unmap_pool

[Colin King]

From: Colin Ian King 

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 
---
 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 _pool[0];
+   #if IS_ENABLED(CONFIG_DMA_ENGINE_RAID)
case 2 ... 4:
return _pool[1];
case 5 ... 7:
return _pool[2];
case 8:
return _pool[3];
+   #endif
default:
BUG();
return NULL;
-- 
2.8.1

[PATCH] dmaengine: do not allow access outside of unmap_pool

[Colin King]

From: Colin Ian King 

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 
---
 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 _pool[0];
+   #if IS_ENABLED(CONFIG_DMA_ENGINE_RAID)
case 2 ... 4:
return _pool[1];
case 5 ... 7:
return _pool[2];
case 8:
return _pool[3];
+   #endif
default:
BUG();
return NULL;
-- 
2.8.1