linux-kernel - Re: [PATCH v1 1/1] ARM: Select DMA_DIRECT

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:   Thu, 28 Sep 2023 12:20:56 -0400
From:   "Arnd Bergmann" <arnd@...db.de>
To:     "Robin Murphy" <robin.murphy@....com>,
        "Jim Quinlan" <james.quinlan@...adcom.com>
Cc:     "Linus Walleij" <linus.walleij@...aro.org>,
        "Christoph Hellwig" <hch@....de>,
        bcm-kernel-feedback-list@...adcom.com, jim2101024@...il.com,
        "Russell King" <linux@...linux.org.uk>,
        "Geert Uytterhoeven" <geert+renesas@...der.be>,
        "Russell King" <rmk+kernel@...linux.org.uk>,
        "Andrew Morton" <akpm@...ux-foundation.org>,
        "Jonathan Corbet" <corbet@....net>,
        "Thomas Gleixner" <tglx@...utronix.de>,
        "Sebastian Reichel" <sebastian.reichel@...labora.com>,
        "Mike Rapoport" <rppt@...nel.org>,
        "Eric DeVolder" <eric.devolder@...cle.com>,
        "Nathan Chancellor" <nathan@...nel.org>,
        "Kirill A. Shutemov" <kirill.shutemov@...ux.intel.com>,
        "Christophe Leroy" <christophe.leroy@...roup.eu>,
        "moderated list:ARM PORT" <linux-arm-kernel@...ts.infradead.org>,
        "open list" <linux-kernel@...r.kernel.org>,
        "Claire Chang" <tientzu@...omium.org>
Subject: Re: [PATCH v1 1/1] ARM: Select DMA_DIRECT_REMAP to fix restricted DMA

On Thu, Sep 28, 2023, at 11:33, Robin Murphy wrote:
> On 28/09/2023 4:16 pm, Arnd Bergmann wrote:
>
>> It's unlikely but not impossible, as the driver has some
>> unusual constructs, using a lot of coherent mappings that
>> might otherwise be streaming mappings, and relying on
>> dma_sync_single_for_device(..., DMA_BIDIRECTIONAL) for other
>> data, but without the corresponding dma_sync_single_for_cpu().
>> If all the testing happens on x86, this might easily lead
>> to a bug that only shows up on non-coherent systems but
>> is never seen during testing.
>
> Probably the significant thing about restricted DMA is that it forces 
> all streaming DMA to be bounce-buffered. That should expose busted 
> synchronisation even more decisively than a lack of coherency. If 
> there's no IOMMU, then testing the driver in the absence of restricted 
> DMA but with "swiotlb=force" should confirm or disprove that.

I see this sequence in the iwlwifi driver, in the
iwl_save_fw_paging() function:

       block = alloc_pages(GFP_KERNEL, order);
       phys = dma_map_page(dev, block, 0,
               PAGE_SIZE << order, DMA_BIDIRECTIONAL);
       memcpy(page_address(block), ...);
       dma_sync_single_for_device(dev, phys, size, DMA_BIDIRECTIONAL);

Which clearly violates the interface by writing into
a page that is already owned by the device, without
giving it back to the cpu first. Not sure if or how this
would explain actual data corruption on armv7, since we
write back the buffers in both the map and sync operations
and never invalidate the cache, but the driver also doesn't
ever read from the buffer (despite it being bidirectional).
If it's not this problem, there is a good chance of others.

     Arnd