linux-kernel - Re: [PATCH] mm:zswap: fix zswap entry reclamation failure in two scenarios

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Thu, 16 Nov 2023 16:31:09 +0800
From:   "Huang, Ying" <ying.huang@...el.com>
To:     Yosry Ahmed <yosryahmed@...gle.com>
Cc:     Zhongkun He <hezhongkun.hzk@...edance.com>,
        akpm@...ux-foundation.org, hannes@...xchg.org, nphamcs@...il.com,
        sjenning@...hat.com, ddstreet@...e.org, vitaly.wool@...sulko.com,
        linux-mm@...ck.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] mm:zswap: fix zswap entry reclamation failure in two
 scenarios

Yosry Ahmed <yosryahmed@...gle.com> writes:

> +Ying
>
> On Mon, Nov 13, 2023 at 5:06 AM Zhongkun He
> <hezhongkun.hzk@...edance.com> wrote:
>>
>> I recently found two scenarios where zswap entry could not be
>> released, which will cause shrink_worker and active recycling
>> to fail.
>> 1)The swap entry has been freed, but cached in swap_slots_cache,
>> no swap cache and swapcount=0.
>> 2)When the option zswap_exclusive_loads_enabled disabled and
>> zswap_load completed(page in swap_cache and swapcount = 0).
>
> For case (1), I think a cleaner solution would be to move the
> zswap_invalidate() call from swap_range_free() (which is called after
> the cached slots are freed) to __swap_entry_free_locked() if the usage
> goes to 0. I actually think conceptually this makes not just for
> zswap_invalidate(), but also for the arch call, memcg uncharging, etc.
> Slots caching is a swapfile optimization that should be internal to
> swapfile code. Once a swap entry is freed (i.e. swap count is 0 AND
> not in the swap cache), all the hooks should be called (memcg, zswap,
> arch, ..) as the swap entry is effectively freed. The fact that
> swapfile code internally batches and caches slots should be
> transparent to other parts of MM. I am not sure if the calls can just
> be moved or if there are underlying assumptions in the implementation
> that would be broken, but it feels like the right thing to do.

This sounds reasonable for me.  Just don't forget to change other
swap_range_free() callers too.

--
Best Regards,
Huang, Ying