Re: [PATCH mm-hotfixes] mm/zswap: add missing kunmap_local()

[Lorenzo Stoakes (Oracle)]

On Tue, Mar 17, 2026 at 10:01:19AM +0000, Lorenzo Stoakes (Oracle) wrote:
> On Mon, Mar 16, 2026 at 02:01:22PM +0000, Lorenzo Stoakes (Oracle) wrote:
> > Commit e2c3b6b21c77 ("mm: zswap: use SG list decompression APIs from
> > zsmalloc") updated zswap_decompress() to use the scatterwalk API to copy
> > data for uncompressed pages.
> >
> > In doing so, it mapped kernel memory locally for 32-bit kernels using
> > kmap_local_folio(), however it never unmapped this memory.
> >
> > This resulted in the linked syzbot report where a BUG_ON() is triggered due
> > to leaking the kmap slot.
> >
> > This patch fixes the issue by explicitly unmapping the established kmap.
> >
> > Reported-by: syzbot+fe426bef95363177631d@xxxxxxxxxxxxxxxxxxxxxxxxx
> > Closes: https://lore.kernel.org/all/69b75e2c.050a0220.12d28.015a.GAE@xxxxxxxxxx
> > Fixes: e2c3b6b21c77 ("mm: zswap: use SG list decompression APIs from zsmalloc")
> > Signed-off-by: Lorenzo Stoakes (Oracle) <ljs@xxxxxxxxxx>
> > ---
> >  mm/zswap.c | 7 ++++++-
> >  1 file changed, 6 insertions(+), 1 deletion(-)
> >
> > diff --git a/mm/zswap.c b/mm/zswap.c
> > index e6ec3295bdb0..499520f65ff0 100644
> > --- a/mm/zswap.c
> > +++ b/mm/zswap.c
> > @@ -942,9 +942,14 @@ static bool zswap_decompress(struct zswap_entry *entry, struct folio *folio)
> >
> >  	/* zswap entries of length PAGE_SIZE are not compressed. */
> >  	if (entry->length == PAGE_SIZE) {
> > +		void *dst;
> > +
> >  		WARN_ON_ONCE(input->length != PAGE_SIZE);
> > -		memcpy_from_sglist(kmap_local_folio(folio, 0), input, 0, PAGE_SIZE);
> > +
> > +		dst = kmap_local_folio(folio, 0);
> > +		memcpy_from_sglist(dst, input, 0, PAGE_SIZE);
> >  		dlen = PAGE_SIZE;
> > +		kunmap_local(dst);
>
> FYI to address (in advance) the AI review from [0] which a couple people made me
> aware of - we don't need a flush_dcache_folio() here, because the folio is not
> yet accessible by userspace, so we can't have virtual aliasing of the folio's
> physical address on VIVT architectures.
>
> Examining call paths:
>
> zswap_writeback_entry() -> only calls zswap_decompress() if allocated
> -> zswap_decompress()
>
> swap_vma_readahead() -> only calls swap_read_folio() if allocated
> swap_cluster_readahead() -> only calls swap_read_folio() if allocated
> read_swap_cache_async() ->  only calls swap_read_folio() if allocated
> do_swap_page() -> called in path where folio allocated
> shmem_swap_alloc_folio() -> as name implies, allocated folio
> -> swap_read_folio()
> -> zswap_load()
> -> zswap_decompress()
>
> So actually no longer doing this is a de-pessimisation ;)

On second thoughts, I'm wrong, this is required. The documentation is really not
clear on this, which is a problem given how horribly fiddly this is.

https://docs.kernel.org/core-api/cachetlb.html it super vague on
update_mmu_cache_range() -

"At the end of every page fault, this routine is invoked to tell the
architecture specific code that translations now exists in the software page
tables for address space “vma->vm_mm” at virtual address “address” for “nr”
consecutive pages."

It doesn't mention that you really need to have invoked dcache flushes in
advance for anything you changed, and that the architecture may then choose to
not do anything at this point otherwise.

Anyway, I'll send a fix-patch.

Cheers, Lorenzo