Re: [PATCH] zsmalloc: zero-initialize zspage memory to prevent KMSAN uninit reads

[Yosry Ahmed]

On Tue, May 12, 2026 at 7:47 PM Sergey Senozhatsky
<senozhatsky@xxxxxxxxxxxx> wrote:
>
> Adding Yosry and Herbert,
>
> On (26/05/12 14:47), Andrew Morton wrote:
> > > Pages allocated via alloc_zpdesc() use alloc_pages_node() without
> > > __GFP_ZERO, leaving physical memory uninitialized. When a compressed
> > > object spans two physical pages in a zspage, zs_obj_read_sg_begin()
> > > sets up a scatterlist pointing directly at the raw second page. If the
> > > second page was freshly allocated and never written beyond the object
> > > boundary, KMSAN detects reads of uninitialized memory downstream in
> > > the decompressor (e.g. sw842_decompress reading the CRC trailer).
>
> I don't get this.  How can sw842_decompress() read more bytes than
> it's told to decompress.  We first compress and store the object,
> before we load and decompress, reading past the known compressed
> object size (which we pass to decompress function) should not happen.
> Yosry, Herbert, any ideas?

I agree it's weird that sw842_decompress() is reading past the
compressed length. I didn't look at the code but maybe it's ready in
chunks of N bytes (e.g. 8 bytes) so it's reading a few bytes past the
end of the compressed length?

Not sure if we should add __GFP_ZERO in zsmalloc just in case, or if
that can hide other problems.

>
> > > Fix this by passing __GFP_ZERO to alloc_zpdesc() in alloc_zspage() so
> > > all pages backing a zspage are zero-initialized at allocation time.
> > >
> > > Reported-by: syzbot+8f77ff6144a73f0cf71b@xxxxxxxxxxxxxxxxxxxxxxxxx
> > > Closes: https://syzkaller.appspot.com/bug?extid=8f77ff6144a73f0cf71b
> > > Signed-off-by: Kartik Nair <contact.kartikn@xxxxxxxxx>
> >
> > Thanks.
> >
> > > --- a/mm/zsmalloc.c
> > > +++ b/mm/zsmalloc.c
> > > @@ -951,7 +951,7 @@ static struct zspage *alloc_zspage(struct zs_pool *pool,
> > >     for (i = 0; i < class->pages_per_zspage; i++) {
> > >             struct zpdesc *zpdesc;
> > >
> > > -           zpdesc = alloc_zpdesc(gfp, nid);
> > > +           zpdesc = alloc_zpdesc(gfp | __GFP_ZERO, nid);
> > >             if (!zpdesc) {
> > >                     while (--i >= 0) {
> > >                             zpdesc_dec_zone_page_state(zpdescs[i]);
> >
> > Decompressing uninitialized memory sounds rather bad, so I'll add a
> > cc:stable to this.
> [..]
> > I think the Fixes: target is 56e5a103a721 ("zsmalloc: prefer the the
> > original page's node for compressed data").  Can people please check
> > this when reviewing?
>
> If we need to pick a zsmalloc commit, then this probably can be related
> to zs_obj_read_sg_begin() API (which is used only by zswap at the moemnt),
> so probably the Fixes: tag be dc2e4982cb018 (zsmalloc: introduce SG-list
> based object read API).

Agreed.