linux-kernel - Re: [PATCH 5.10 11/38] swiotlb: rework "fix info leak with DMA_FROM

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]

Message-ID: <YkFwEWy9Pt0v5KLr@kroah.com>
Date:   Mon, 28 Mar 2022 10:21:37 +0200
From:   Greg Kroah-Hartman <gregkh@...uxfoundation.org>
To:     Linus Torvalds <torvalds@...ux-foundation.org>
Cc:     Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        stable <stable@...r.kernel.org>,
        Halil Pasic <pasic@...ux.ibm.com>,
        Christoph Hellwig <hch@....de>
Subject: Re: [PATCH 5.10 11/38] swiotlb: rework "fix info leak with
 DMA_FROM_DEVICE"

On Sun, Mar 27, 2022 at 11:02:12AM -0700, Linus Torvalds wrote:
> On Sun, Mar 27, 2022 at 2:30 AM Greg Kroah-Hartman
> <gregkh@...uxfoundation.org> wrote:
> >
> > But why did you just revert that commit, and not the previous one (i.e.
> > the one that this one "fixes")?  Shouldn't ddbd89deb7d3 ("swiotlb: fix
> > info leak with DMA_FROM_DEVICE") also be dropped?
> 
> The previous one wasn't obviously broken, and while it's a bit ugly,
> it doesn't have the fundamental issues that the "fix" commit had.
> 
> And it does fix the whole "bounce buffer contents are undefined, and
> can get copied back later" at the bounce buffer allocation (well,
> "mapping") stage.
> 
> Which could cause wasted CPU cycles and isn't great, but should fix
> the stale content thing for at least the common "map DMA, do DMA,
> unmap" situation.
> 
> What commit aa6f8dcbab47 tried to fix was the "do multiple DMA
> sequences using one single mapping" case, but that's also what then
> broke ath9k because it really does want to do exactly that, but it
> very much needs to do it using the same buffer with no "let's reset
> it".
> 
> So I think you're fine to drop ddbd89deb7d3 too, but that commit
> doesn't seem *wrong* per se.
> 
> I do think we need some model for "clear the bounce buffer of stale
> data", and I do think that commit ddbd89deb7d3 probably isn't the
> final word, but we don't actually _have_ the final word on this all,
> so stable dropping it all is sane.
> 
> But as mentioned, commit ddbd89deb7d3 can actually fix some cases.
> 
> In particular, I do think it fixes the SG_IO data leak case that
> triggered the whole issue. It was just then the "let's expand on this
> fix" that was a disaster.

Ok, I have just queued that one up now for the older kernels, and the
revert for 5.15 and newer, thanks.

greg k-h