Re: [PATCH] zswap: initialize entry->pool on same filled entry

[Chris Li <chrisl@kernel.org>]

On Thu, Mar 21, 2024 at 4:56 PM Yosry Ahmed <yosryahmed@google.com> wrote:
>
> On Thu, Mar 21, 2024 at 4:53 PM Chris Li <chrisl@kernel.org> wrote:
> >
> > Current zswap will leave the entry->pool uninitialized if
> > the page is same  filled. The entry->pool pointer can
> > contain data written by previous usage.
> >
> > Initialize entry->pool to zero for the same filled zswap entry.
> >
> > Signed-off-by: Chris Li <chrisl@kernel.org>
> > ---
> > Per Yosry's suggestion to split out this clean up
> > from the zxwap rb tree to xarray patch.
> >
> > https://lore.kernel.org/all/ZemDuW25YxjqAjm-@google.com/
> > ---
> >  mm/zswap.c | 1 +
> >  1 file changed, 1 insertion(+)
> >
> > diff --git a/mm/zswap.c b/mm/zswap.c
> > index b31c977f53e9..f04a75a36236 100644
> > --- a/mm/zswap.c
> > +++ b/mm/zswap.c
> > @@ -1527,6 +1527,7 @@ bool zswap_store(struct folio *folio)
> >                         kunmap_local(src);
> >                         entry->length = 0;
> >                         entry->value = value;
> > +                       entry->pool = 0;
>
> This should be NULL.
>
> That being said, I am working on a series that should make non-filled
> entries not use a zswap_entry at all. So I think this cleanup is
> unnecessary, especially that it is documented in the definition of
> struct zswap_entry that entry->pool is invalid for same-filled
> entries.

It does not really hurt to initialize it. It is obviously correct if
we initialize it as well. One thing to consider is that, this pointer
can contain user space data if the page previously was map to user
space. Kdump typically doesn't save user space data. This
uninitialized value might let kdump contain user space data.

Chris

>
> >                         atomic_inc(&zswap_same_filled_pages);
> >                         goto insert_entry;
> >                 }
> >
> > ---
> > base-commit: a824831a082f1d8f9b51a4c0598e633d38555fcf
> > change-id: 20240315-zswap-fill-f65f44574760
> >
> > Best regards,
> > --
> > Chris Li <chrisl@kernel.org>
> >