linux-kernel - Re: [PATCH] crypto: scompress - fix req->dst buffer overflow

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

Message-ID: <ZYvtqW7TAm6mCelt@gondor.apana.org.au>
Date: Wed, 27 Dec 2023 17:26:01 +0800
From: Herbert Xu <herbert@...dor.apana.org.au>
To: chengming.zhou@...ux.dev
Cc: akpm@...ux-foundation.org, chrisl@...nel.org, davem@...emloft.net,
	linux-crypto@...r.kernel.org, linux-kernel@...r.kernel.org,
	nphamcs@...il.com, syzkaller-bugs@...glegroups.com,
	yosryahmed@...gle.com, 21cnbao@...il.com,
	zhouchengming@...edance.com,
	syzbot+3eff5e51bf1db122a16e@...kaller.appspotmail.com
Subject: Re: [PATCH] crypto: scompress - fix req->dst buffer overflow

On Wed, Dec 27, 2023 at 06:50:43AM +0000, chengming.zhou@...ux.dev wrote:
> From: Chengming Zhou <zhouchengming@...edance.com>
> 
> The req->dst buffer size should be checked before copying from the
> scomp_scratch->dst to avoid req->dst buffer overflow problem.
> 
> Fixes: 1ab53a77b772 ("crypto: acomp - add driver-side scomp interface")
> Reported-by: syzbot+3eff5e51bf1db122a16e@...kaller.appspotmail.com
> Closes: https://lore.kernel.org/all/0000000000000b05cd060d6b5511@google.com/
> Signed-off-by: Chengming Zhou <zhouchengming@...edance.com>
> ---
>  crypto/scompress.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/crypto/scompress.c b/crypto/scompress.c
> index 442a82c9de7d..e654a120ae5a 100644
> --- a/crypto/scompress.c
> +++ b/crypto/scompress.c
> @@ -117,6 +117,7 @@ static int scomp_acomp_comp_decomp(struct acomp_req *req, int dir)
>  	struct crypto_scomp *scomp = *tfm_ctx;
>  	void **ctx = acomp_request_ctx(req);
>  	struct scomp_scratch *scratch;
> +	unsigned int dlen;
>  	int ret;
>  
>  	if (!req->src || !req->slen || req->slen > SCOMP_SCRATCH_SIZE)
> @@ -128,6 +129,8 @@ static int scomp_acomp_comp_decomp(struct acomp_req *req, int dir)
>  	if (!req->dlen || req->dlen > SCOMP_SCRATCH_SIZE)
>  		req->dlen = SCOMP_SCRATCH_SIZE;
>  
> +	dlen = req->dlen;
> +
>  	scratch = raw_cpu_ptr(&scomp_scratch);
>  	spin_lock(&scratch->lock);
>  
> @@ -145,6 +148,9 @@ static int scomp_acomp_comp_decomp(struct acomp_req *req, int dir)
>  				ret = -ENOMEM;
>  				goto out;
>  			}
> +		} else if (req->dlen > dlen) {
> +			ret = -ENOMEM;
> +			goto out;

I think ENOMEM is ambiguous, perhaps ENOSPC?

Thanks,
-- 
Email: Herbert Xu <herbert@...dor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt