linux-kernel - Re: [PATCH 1/3] mtd: spi-nor: add optional DMA-safe bounce buffer for data transfer

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:   Tue, 2 Jan 2018 15:30:37 +0530
From:   Vignesh R <vigneshr@...com>
To:     Trent Piepho <tpiepho@...inj.com>,
        "linux-mtd@...ts.infradead.org" <linux-mtd@...ts.infradead.org>,
        "linux@...linux.org.uk" <linux@...linux.org.uk>,
        "broonie@...nel.org" <broonie@...nel.org>,
        "cyrille.pitchen@...ev4u.fr" <cyrille.pitchen@...ev4u.fr>,
        "dwmw2@...radead.org" <dwmw2@...radead.org>,
        "computersforpeace@...il.com" <computersforpeace@...il.com>,
        "boris.brezillon@...e-electrons.com" 
        <boris.brezillon@...e-electrons.com>,
        "richard@....at" <richard@....at>,
        "marek.vasut@...il.com" <marek.vasut@...il.com>
CC:     "linux-spi@...r.kernel.org" <linux-spi@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "nicolas.ferre@...rochip.com" <nicolas.ferre@...rochip.com>,
        "radu.pirea@...rochip.com" <radu.pirea@...rochip.com>,
        "robh@...nel.org" <robh@...nel.org>,
        "devicetree@...r.kernel.org" <devicetree@...r.kernel.org>
Subject: Re: [PATCH 1/3] mtd: spi-nor: add optional DMA-safe bounce buffer for
 data transfer



On Friday 29 December 2017 11:33 PM, Trent Piepho wrote:
> On Fri, 2017-12-29 at 15:46 +0530, Vignesh R wrote:
>> On Friday 29 December 2017 12:24 AM, Trent Piepho wrote:
>> > 
>> > > Vignesh has suggested to call virt_addr_valid() instead.
>> > > I think Boris has also told me about this function.
>> > > So it might be the right solution. What do you think about their proposal?
>> > 
>> > Not sure what exactly the differences are between these methods.  The
>> > fact that each of the many existing DMA fixes uses slightly different
>> > code to detect what is unsafe speaks to the difficulty of this problem!
>> 
>> My understanding based on Documentation/DMA-API-HOWTO.txt and
>> Documentation/arm/memory.txt is that
>> virt_addr_valid() will guarantee that address is in range of
>> PAGE_OFFSET to high_memory-1 (Kernel direct-mapped RAM region) which is
>> address range of buffers that are DMA'able.
> 
> There's code in gpmi-nand.c that does:
> 
>         /* first try to map the upper buffer directly */
>         if (virt_addr_valid(this->upper_buf) &&
>                 !object_is_on_stack(this->upper_buf)) {
>                 sg_init_one(sgl, this->upper_buf, this->upper_len);
> 
> So whoever wrote that thought that stack objects needed an additional
> test beyond virt_addr_valid.  But it does appear to be far more common
> to depend on just virt_addr_valid, so perhaps the code in gpmi-nand is
> in error.
> 

The test in gpmi-nand.c is right. Enabling CONFIG_DMA_API_DEBUG does
warn about DMA'ing into stack objects (in lib/dma-debug.c). So other
places needs to be fixed, I guess.

>> >  virt_addr_valid() is already used by spi-ti-qspi.  spi core uses for
>> > the buffer map helper, but that code path is for buffers which are NOT
>> > vmalloc or highmem, but are still not virt_addr_valid() for some other
>> > reason.
>> > 
>> 
>>        if (vmalloced_buf || kmap_buf) {
>>                /* Handle vmalloc'd or kmap'd buffers */
>>                ...
> This stuff does get DMAed.  So I have to wonder, if spi.c thinks it can
> use DMA with vmalloc or highmem, couldn't spi-not do the same instead
> of the bounce buffer?

SPI core does try to DMA into underlying physical pages of vmalloc'd
buffer. But this has two problems:

1. Does not work well with VIVT caches[1].
2. On ARM LPAE systems, vmalloc'd buffers can be from highmem region
that are not addressable using 32 bit addresses and is backed by LPAE.
So, a 32 bit DMA cannot access these buffers.

Both these issues lead to random crashes and errors with UBIFS and JFFS2
flash file systems which this patch series tries to address using bounce
buffer

[1] https://patchwork.kernel.org/patch/9579553/


-- 
Regards
Vignesh