linux-kernel - Re: [v4,1/3] drm/prime: use dma length macro when mapping sg

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Fri, 27 Mar 2020 09:11:52 +0100
From:   Christian König <christian.koenig@....com>
To:     Marek Szyprowski <m.szyprowski@...sung.com>,
        Shane Francis <bigbeeshane@...il.com>,
        dri-devel@...ts.freedesktop.org
Cc:     airlied@...ux.ie, linux-kernel@...r.kernel.org,
        amd-gfx-request@...ts.freedesktop.org, alexander.deucher@....com
Subject: Re: [v4,1/3] drm/prime: use dma length macro when mapping sg

Am 27.03.20 um 08:54 schrieb Marek Szyprowski:
> Hi All,
>
> On 2020-03-25 10:07, Shane Francis wrote:
>> As dma_map_sg can reorganize scatter-gather lists in a
>> way that can cause some later segments to be empty we should
>> always use the sg_dma_len macro to fetch the actual length.
>>
>> This could now be 0 and not need to be mapped to a page or
>> address array
>>
>> Signed-off-by: Shane Francis <bigbeeshane@...il.com>
>> Reviewed-by: Michael J. Ruhl <michael.j.ruhl@...el.com>
> This patch landed in linux-next 20200326 and it causes a kernel panic on
> various Exynos SoC based boards.
>> ---
>>    drivers/gpu/drm/drm_prime.c | 2 +-
>>    1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c
>> index 86d9b0e45c8c..1de2cde2277c 100644
>> --- a/drivers/gpu/drm/drm_prime.c
>> +++ b/drivers/gpu/drm/drm_prime.c
>> @@ -967,7 +967,7 @@ int drm_prime_sg_to_page_addr_arrays(struct sg_table *sgt, struct page **pages,
>>    
>>    	index = 0;
>>    	for_each_sg(sgt->sgl, sg, sgt->nents, count) {
>> -		len = sg->length;
>> +		len = sg_dma_len(sg);
>>    		page = sg_page(sg);
>>    		addr = sg_dma_address(sg);
>>    
> Sorry, but this code is wrong :(

Well it is at least better than before because it makes most drivers 
work correctly again.

See we only fill the pages array because some drivers (like Exynos) are 
still buggy and require this.

Accessing the pages of an DMA-buf imported sg_table is illegal and 
should be fixed in the drivers.

> [SNIP]
>
> I will send a patch in a few minutes with the above fixed code.

That is certainly a good idea for now, but could we also put on 
somebodies todo list an item to fix Exynos?

Thanks in advance,
Christian.

>
> Best regards