Re: [PATCH] crypto: scomp - Fix off-by-one bug when calculating last page

Marius Dinu
2025-04-21T19:35:43Z

On Mon, 2025-04-21 11.31.31 ++0800, Herbert Xu wrote:
> On Sun, Apr 20, 2025 at 04:35:44PM -0700, Nhat Pham wrote:
> >
> > Anyhow, this looks like a crypto/compression infra bug. Herbert, does
> > this ring any bell for you?
>
> Yes this looks like an off-by-one bug in the new scomp scratch
> code.
>
> ---8<---
> Fix off-by-one bug in the last page calculation for src and dst.
>
> Reported-by: Nhat Pham <nphamcs@gmail.com>
> Fixes: 2d3553ecb4e3 ("crypto: scomp - Remove support for some non-trivial SG lists")
> Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
>
> diff --git a/crypto/scompress.c b/crypto/scompress.c
> index 5762fcc63b51..36934c78d127 100644
> --- a/crypto/scompress.c
> +++ b/crypto/scompress.c
> @@ -215,8 +215,8 @@ static int scomp_acomp_comp_decomp(struct acomp_req *req, int dir)
> spage = nth_page(spage, soff / PAGE_SIZE);
> soff = offset_in_page(soff);
>
> - n = slen / PAGE_SIZE;
> - n += (offset_in_page(slen) + soff - 1) / PAGE_SIZE;
> + n = (slen - 1) / PAGE_SIZE;
> + n += (offset_in_page(slen - 1) + soff) / PAGE_SIZE;
> if (PageHighMem(nth_page(spage, n)) &&
> size_add(soff, slen) > PAGE_SIZE)
> break;
> @@ -243,9 +243,9 @@ static int scomp_acomp_comp_decomp(struct acomp_req *req, int dir)
> dpage = nth_page(dpage, doff / PAGE_SIZE);
> doff = offset_in_page(doff);
>
> - n = dlen / PAGE_SIZE;
> - n += (offset_in_page(dlen) + doff - 1) / PAGE_SIZE;
> - if (PageHighMem(dpage + n) &&
> + n = (dlen - 1) / PAGE_SIZE;
> + n += (offset_in_page(dlen - 1) + doff) / PAGE_SIZE;
> + if (PageHighMem(nth_page(dpage, n)) &&
> size_add(doff, dlen) > PAGE_SIZE)
> break;
> dst = kmap_local_page(dpage) + doff;
> --
> Email: Herbert Xu <herbert@gondor.apana.org.au>
> Home Page: http://gondor.apana.org.au/~herbert/
> PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Tested the patch with git master branch on the same SBC as my bug report.
It works. stress-ng --pageswap doesn't crash anymore.

Thank you!

Marius Dinu