netdev - Re: [REGRESSION] Recent swiotlb DMA_FROM_DEVICE fixes break ath9k-based AP

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, 24 Mar 2022 19:31:58 +0100
From:   Halil Pasic <pasic@...ux.ibm.com>
To:     Robin Murphy <robin.murphy@....com>
Cc:     Christoph Hellwig <hch@....de>, Maxime Bizon <mbizon@...ebox.fr>,
        Toke Høiland-Jørgensen <toke@...e.dk>,
        Oleksandr Natalenko <oleksandr@...alenko.name>,
        Linus Torvalds <torvalds@...ux-foundation.org>,
        Marek Szyprowski <m.szyprowski@...sung.com>,
        Kalle Valo <kvalo@...nel.org>,
        "David S. Miller" <davem@...emloft.net>,
        Jakub Kicinski <kuba@...nel.org>,
        Paolo Abeni <pabeni@...hat.com>,
        Olha Cherevyk <olha.cherevyk@...il.com>,
        iommu <iommu@...ts.linux-foundation.org>,
        linux-wireless <linux-wireless@...r.kernel.org>,
        Netdev <netdev@...r.kernel.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        stable <stable@...r.kernel.org>,
        Halil Pasic <pasic@...ux.ibm.com>
Subject: Re: [REGRESSION] Recent swiotlb DMA_FROM_DEVICE fixes break
 ath9k-based AP

On Thu, 24 Mar 2022 16:52:31 +0000
Robin Murphy <robin.murphy@....com> wrote:

> > Creating a new mapping for the same buffer before unmapping the
> > previous one does looks rather bogus.  But it does not fit the
> > pattern where revering the sync_single changes make the driver
> > work again.  
> 
> OK, you made me look :)
> 
> Now that it's obvious what to look for, I can only conclude that during 
> the stanza in ath_edma_get_buffers(), the device is still writing to the 
> buffer while ownership has been transferred to the CPU, and whatever got 
> written while ath9k_hw_process_rxdesc_edma() was running then gets wiped 
> out by the subsequent sync_for_device, which currently resets the 
> SWIOTLB slot to the state that sync_for_cpu copied out. By the letter of 
> the DMA API that's not allowed, but on the other hand I'm not sure if we 
> even have a good idiom for "I can't tell if the device has finished with 
> this buffer or not unless I look at it" :/

I agree with your analysis. Especially with the latter part (were you
state that we don't have a good idiom for that use case). 

I believe, a stronger statement is also true: it is fundamentally
impossible to accommodate use cases where the device and the cpu need
concurrent access to a dma buffer, if the dma buffer isn't in dma
coherent memory.

If the dma buffer is in dma coherent memory, and we don't need swiotlb,
then we don't need the dma_sync functionality. 

Specifically for swiotlb, if the swiotlb buffer is in dma coherent
memory, the driver could peek the swiotlb buffer, but I have no idea if
we can provide a remotely sane API for that. The point is the device
would have peek not via a pointer to the original buffer, but get
suitable pointer to the bounce buffer, which would be probably be
considered valid, as long as the mapping is valid. Honestly IMHO quite
ugly but I see no other way. 

Regards,
Halil