[PATCH] mm/zswap: flush dcache in the compressed decompression path

[PATCH] mm/zswap: flush dcache in the compressed decompression path

[Song Hu]

Both branches of zswap_decompress() write the destination folio through a
kernel mapping: the incompressible branch via kmap_local_folio() and the
compressed branch via the crypto scatterwalk. Yet only the incompressible
branch calls flush_dcache_folio(); the compressed branch was missed. On
aliasing D-cache architectures this can leave stale cache lines visible to
userland after a zswap load.

zram flushes in the same situation (zram_bio_read()). Hoist a single
flush_dcache_folio() out of the incompressible branch so both paths are
covered.

Fixes: e2c3b6b21c77 ("mm: zswap: use SG list decompression APIs from zsmalloc")
Signed-off-by: Song Hu <husong@kylinos.cn>
---
 mm/zswap.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/mm/zswap.c b/mm/zswap.c
index b5a17ea20237..bb0c16d102d5 100644
--- a/mm/zswap.c
+++ b/mm/zswap.c
@@ -943,7 +943,6 @@ static bool zswap_decompress(struct zswap_entry *entry, struct folio *folio)
 		memcpy_from_sglist(dst, input, 0, PAGE_SIZE);
 		dlen = PAGE_SIZE;
 		kunmap_local(dst);
-		flush_dcache_folio(folio);
 	} else {
 		sg_init_table(&output, 1);
 		sg_set_folio(&output, folio, PAGE_SIZE, 0);
@@ -954,6 +953,9 @@ static bool zswap_decompress(struct zswap_entry *entry, struct folio *folio)
 		dlen = acomp_ctx->req->dlen;
 	}
 
+	/* Both branches write the folio through a kernel alias. */
+	flush_dcache_folio(folio);
+
 	zs_obj_read_sg_end(pool->zs_pool, entry->handle);
 	mutex_unlock(&acomp_ctx->mutex);
 
-- 
2.43.0

Re: [PATCH] mm/zswap: flush dcache in the compressed decompression path

[Nhat Pham]

On Fri, Jul 10, 2026 at 1:28 AM Song Hu <husong@kylinos.cn> wrote:
>
> Both branches of zswap_decompress() write the destination folio through a
> kernel mapping: the incompressible branch via kmap_local_folio() and the
> compressed branch via the crypto scatterwalk. Yet only the incompressible
> branch calls flush_dcache_folio(); the compressed branch was missed. On
> aliasing D-cache architectures this can leave stale cache lines visible to
> userland after a zswap load.
>
> zram flushes in the same situation (zram_bio_read()). Hoist a single
> flush_dcache_folio() out of the incompressible branch so both paths are
> covered.
>
> Fixes: e2c3b6b21c77 ("mm: zswap: use SG list decompression APIs from zsmalloc")

I'll leave other maintainers take a closer look at this in terms of correctness.

I'll just ask one question: was the bug (if real) introduced by this
patch, or has it been problematic before this?

Seems like this patch changes the input end of decompression, but does
not actually touch the output (folio) end?

I managed to trace this all the way to 1ec3b5f ("mm/zswap: move to use
crypto_acomp API for hardware acceleration" +Barry), which introduces
the SG/crypto API, but not entirely sure if it was broken here
already? Or maybe even before this?

Yosry and Barry, WDYT?

Re: [PATCH] mm/zswap: flush dcache in the compressed decompression path

[Yosry Ahmed]

On Fri, Jul 10, 2026 at 1:28 AM Song Hu <husong@kylinos.cn> wrote:
>
> Both branches of zswap_decompress() write the destination folio through a
> kernel mapping: the incompressible branch via kmap_local_folio() and the
> compressed branch via the crypto scatterwalk. Yet only the incompressible
> branch calls flush_dcache_folio(); the compressed branch was missed. On
> aliasing D-cache architectures this can leave stale cache lines visible to
> userland after a zswap load.

Adding Herbert here.

Doesn't the Crypto API take care of this for the decompression path?
Maybe in scomp_acomp_comp_decomp()?

>
> zram flushes in the same situation (zram_bio_read()). Hoist a single
> flush_dcache_folio() out of the incompressible branch so both paths are
> covered.
>
> Fixes: e2c3b6b21c77 ("mm: zswap: use SG list decompression APIs from zsmalloc")
> Signed-off-by: Song Hu <husong@kylinos.cn>
> ---
>  mm/zswap.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/mm/zswap.c b/mm/zswap.c
> index b5a17ea20237..bb0c16d102d5 100644
> --- a/mm/zswap.c
> +++ b/mm/zswap.c
> @@ -943,7 +943,6 @@ static bool zswap_decompress(struct zswap_entry *entry, struct folio *folio)
>                 memcpy_from_sglist(dst, input, 0, PAGE_SIZE);
>                 dlen = PAGE_SIZE;
>                 kunmap_local(dst);
> -               flush_dcache_folio(folio);
>         } else {
>                 sg_init_table(&output, 1);
>                 sg_set_folio(&output, folio, PAGE_SIZE, 0);
> @@ -954,6 +953,9 @@ static bool zswap_decompress(struct zswap_entry *entry, struct folio *folio)
>                 dlen = acomp_ctx->req->dlen;
>         }
>
> +       /* Both branches write the folio through a kernel alias. */
> +       flush_dcache_folio(folio);
> +
>         zs_obj_read_sg_end(pool->zs_pool, entry->handle);
>         mutex_unlock(&acomp_ctx->mutex);
>
> --
> 2.43.0
>

Re: [PATCH] mm/zswap: flush dcache in the compressed decompression path

[Joshua Hahn]

On Fri, 10 Jul 2026 11:22:43 -0700 Yosry Ahmed <yosry@kernel.org> wrote:

> On Fri, Jul 10, 2026 at 1:28 AM Song Hu <husong@kylinos.cn> wrote:
> >
> > Both branches of zswap_decompress() write the destination folio through a
> > kernel mapping: the incompressible branch via kmap_local_folio() and the
> > compressed branch via the crypto scatterwalk. Yet only the incompressible
> > branch calls flush_dcache_folio(); the compressed branch was missed. On
> > aliasing D-cache architectures this can leave stale cache lines visible to
> > userland after a zswap load.
> 
> Adding Herbert here.
> 
> Doesn't the Crypto API take care of this for the decompression path?
> Maybe in scomp_acomp_comp_decomp()?

This is what I think too. Actually if you use something like
zstd_decompress() as well, it ends up calling:
acomp_walk_done_dst() --> scatterwalk_done_dst()
		      --> __scatterwalk_flush_dcache_pages()
		      --> flush_dcache_page()
over all the pages in the loop.

Maybe I'm missing a path here that would require us to manually flush
from zswap? But even still, I think that should be handled by the
path that is repsonsible for doing the actual decompression, not
zswap.

What do you think?
Joshua

Re: [PATCH] mm/zswap: flush dcache in the compressed decompression path

[Yosry Ahmed]

On Fri, Jul 10, 2026 at 11:40 AM Joshua Hahn <joshua.hahnjy@gmail.com> wrote:
>
> On Fri, 10 Jul 2026 11:22:43 -0700 Yosry Ahmed <yosry@kernel.org> wrote:
>
> > On Fri, Jul 10, 2026 at 1:28 AM Song Hu <husong@kylinos.cn> wrote:
> > >
> > > Both branches of zswap_decompress() write the destination folio through a
> > > kernel mapping: the incompressible branch via kmap_local_folio() and the
> > > compressed branch via the crypto scatterwalk. Yet only the incompressible
> > > branch calls flush_dcache_folio(); the compressed branch was missed. On
> > > aliasing D-cache architectures this can leave stale cache lines visible to
> > > userland after a zswap load.
> >
> > Adding Herbert here.
> >
> > Doesn't the Crypto API take care of this for the decompression path?
> > Maybe in scomp_acomp_comp_decomp()?
>
> This is what I think too. Actually if you use something like
> zstd_decompress() as well, it ends up calling:
> acomp_walk_done_dst() --> scatterwalk_done_dst()
>                       --> __scatterwalk_flush_dcache_pages()
>                       --> flush_dcache_page()
> over all the pages in the loop.
>
> Maybe I'm missing a path here that would require us to manually flush
> from zswap? But even still, I think that should be handled by the
> path that is repsonsible for doing the actual decompression, not
> zswap.
>
> What do you think?

Yeah I am hoping Herbert will help us out here.

Re: [PATCH] mm/zswap: flush dcache in the compressed decompression path

[Zenghui Yu]

[+Cc Herbert, in case you missed that]

On 7/11/26 2:43 AM, Yosry Ahmed wrote:
> On Fri, Jul 10, 2026 at 11:40 AM Joshua Hahn <joshua.hahnjy@gmail.com> wrote:
> >
> > On Fri, 10 Jul 2026 11:22:43 -0700 Yosry Ahmed <yosry@kernel.org> wrote:
> >
> > > On Fri, Jul 10, 2026 at 1:28 AM Song Hu <husong@kylinos.cn> wrote:
> > > >
> > > > Both branches of zswap_decompress() write the destination folio through a
> > > > kernel mapping: the incompressible branch via kmap_local_folio() and the
> > > > compressed branch via the crypto scatterwalk. Yet only the incompressible
> > > > branch calls flush_dcache_folio(); the compressed branch was missed. On
> > > > aliasing D-cache architectures this can leave stale cache lines visible to
> > > > userland after a zswap load.
> > >
> > > Adding Herbert here.
> > >
> > > Doesn't the Crypto API take care of this for the decompression path?
> > > Maybe in scomp_acomp_comp_decomp()?
> >
> > This is what I think too. Actually if you use something like
> > zstd_decompress() as well, it ends up calling:
> > acomp_walk_done_dst() --> scatterwalk_done_dst()
> >                       --> __scatterwalk_flush_dcache_pages()
> >                       --> flush_dcache_page()
> > over all the pages in the loop.
> >
> > Maybe I'm missing a path here that would require us to manually flush
> > from zswap? But even still, I think that should be handled by the
> > path that is repsonsible for doing the actual decompression, not
> > zswap.
> >
> > What do you think?
> 
> Yeah I am hoping Herbert will help us out here.
>