Re: [PATCH 5.10 11/38] swiotlb: rework "fix info leak with DMA_FROM_DEVICE"

[Greg Kroah-Hartman <gregkh@linuxfoundation.org>]

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@linuxfoundation.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