[PATCH] mm/zswap: validate dlen and normalize comp_ret on compression success

[PATCH] mm/zswap: validate dlen and normalize comp_ret on compression success

[Suseendran Bhaskaran]

In zswap_compress(), dlen is only meaningful when compression succeeds
(!comp_ret), as compressors do not consistently set req->dlen on error
(e.g. zstd, 842, and qat set dlen = 0; lz4, lz4hc, lzo, lzo-rle,
deflate, and iaa leave dlen at PAGE_SIZE; hisi_zip sets dlen to the
hardware-reported produced bytes).

When !comp_ret and dlen == PAGE_SIZE with zswap writeback disabled,
zswap_compress() sets comp_ret to -EINVAL instead of -ENOSPC,
miscounting the page under reject_compress_fail instead of
reject_compress_poor.

When !comp_ret, set comp_ret = -ENOSPC if dlen == PAGE_SIZE, or
comp_ret = -EINVAL if WARN_ON_ONCE(!dlen || dlen > PAGE_SIZE). This
allows simplifying the failure check to 'if (comp_ret)' and dropping
the ternary assignment in the writeback-disabled path.

Fixes: dca4437a5861 ("mm/zswap: store <PAGE_SIZE compression failed page as-is")
Signed-off-by: Suseendran Bhaskaran <suseendranb@google.com>
---
Previously sent as [PATCH 2/2] of:
https://lore.kernel.org/all/20261021205626.3286270-3-suseendranb@google.com/

Changes since v1:
- Split from patch 1/2 (which is going through independently).
- Keep existing writeback-enabled behavior for comp_ret != 0.
- Normalize comp_ret under !comp_ret and check 'if (comp_ret)'.

 mm/zswap.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/mm/zswap.c b/mm/zswap.c
index 37f34e406c8e..1e727abcf9fb 100644
--- a/mm/zswap.c
+++ b/mm/zswap.c
@@ -873,6 +873,12 @@ static bool zswap_compress(struct page *page, struct zswap_entry *entry,
 	 */
 	comp_ret = crypto_wait_req(crypto_acomp_compress(acomp_ctx->req), &acomp_ctx->wait);
 	dlen = acomp_ctx->req->dlen;
+	if (!comp_ret) {
+		if (dlen == PAGE_SIZE)
+			comp_ret = -ENOSPC;
+		else if (WARN_ON_ONCE(!dlen || dlen > PAGE_SIZE))
+			comp_ret = -EINVAL;
+	}
 
 	/*
 	 * If a page cannot be compressed into a size smaller than PAGE_SIZE,
@@ -881,12 +887,11 @@ 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 || dlen >= PAGE_SIZE) {
+	if (comp_ret) {
 		rcu_read_lock();
 		if (!mem_cgroup_zswap_writeback_enabled(
 					folio_memcg(page_folio(page)))) {
 			rcu_read_unlock();
-			comp_ret = comp_ret ? comp_ret : -EINVAL;
 			goto unlock;
 		}
 		rcu_read_unlock();
-- 
2.56.0.257.gf7497c7d27-goog

Re: [PATCH] mm/zswap: validate dlen and normalize comp_ret on compression success

[Yosry Ahmed]

On Thu, Oct 22, 2026 at 4:27 PM Suseendran Bhaskaran
<suseendranb@google.com> wrote:
>
> In zswap_compress(), dlen is only meaningful when compression succeeds
> (!comp_ret), as compressors do not consistently set req->dlen on error
> (e.g. zstd, 842, and qat set dlen = 0; lz4, lz4hc, lzo, lzo-rle,
> deflate, and iaa leave dlen at PAGE_SIZE; hisi_zip sets dlen to the
> hardware-reported produced bytes).
>
> When !comp_ret and dlen == PAGE_SIZE with zswap writeback disabled,
> zswap_compress() sets comp_ret to -EINVAL instead of -ENOSPC,
> miscounting the page under reject_compress_fail instead of
> reject_compress_poor.
>
> When !comp_ret, set comp_ret = -ENOSPC if dlen == PAGE_SIZE, or
> comp_ret = -EINVAL if WARN_ON_ONCE(!dlen || dlen > PAGE_SIZE). This
> allows simplifying the failure check to 'if (comp_ret)' and dropping
> the ternary assignment in the writeback-disabled path.
>
> Fixes: dca4437a5861 ("mm/zswap: store <PAGE_SIZE compression failed page as-is")
> Signed-off-by: Suseendran Bhaskaran <suseendranb@google.com>

Acked-by: Yosry Ahmed <yosry@kernel.org>

Re: [PATCH] mm/zswap: validate dlen and normalize comp_ret on compression success

[Andrew Morton]

On Thu, 22 Oct 2026 23:27:33 +0000 Suseendran Bhaskaran <suseendranb@google.com> wrote:

> In zswap_compress(), dlen is only meaningful when compression succeeds
> (!comp_ret), as compressors do not consistently set req->dlen on error
> (e.g. zstd, 842, and qat set dlen = 0; lz4, lz4hc, lzo, lzo-rle,
> deflate, and iaa leave dlen at PAGE_SIZE; hisi_zip sets dlen to the
> hardware-reported produced bytes).
> 
> When !comp_ret and dlen == PAGE_SIZE with zswap writeback disabled,
> zswap_compress() sets comp_ret to -EINVAL instead of -ENOSPC,
> miscounting the page under reject_compress_fail instead of
> reject_compress_poor.
> 
> When !comp_ret, set comp_ret = -ENOSPC if dlen == PAGE_SIZE, or
> comp_ret = -EINVAL if WARN_ON_ONCE(!dlen || dlen > PAGE_SIZE). This
> allows simplifying the failure check to 'if (comp_ret)' and dropping
> the ternary assignment in the writeback-disabled path.

Sounds like we're complexicating core code in order to accommodate
inconsistent compressor behavior.  Would it be better to

a) design the compressor interface(!)
b) document it
c) ensure that all compressors comply with it?

Maintainers may choose to go ahead with this as a backportable
quickfix, but it all seems to be a bit of a mess in there.


Wht you forgot to tell us (please never forget this) is the
userspace-visible runtime effects of the bug.  Autobot says:

: The userspace-visible effects are limited to corrected zswap
: statistics and a new warning for invalid compressor results.
: 
: When compression succeeds but produces exactly PAGE_SIZE bytes, and
: zswap writeback is disabled, the page is rejected as before. However,
: reject_compress_poor now increments instead of reject_compress_fail.
: This makes the debugfs statistics distinguish incompressible pages
: from actual compressor failures more accurately.
: 
: A compressor which reports success with a zero or oversized output
: length will now trigger WARN_ON_ONCE(). Previously, zswap handled
: these results without a warning. On systems with panic_on_warn
: enabled, this could cause a kernel panic.
: 
: There is no intended change to which pages zswap accepts, rejects
: or stores uncompressed, nor to normal application behavior.
: 
: I think these runtime effects should be described more prominently
: in the changelog, particularly the new warning.



My autobot is all worried about the HiSilicon ZIP compressor.

: I think the zswap-side fix is reasonable. On success, dlen should
: contain the actual output length, and zswap should validate it. On
: failure, dlen isn't necessarily meaningful, so we should rely on the
: compressor's error code.
: 
: Separately, I think we should make the compressors consistently return
: -ENOSPC when the output buffer is too small. Some currently return
: -EINVAL, which means reject_compress_fail versus reject_compress_poor
: will remain inaccurate even with this patch.
: 
: A compressor which successfully produces exactly PAGE_SIZE bytes
: should still report success. Deciding that the result isn't worth
: storing belongs in zswap.
: 
: One thing worth checking is HiSilicon ZIP's negative-compression
: status. It is treated as success, with dlen taken from the hardware.
: Could that produce zero or more than PAGE_SIZE bytes? If so, the new
: WARN_ON_ONCE() might trigger on ordinary incompressible data.
: 
: I'd take this fix independently of the compressor cleanup, assuming
: that last case is safe.

Re: [PATCH] mm/zswap: validate dlen and normalize comp_ret on compression success

[Suseendran Bhaskaran]

On Fri, 23 Oct 2026 10:26:07 -0700 Andrew Morton <akpm@linux-foundation.org> wrote:

> On Thu, 22 Oct 2026 23:27:33 +0000 Suseendran Bhaskaran <suseendranb@google.com> wrote:
>
> > In zswap_compress(), dlen is only meaningful when compression succeeds
> > (!comp_ret), as compressors do not consistently set req->dlen on error
> > (e.g. zstd, 842, and qat set dlen = 0; lz4, lz4hc, lzo, lzo-rle,
> > deflate, and iaa leave dlen at PAGE_SIZE; hisi_zip sets dlen to the
> > hardware-reported produced bytes).
> >
> > When !comp_ret and dlen == PAGE_SIZE with zswap writeback disabled,
> > zswap_compress() sets comp_ret to -EINVAL instead of -ENOSPC,
> > miscounting the page under reject_compress_fail instead of
> > reject_compress_poor.
> >
> > When !comp_ret, set comp_ret = -ENOSPC if dlen == PAGE_SIZE, or
> > comp_ret = -EINVAL if WARN_ON_ONCE(!dlen || dlen > PAGE_SIZE). This
> > allows simplifying the failure check to 'if (comp_ret)' and dropping
> > the ternary assignment in the writeback-disabled path.
>
> Sounds like we're complexicating core code in order to accommodate
> inconsistent compressor behavior.  Would it be better to
>
> a) design the compressor interface(!)
> b) document it
> c) ensure that all compressors comply with it?
>
> Maintainers may choose to go ahead with this as a backportable
> quickfix, but it all seems to be a bit of a mess in there.

Thank you very much for the review and feedback, Andrew.

I completely agree that clearly defining, documenting, and enforcing a
consistent compressor interface across all implementations is the right
long-term approach. I sent a separate patch that standardizes return
codes on destination buffer overflows [1], but having a well-defined
compressor interface would tighten all gaps.

[1] https://lore.kernel.org/all/20261021205626.3286270-2-suseendranb@google.com/

> Wht you forgot to tell us (please never forget this) is the
> userspace-visible runtime effects of the bug.  Autobot says:
>
> : The userspace-visible effects are limited to corrected zswap
> : statistics and a new warning for invalid compressor results.
> :
> : When compression succeeds but produces exactly PAGE_SIZE bytes, and
> : zswap writeback is disabled, the page is rejected as before. However,
> : reject_compress_poor now increments instead of reject_compress_fail.
> : This makes the debugfs statistics distinguish incompressible pages
> : from actual compressor failures more accurately.
> :
> : A compressor which reports success with a zero or oversized output
> : length will now trigger WARN_ON_ONCE(). Previously, zswap handled
> : these results without a warning. On systems with panic_on_warn
> : enabled, this could cause a kernel panic.
> :
> : There is no intended change to which pages zswap accepts, rejects
> : or stores uncompressed, nor to normal application behavior.
> :
> : I think these runtime effects should be described more prominently
> : in the changelog, particularly the new warning.

Apologies for missing the userspace-visible runtime impact in the commit
description. I will make sure to always include this going forward.
Autobot is spot-on here. I would be happy to send a v2 with an updated
commit message that clearly documents these runtime effects and
behavioral changes.

> My autobot is all worried about the HiSilicon ZIP compressor.
>
> : I think the zswap-side fix is reasonable. On success, dlen should
> : contain the actual output length, and zswap should validate it. On
> : failure, dlen isn't necessarily meaningful, so we should rely on the
> : compressor's error code.
> :
> : Separately, I think we should make the compressors consistently return
> : -ENOSPC when the output buffer is too small. Some currently return
> : -EINVAL, which means reject_compress_fail versus reject_compress_poor
> : will remain inaccurate even with this patch.
> :
> : A compressor which successfully produces exactly PAGE_SIZE bytes
> : should still report success. Deciding that the result isn't worth
> : storing belongs in zswap.
> :
> : One thing worth checking is HiSilicon ZIP's negative-compression
> : status. It is treated as success, with dlen taken from the hardware.
> : Could that produce zero or more than PAGE_SIZE bytes? If so, the new
> : WARN_ON_ONCE() might trigger on ordinary incompressible data.
> :
> : I'd take this fix independently of the compressor cleanup, assuming
> : that last case is safe.

I checked drivers/crypto/hisilicon/zip/zip_crypto.c and the HiSilicon
ZIP hardware status definitions (in Linaro UADK drv/hisi_comp.c), and
this case is safe:

1. In hisi_zip_fill_buf_size(), sqe->input_data_length = a_req->slen and
   sqe->dest_avail_out = a_req->dlen (both PAGE_SIZE in zswap_compress()).

2. When an incompressible page overflows dest_avail_out (PAGE_SIZE), the
   hardware aborts at dest_avail_out and reports status 0x0e
   (ERR_DSTLEN_OUT, insufficient dest_avail_out), not 0x0d (HZIP_NC_ERR).
   In hisi_zip_acomp_cb(), 'status != 0 && status != HZIP_NC_ERR' sets
   err = -EIO (comp_ret != 0), so the !comp_ret branch is not entered.

3. HZIP_NC_ERR (0x0d, negative compression) only occurs when the output
   fits within dest_avail_out (0 < sqe->produced <= dest_avail_out) while
   exceeding input_data_length (i.e. when a caller passes dlen > slen).
   Even if HZIP_NC_ERR occurred with sqe->produced == PAGE_SIZE,
   'if (dlen == PAGE_SIZE)' handles it (comp_ret = -ENOSPC) before the
   'else if (WARN_ON_ONCE(!dlen || dlen > PAGE_SIZE))' check.

4. When ctx->fallback is enabled (hisi_zip_fallback_do_work()), it
   delegates to the software deflate/lz4 backend and only updates dlen on
   ret == 0 (0 < dlen <= PAGE_SIZE).

So hisi_zip will not return !comp_ret with dlen == 0 or dlen > PAGE_SIZE
on incompressible pages, and WARN_ON_ONCE() will not trigger on normal
incompressible data.

Please let me know if you would like me to send a v2 with the updated
commit message including the userspace-visible runtime impact.

Re: [PATCH] mm/zswap: validate dlen and normalize comp_ret on compression success

[Andrew Morton]

On Fri, 23 Oct 2026 20:04:26 +0000 Suseendran Bhaskaran <suseendranb@google.com> wrote:

> > Wht you forgot to tell us (please never forget this) is the
> > userspace-visible runtime effects of the bug.  Autobot says:
> >
> > : The userspace-visible effects are limited to corrected zswap
> > : statistics and a new warning for invalid compressor results.
> > :
> > : When compression succeeds but produces exactly PAGE_SIZE bytes, and
> > : zswap writeback is disabled, the page is rejected as before. However,
> > : reject_compress_poor now increments instead of reject_compress_fail.
> > : This makes the debugfs statistics distinguish incompressible pages
> > : from actual compressor failures more accurately.
> > :
> > : A compressor which reports success with a zero or oversized output
> > : length will now trigger WARN_ON_ONCE(). Previously, zswap handled
> > : these results without a warning. On systems with panic_on_warn
> > : enabled, this could cause a kernel panic.
> > :
> > : There is no intended change to which pages zswap accepts, rejects
> > : or stores uncompressed, nor to normal application behavior.
> > :
> > : I think these runtime effects should be described more prominently
> > : in the changelog, particularly the new warning.
> 
> Apologies for missing the userspace-visible runtime impact in the commit
> description. I will make sure to always include this going forward.
> Autobot is spot-on here. I would be happy to send a v2 with an updated
> commit message that clearly documents these runtime effects and
> behavioral changes.

I'm mainly interested in answering "should we backport this into
-stable kernels?".  A small debugfs accounting error is probably a
"no", but a WARN is "yes".  Is that real?  Can current kernels truly
emit WARNs due to this issue?

If so then yup, please send along a new changelog.  Or a v2 with a new
changelog.

Re: [PATCH] mm/zswap: validate dlen and normalize comp_ret on compression success

[Yosry Ahmed]

On Fri, Oct 23, 2026 at 4:21 PM Andrew Morton <akpm@linux-foundation.org> wrote:
>
> On Fri, 23 Oct 2026 20:04:26 +0000 Suseendran Bhaskaran <suseendranb@google.com> wrote:
>
> > > Wht you forgot to tell us (please never forget this) is the
> > > userspace-visible runtime effects of the bug.  Autobot says:
> > >
> > > : The userspace-visible effects are limited to corrected zswap
> > > : statistics and a new warning for invalid compressor results.
> > > :
> > > : When compression succeeds but produces exactly PAGE_SIZE bytes, and
> > > : zswap writeback is disabled, the page is rejected as before. However,
> > > : reject_compress_poor now increments instead of reject_compress_fail.
> > > : This makes the debugfs statistics distinguish incompressible pages
> > > : from actual compressor failures more accurately.
> > > :
> > > : A compressor which reports success with a zero or oversized output
> > > : length will now trigger WARN_ON_ONCE(). Previously, zswap handled
> > > : these results without a warning. On systems with panic_on_warn
> > > : enabled, this could cause a kernel panic.
> > > :
> > > : There is no intended change to which pages zswap accepts, rejects
> > > : or stores uncompressed, nor to normal application behavior.
> > > :
> > > : I think these runtime effects should be described more prominently
> > > : in the changelog, particularly the new warning.
> >
> > Apologies for missing the userspace-visible runtime impact in the commit
> > description. I will make sure to always include this going forward.
> > Autobot is spot-on here. I would be happy to send a v2 with an updated
> > commit message that clearly documents these runtime effects and
> > behavioral changes.
>
> I'm mainly interested in answering "should we backport this into
> -stable kernels?".  A small debugfs accounting error is probably a
> "no", but a WARN is "yes".  Is that real?  Can current kernels truly
> emit WARNs due to this issue?

No need for a stable backport, it's just debugfs accounting, and the
new WARN is just hardening.

>
> If so then yup, please send along a new changelog.  Or a v2 with a new
> changelog.

Re: [PATCH] mm/zswap: validate dlen and normalize comp_ret on compression success

[SJ Park]

On Thu, 22 Oct 2026 23:27:33 +0000 Suseendran Bhaskaran <suseendranb@google.com> wrote:

> In zswap_compress(), dlen is only meaningful when compression succeeds
> (!comp_ret), as compressors do not consistently set req->dlen on error
> (e.g. zstd, 842, and qat set dlen = 0; lz4, lz4hc, lzo, lzo-rle,
> deflate, and iaa leave dlen at PAGE_SIZE; hisi_zip sets dlen to the
> hardware-reported produced bytes).
> 
> When !comp_ret and dlen == PAGE_SIZE with zswap writeback disabled,
> zswap_compress() sets comp_ret to -EINVAL instead of -ENOSPC,
> miscounting the page under reject_compress_fail instead of
> reject_compress_poor.
> 
> When !comp_ret, set comp_ret = -ENOSPC if dlen == PAGE_SIZE, or
> comp_ret = -EINVAL if WARN_ON_ONCE(!dlen || dlen > PAGE_SIZE). This
> allows simplifying the failure check to 'if (comp_ret)' and dropping
> the ternary assignment in the writeback-disabled path.

Nice catch and fix!

> 
> Fixes: dca4437a5861 ("mm/zswap: store <PAGE_SIZE compression failed page as-is")

Thank you for fixing this, Suseendran!

> Signed-off-by: Suseendran Bhaskaran <suseendranb@google.com>

Reviewed-by: SJ Park <sj@kernel.org>


Thanks,
SJ

[...]