linux-kernel - Re: [PATCH v5] mm/zswap: store <PAGE_SIZE compression failed page as-is

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] [day] [month] [year] [list]

Message-Id: <20250828163913.57957-1-sj@kernel.org>
Date: Thu, 28 Aug 2025 09:39:13 -0700
From: SeongJae Park <sj@...nel.org>
To: Chris Li <chrisl@...nel.org>
Cc: SeongJae Park <sj@...nel.org>,
	Andrew Morton <akpm@...ux-foundation.org>,
	Chengming Zhou <chengming.zhou@...ux.dev>,
	Herbert Xu <herbert@...dor.apana.org.au>,
	Johannes Weiner <hannes@...xchg.org>,
	Nhat Pham <nphamcs@...il.com>,
	Yosry Ahmed <yosry.ahmed@...ux.dev>,
	kernel-team@...a.com,
	linux-kernel@...r.kernel.org,
	linux-mm@...ck.org,
	Takero Funaki <flintglass@...il.com>,
	David Hildenbrand <david@...hat.com>,
	Baoquan He <bhe@...hat.com>,
	Barry Song <baohua@...nel.org>,
	Kairui Song <kasong@...cent.com>
Subject: Re: [PATCH v5] mm/zswap: store <PAGE_SIZE compression failed page as-is

On Wed, 27 Aug 2025 14:16:37 -0700 Chris Li <chrisl@...nel.org> wrote:

> On Wed, Aug 27, 2025 at 10:45 AM SeongJae Park <sj@...nel.org> wrote:
[...]
> Anyway, I just opened the editor to check again. Yes, if we goto the
> read_done, the if condition using dlen can introduce a new incoming
> edge that has len uninitialized value to be used. Yes, need to
> initialize dlen = PAGE_SIZE or you initialize it at the memcpy of
> page.

Thank you for confirming.

> 
> > I will post the fixup by tomorrow morning (Pacific time) unless I
> > hear other opinions or find my mistakes on the above plan by tonight.
> 
> You are too humble, that is the normal reviewing process. You can take
> your time.

No worry, I just wanted to give you an expectation of the time line :)

Andrew, could you please pick the below attached cleanup patch as a fixup of
the original patch?  Please feel free to let me know if you prefer posting a
new version of the patch instead or any other approach.


Thanks,
SJ

[...]

==== Attachment 0 (0001-mm-zswap-cleanup-incompressible-pages-handling-code.patch) ====
>From 867c3789fa80ac163427f1d7804bf2c8667684ca Mon Sep 17 00:00:00 2001
From: SeongJae Park <sj@...nel.org>
Date: Wed, 27 Aug 2025 13:18:38 -0700
Subject: [PATCH] mm/zswap: cleanup incompressible pages handling code

Following Chris Li's suggestions[1], make the code easier to read and
manage.

[1] https://lore.kernel.org/CACePvbWGPApYr7G29FzbmWzRw-BJE39WH7kUHSaHs+Lnw8=-qQ@mail.gmail.com

Signed-off-by: SeongJae Park <sj@...nel.org>
Acked-by: Chris Li <chrisl@...nel.org>
---
 mm/zswap.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/mm/zswap.c b/mm/zswap.c
index f865a930dc88..e5e1f5687f5e 100644
--- a/mm/zswap.c
+++ b/mm/zswap.c
@@ -952,6 +952,7 @@ static bool zswap_compress(struct page *page, struct zswap_entry *entry,
 	struct zpool *zpool;
 	gfp_t gfp;
 	u8 *dst;
+	bool mapped = false;
 
 	acomp_ctx = acomp_ctx_get_cpu_lock(pool);
 	dst = acomp_ctx->buffer;
@@ -983,9 +984,8 @@ static bool zswap_compress(struct page *page, struct zswap_entry *entry,
 	 * only adds metadata overhead.  swap_writeout() will put the page back
 	 * to the active LRU list in the case.
 	 */
-	if (comp_ret || !dlen)
+	if (comp_ret || !dlen || dlen >= PAGE_SIZE) {
 		dlen = PAGE_SIZE;
-	if (dlen >= PAGE_SIZE) {
 		if (!mem_cgroup_zswap_writeback_enabled(
 					folio_memcg(page_folio(page)))) {
 			comp_ret = comp_ret ? comp_ret : -EINVAL;
@@ -994,6 +994,7 @@ static bool zswap_compress(struct page *page, struct zswap_entry *entry,
 		comp_ret = 0;
 		dlen = PAGE_SIZE;
 		dst = kmap_local_page(page);
+		mapped = true;
 	}
 
 	zpool = pool->zpool;
@@ -1007,7 +1008,7 @@ static bool zswap_compress(struct page *page, struct zswap_entry *entry,
 	entry->length = dlen;
 
 unlock:
-	if (dst != acomp_ctx->buffer)
+	if (mapped)
 		kunmap_local(dst);
 	if (comp_ret == -ENOSPC || alloc_ret == -ENOSPC)
 		zswap_reject_compress_poor++;
@@ -1025,7 +1026,7 @@ static bool zswap_decompress(struct zswap_entry *entry, struct folio *folio)
 	struct zpool *zpool = entry->pool->zpool;
 	struct scatterlist input, output;
 	struct crypto_acomp_ctx *acomp_ctx;
-	int decomp_ret, dlen;
+	int decomp_ret = 0, dlen = PAGE_SIZE;
 	u8 *src, *obj;
 
 	acomp_ctx = acomp_ctx_get_cpu_lock(entry->pool);
@@ -1034,9 +1035,7 @@ 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) {
 		memcpy_to_folio(folio, 0, obj, entry->length);
-		zpool_obj_read_end(zpool, entry->handle, obj);
-		acomp_ctx_put_unlock(acomp_ctx);
-		return true;
+		goto read_done;
 	}
 
 	/*
@@ -1059,6 +1058,7 @@ static bool zswap_decompress(struct zswap_entry *entry, struct folio *folio)
 	decomp_ret = crypto_wait_req(crypto_acomp_decompress(acomp_ctx->req), &acomp_ctx->wait);
 	dlen = acomp_ctx->req->dlen;
 
+read_done:
 	zpool_obj_read_end(zpool, entry->handle, obj);
 	acomp_ctx_put_unlock(acomp_ctx);
 
-- 
2.39.5