linux-kernel - Re: scatterlist: sg_set_buf() argument must be in linear mapping (sha1: ac4e97abce9b80c020e7113325f49e58b7b15e3f)

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] [day] [month] [year] [list]

Message-ID: <51EE7319.408@monstr.eu>
Date:	Tue, 23 Jul 2013 14:12:09 +0200
From:	Michal Simek <monstr@...str.eu>
To:	Russell King - ARM Linux <linux@....linux.org.uk>
CC:	Rusty Russell <rusty@...tcorp.com.au>,
	Jens Axboe <axboe@...nel.dk>,
	LKML <linux-kernel@...r.kernel.org>,
	Ohad Ben-Cohen <ohad@...ery.com>,
	linux-arm-kernel@...ts.infradead.org
Subject: Re: scatterlist: sg_set_buf() argument must be in linear mapping
 (sha1: ac4e97abce9b80c020e7113325f49e58b7b15e3f)

On 07/23/2013 12:37 PM, Russell King - ARM Linux wrote:
> On Tue, Jul 23, 2013 at 12:00:30PM +0930, Rusty Russell wrote:
>> Michal Simek <monstr@...str.eu> writes:
>>> Let me take some code from virtio_rpmsg_bus.c to show that problematic part.
>>>
>>> bufs_va = dma_alloc_coherent(vdev->dev.parent->parent,
>>> 				RPMSG_TOTAL_BUF_SPACE,
>>> 				&vrp->bufs_dma, GFP_KERNEL);
>>> vrp->rbufs = bufs_va;
>>> for (i = 0; i < RPMSG_NUM_BUFS / 2; i++) {
>>> 	struct scatterlist sg;
>>> 	void *cpu_addr = vrp->rbufs + i * RPMSG_BUF_SIZE;
>>>
>>> 	sg_init_one(&sg, cpu_addr, RPMSG_BUF_SIZE);
>>> }
>>
>> Hmm.  Looking at arch/arm/include/asm/memory.h:
>>
>> #define virt_to_page(kaddr)	pfn_to_page(__pa(kaddr) >> PAGE_SHIFT)
>> #define virt_addr_valid(kaddr)	((unsigned long)(kaddr) >= PAGE_OFFSET && (unsigned long)(kaddr) < (unsigned long)high_memory)
>>
>> pfn_to_page in ARM seems to be the asm-generic one, which depends on
>> CONFIG_SPARSEMEM/CONFIG_DISCONTIGMEM/CONFIG_FLATMEM etc.
>>
>> Perhaps virt_addr_valid() is wrong for your config?  It's pretty clear
>> that you shouldn't call virt_to_page() on something for which
>> !virt_addr_valid().
> 
> The above code fragment is just wrong.  You can't make any assumptions about
> the memory returned from dma_alloc_coherent(), because:
> 
> - On x86, it is kernel direct mapped memory, where things like
>   virt_to_phys(cpuaddr) will work fine with it.
> 
> - On any architecture which needs to remap memory to make it coherent
>   with the DMA device, the key word there is "remap" - it's not kernel
>   direct mapped memory, and virt_to_phys(cpuaddr) on it is illegal.
> 
> The only valid operation with scatterlists and such memory returned from
> dma_alloc_coherent() is:
> 
> 	sg_dma_address(sg) = dma_address;
> 	sg_dma_len(sg) = dma_length;
> 
> and not to use the virtual address(es) at all in the scatterlist.  And
> it is very very important to realise that you must not mix the streaming
> and coherent DMA APIs - you must never pass coherent memory into the
> dma_map_* functions.

Ohad: This code is probably yours and you know the whole history of it.
Can you please comment it?

Thanks,
Michal

-- 
Michal Simek, Ing. (M.Eng), OpenPGP -> KeyID: FE3D1F91
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel - Microblaze cpu - http://www.monstr.eu/fdt/
Maintainer of Linux kernel - Xilinx Zynq ARM architecture
Microblaze U-BOOT custodian and responsible for u-boot arm zynq platform



Download attachment "signature.asc" of type "application/pgp-signature" (264 bytes)