[PATCH] arch/arm/mm/dma-mapping.c: fix __alloc_from_pool returning a dirty buffer

[Robin Murphy]

On 03/01/2023 9:51 am, Russell King (Oracle) wrote:
> On Fri, Dec 23, 2022 at 11:51:43PM +0100, Igor Klochko wrote:
>> Thanks Christoph,
>>
>> Added fixes and a changelog.
>> This issue is present in all current LTS versions.
>>
>> ----
>> Buffers allocated by __alloc_from_pool() should be zeroed out as done by other allocators.
>> Certain drivers expect a clean buffer and clearing the buffer is beneficial from the security point of view.
>> ---
>> Fixes: 36d0fd2198da3 (*arm: use genalloc for the atomic pool*)
>> Signed-off-by: Igor Klochko <igor.klochko at gmail.com>
>> ---
>>   arch/arm/mm/dma-mapping.c | 1 +
>>   1 file changed, 1 insertion(+)
>>
>> diff --git a/arch/arm/mm/dma-mapping.c b/arch/arm/mm/dma-mapping.c
>> index c135f6e37a00..bb2bb3ab497a 100644
>> --- a/arch/arm/mm/dma-mapping.c
>> +++ b/arch/arm/mm/dma-mapping.c
>> @@ -366,6 +366,7 @@ static void *__alloc_from_pool(size_t size, struct page **ret_page)
>>   
>>   		*ret_page = phys_to_page(phys);
>>   		ptr = (void *)val;
>> +		memset(ptr, 0, size);
> 
> I'm not up to speed with the current structure of the DMA implementation
> on ARM, but isn't this going to make cache lines dirty for the returned
> buffer, which will corrupt DMA on non-coherent devices?

The virtual address of the pool, and thus VAs within the pool as 
returned by gen_pool_alloc(), should be a non-cacheable remap set up by 
atomic_pool_init(), so it looks appropriate to me. In fact "ptr" here 
appears to be the final return value propagated all the way back out to 
the caller of dma_alloc_coherent(), so if that were dodgy then there 
would already be problems afoot.

Robin.