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

From: Colin Ian King
Date: Tue Aug 23 2016 - 05:53:22 EST


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