* 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 [PATCH] dmaengine: do not allow access outside of unmap_pool 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
* [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 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
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-08-23 9:24 [PATCH] dmaengine: do not allow access outside of unmap_pool Xulin Sun
2016-08-23 9:51 ` Colin Ian King
-- strict thread matches above, loose matches on Subject: below --
2016-05-17 12:00 Colin King
2016-06-07 6:05 ` Vinod Koul
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox