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  PHC 
Open Source and information security mailing list archives
Hash Suite for Android: free password hash cracker in your pocket
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:   Sat, 26 Mar 2022 11:38:59 -0700
From:   Linus Torvalds <>
To:     Toke Høiland-Jørgensen <>
Cc:     Halil Pasic <>,
        Robin Murphy <>,
        Maxime Bizon <>,
        Netdev <>, Kalle Valo <>,
        linux-wireless <>,
        Oleksandr Natalenko <>,
        stable <>,
        "David S. Miller" <>,
        iommu <>,
        Olha Cherevyk <>,
        Greg Kroah-Hartman <>,
        Jakub Kicinski <>,
        Paolo Abeni <>,
        Christoph Hellwig <>,
        Linux Kernel Mailing List <>
Subject: Re: [REGRESSION] Recent swiotlb DMA_FROM_DEVICE fixes break
 ath9k-based AP

On Sat, Mar 26, 2022 at 9:06 AM Toke Høiland-Jørgensen <> wrote:
> I was also toying with the idea of having a copy-based peek helper like:
> u32 data = dma_peek_word(buf, offset)

I really don't think you can or want to have a word-based one.

That said, I like the *name* of that thing.

I think a lot of confusion comes from the very subtle naming of
fundamentally having a lot of odd conditions with

 - two different "directions of the sync" - ie who it is that cares:


 - three different "direction of the data" - ie who it is that writes the data:


so you have six possible combinations, three of which seem insane and
not useful, and of the three that are actually possible, some are very
unusual (it exactly that "device is the one writing, but we want to
sync the dma area for the device").

I do not think it helps that not only do we have this combinatorial
naming, we also use _different_ names. We say "for device" and "for
cpu", but then when we specify who does the writing, we don't say "cpu
vs device", we just specify the direction instead (FROM_DEVICE means
the device did the writing, TO_DEVICE means that the CPU did the

Anyway, I spent a lot of time looking at this, and I am now personally
convinced that commit aa6f8dcbab47 (swiotlb: rework "fix info leak
with DMA_FROM_DEVICE") was just completely buggy, and was buggy
exactly becasue it was fundamentally confused even about which
direction the bounce was happening.

I have reverted it in my tree, and I tried to write a comprehensive
summary about why it was wrong.

What I *didn't* do in that commit was to argue against the naming, and
try to enumerate all the different valid cases.

Because I think naming matters, and I think the current dma_sync()
interfaces are horribly confusing exactly due to those naming

But I think "peek" is a good name, not because I think reading one
work is a valid thing (you want to often peek more than that), but
because it seems much more intuitive than

Similarly, I would think that "flush" is a much better word for

I don't know what a good word for
"dma_sync_for_device(DMA_FROM_DEVICE)" is, but maybe "forget" would
come closest - we want the CPU to "forget" what it peeked.

Anyway, I have reverted that commit, and I think it was wrong both in
spirit and in implementation, and I'll ask Greg to remove it from

And I think the security argument was entirely bogus, because the
whole security argument was based on an incorrect understanding of the
direction of the data.

But hey, I may currently be convinced that revert is the right thing
to do, BUT I've been wrong before, and I'll happily change my mind if
somebody makes a really cogent argument


Powered by blists - more mailing lists