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@xxxxxxxxxx> 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.