linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
From: Andrew Morton <akpm@linux-foundation.org>
To: chengming.zhou@linux.dev
Cc: hannes@cmpxchg.org, yosryahmed@google.com, nphamcs@gmail.com,
	linux-mm@kvack.org, linux-kernel@vger.kernel.org,
	Chengming Zhou <zhouchengming@bytedance.com>,
	stable@vger.kernel.org, Chris Li <chrisl@kernel.org>
Subject: Re: [PATCH v4] mm/zswap: invalidate old entry when store fail or !zswap_enabled
Date: Wed, 7 Feb 2024 15:43:08 -0800	[thread overview]
Message-ID: <20240207154308.bc275f3e72ec1c1fd06cf5a2@linux-foundation.org> (raw)
In-Reply-To: <20240207115406.3865746-1-chengming.zhou@linux.dev>

On Wed,  7 Feb 2024 11:54:06 +0000 chengming.zhou@linux.dev wrote:

> From: Chengming Zhou <zhouchengming@bytedance.com>
> 
> We may encounter duplicate entry in the zswap_store():
> 
> 1. swap slot that freed to per-cpu swap cache, doesn't invalidate
>    the zswap entry, then got reused. This has been fixed.
> 
> 2. !exclusive load mode, swapin folio will leave its zswap entry
>    on the tree, then swapout again. This has been removed.
> 
> 3. one folio can be dirtied again after zswap_store(), so need to
>    zswap_store() again. This should be handled correctly.
> 
> So we must invalidate the old duplicate entry before insert the
> new one, which actually doesn't have to be done at the beginning
> of zswap_store(). And this is a normal situation, we shouldn't
> WARN_ON(1) in this case, so delete it. (The WARN_ON(1) seems want
> to detect swap entry UAF problem? But not very necessary here.)
> 
> The good point is that we don't need to lock tree twice in the
> store success path.
> 
> Note we still need to invalidate the old duplicate entry in the
> store failure path, otherwise the new data in swapfile could be
> overwrite by the old data in zswap pool when lru writeback.
> 
> We have to do this even when !zswap_enabled since zswap can be
> disabled anytime. If the folio store success before, then got
> dirtied again but zswap disabled, we won't invalidate the old
> duplicate entry in the zswap_store(). So later lru writeback
> may overwrite the new data in swapfile.
> 
> Fixes: 42c06a0e8ebe ("mm: kill frontswap")
> Cc: <stable@vger.kernel.org>

We have a patch ordering issue.

As a cc:stable hotfix, this should be merged into 6.8-rcX and later
backported into -stable trees.  So it will go
mm-hotfixes-unstable->mm-hotfixes-stable->mainline.  So someone has to
make this patch merge and work against latest mm-hotfixes-unstable.

The patch you sent appears to be based on linux-next, so it has
dependencies upon mm-unstable patches which won't be merged into
mainline until the next merge window.

So can you please redo and retest this against mm.git's
mm-hotfixes-unstable branch?  Then I'll try to figure out how to merge
the gigentic pile of mm-unstable zswap changes on top of that.

Thanks.


  parent reply	other threads:[~2024-02-07 23:43 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-02-07 11:54 chengming.zhou
2024-02-07 23:06 ` Nhat Pham
2024-02-08  2:34   ` Chengming Zhou
2024-02-07 23:43 ` Andrew Morton [this message]
2024-02-08  2:32   ` [PATCH mm-hotfixes-unstable] mm/zswap: invalidate duplicate entry when !zswap_enabled chengming.zhou
2024-02-08 13:14     ` Johannes Weiner
2024-02-08 21:09     ` Andrew Morton
2024-02-09  4:50       ` Chengming Zhou
2024-02-08  2:41   ` [PATCH v4] mm/zswap: invalidate old entry when store fail or !zswap_enabled Chengming Zhou
2024-02-09  4:41 ` [PATCH mm-unstable] mm/zswap: optimize and cleanup the invalidation of duplicate entry chengming.zhou

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20240207154308.bc275f3e72ec1c1fd06cf5a2@linux-foundation.org \
    --to=akpm@linux-foundation.org \
    --cc=chengming.zhou@linux.dev \
    --cc=chrisl@kernel.org \
    --cc=hannes@cmpxchg.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=nphamcs@gmail.com \
    --cc=stable@vger.kernel.org \
    --cc=yosryahmed@google.com \
    --cc=zhouchengming@bytedance.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox