| Message ID | 20231227065043.2730440-1-chengming.zhou@linux.dev (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Herbert Xu |
| Headers | show |
| Series | crypto: scompress - fix req->dst buffer overflow | expand |
On Wed, Dec 27, 2023 at 06:50:43AM +0000, chengming.zhou@linux.dev wrote: > From: Chengming Zhou <zhouchengming@bytedance.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@syzkaller.appspotmail.com > Closes: https://lore.kernel.org/all/0000000000000b05cd060d6b5511@google.com/ > Signed-off-by: Chengming Zhou <zhouchengming@bytedance.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,
On 2023/12/27 17:26, Herbert Xu wrote: > On Wed, Dec 27, 2023 at 06:50:43AM +0000, chengming.zhou@linux.dev wrote: >> From: Chengming Zhou <zhouchengming@bytedance.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@syzkaller.appspotmail.com >> Closes: https://lore.kernel.org/all/0000000000000b05cd060d6b5511@google.com/ >> Signed-off-by: Chengming Zhou <zhouchengming@bytedance.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? Right, ENOSPC is better. Should I send a v2? Thanks.
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,
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; } scatterwalk_map_and_copy(scratch->dst, req->dst, 0, req->dlen, 1);