2023-12-27 09:26:29

by Herbert Xu

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

On Wed, Dec 27, 2023 at 06:50:43AM +0000, [email protected] wrote:
> From: Chengming Zhou <[email protected]>
>
> 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: [email protected]
> Closes: https://lore.kernel.org/all/[email protected]/
> Signed-off-by: Chengming Zhou <[email protected]>
> ---
> 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 <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

2023-12-27 09:28:53

by Chengming Zhou

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

On 2023/12/27 17:26, Herbert Xu wrote:
> On Wed, Dec 27, 2023 at 06:50:43AM +0000, [email protected] wrote:
>> From: Chengming Zhou <[email protected]>
>>
>> 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: [email protected]
>> Closes: https://lore.kernel.org/all/[email protected]/
>> Signed-off-by: Chengming Zhou <[email protected]>
>> ---
>> 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?

Right, ENOSPC is better. Should I send a v2?

Thanks.

2023-12-27 09:31:04

by Herbert Xu

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

On Wed, Dec 27, 2023 at 05:28:35PM +0800, Chengming Zhou wrote:
>
> Right, ENOSPC is better. Should I send a v2?

Yes please.

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