| Message ID | 20210616203459.1248036-1-keescook@chromium.org |
|---|---|
| State | Accepted |
| Commit | 74c66120fda6596ad57f41e1607b3a5d51ca143d |
| Headers | show |
| Series | crypto: nx: Fix memcpy() over-reading in nonce | expand |
Kees Cook <keescook@chromium.org> writes: > Fix typo in memcpy() where size should be CTR_RFC3686_NONCE_SIZE. > > Fixes: 030f4e968741 ("crypto: nx - Fix reentrancy bugs") > Cc: stable@vger.kernel.org > Signed-off-by: Kees Cook <keescook@chromium.org> Thanks. > --- > drivers/crypto/nx/nx-aes-ctr.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/crypto/nx/nx-aes-ctr.c b/drivers/crypto/nx/nx-aes-ctr.c > index 13f518802343..6120e350ff71 100644 > --- a/drivers/crypto/nx/nx-aes-ctr.c > +++ b/drivers/crypto/nx/nx-aes-ctr.c > @@ -118,7 +118,7 @@ static int ctr3686_aes_nx_crypt(struct skcipher_request *req) > struct nx_crypto_ctx *nx_ctx = crypto_skcipher_ctx(tfm); > u8 iv[16]; > > - memcpy(iv, nx_ctx->priv.ctr.nonce, CTR_RFC3686_IV_SIZE); > + memcpy(iv, nx_ctx->priv.ctr.nonce, CTR_RFC3686_NONCE_SIZE); > memcpy(iv + CTR_RFC3686_NONCE_SIZE, req->iv, CTR_RFC3686_IV_SIZE); > iv[12] = iv[13] = iv[14] = 0; > iv[15] = 1; Where IV_SIZE is 8 and NONCE_SIZE is 4. And iv is 16 bytes, so it's not a buffer overflow. But priv.ctr.nonce is 4 bytes, and at the end of the struct, so it reads 4 bytes past the end of the nx_crypto_ctx, which is not good. But then immediately overwrites whatever it read with req->iv. So seems pretty harmless in practice? cheers
On Thu, Jun 17, 2021 at 04:08:15PM +1000, Michael Ellerman wrote: > Kees Cook <keescook@chromium.org> writes: > > Fix typo in memcpy() where size should be CTR_RFC3686_NONCE_SIZE. > > > > Fixes: 030f4e968741 ("crypto: nx - Fix reentrancy bugs") > > Cc: stable@vger.kernel.org > > Signed-off-by: Kees Cook <keescook@chromium.org> > > Thanks. > > > --- > > drivers/crypto/nx/nx-aes-ctr.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/drivers/crypto/nx/nx-aes-ctr.c b/drivers/crypto/nx/nx-aes-ctr.c > > index 13f518802343..6120e350ff71 100644 > > --- a/drivers/crypto/nx/nx-aes-ctr.c > > +++ b/drivers/crypto/nx/nx-aes-ctr.c > > @@ -118,7 +118,7 @@ static int ctr3686_aes_nx_crypt(struct skcipher_request *req) > > struct nx_crypto_ctx *nx_ctx = crypto_skcipher_ctx(tfm); > > u8 iv[16]; > > > > - memcpy(iv, nx_ctx->priv.ctr.nonce, CTR_RFC3686_IV_SIZE); > > + memcpy(iv, nx_ctx->priv.ctr.nonce, CTR_RFC3686_NONCE_SIZE); > > memcpy(iv + CTR_RFC3686_NONCE_SIZE, req->iv, CTR_RFC3686_IV_SIZE); > > iv[12] = iv[13] = iv[14] = 0; > > iv[15] = 1; > > Where IV_SIZE is 8 and NONCE_SIZE is 4. > > And iv is 16 bytes, so it's not a buffer overflow. > > But priv.ctr.nonce is 4 bytes, and at the end of the struct, so it reads > 4 bytes past the end of the nx_crypto_ctx, which is not good. > > But then immediately overwrites whatever it read with req->iv. > > So seems pretty harmless in practice? Right -- there's no damage done, but future memcpy() FORTIFY work alerts on this, so I'm going through cleaning all of these up. :)
On Wed, Jun 16, 2021 at 01:34:59PM -0700, Kees Cook wrote: > Fix typo in memcpy() where size should be CTR_RFC3686_NONCE_SIZE. > > Fixes: 030f4e968741 ("crypto: nx - Fix reentrancy bugs") > Cc: stable@vger.kernel.org > Signed-off-by: Kees Cook <keescook@chromium.org> > --- > drivers/crypto/nx/nx-aes-ctr.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) Patch applied. Thanks.
diff --git a/drivers/crypto/nx/nx-aes-ctr.c b/drivers/crypto/nx/nx-aes-ctr.c index 13f518802343..6120e350ff71 100644 --- a/drivers/crypto/nx/nx-aes-ctr.c +++ b/drivers/crypto/nx/nx-aes-ctr.c @@ -118,7 +118,7 @@ static int ctr3686_aes_nx_crypt(struct skcipher_request *req) struct nx_crypto_ctx *nx_ctx = crypto_skcipher_ctx(tfm); u8 iv[16]; - memcpy(iv, nx_ctx->priv.ctr.nonce, CTR_RFC3686_IV_SIZE); + memcpy(iv, nx_ctx->priv.ctr.nonce, CTR_RFC3686_NONCE_SIZE); memcpy(iv + CTR_RFC3686_NONCE_SIZE, req->iv, CTR_RFC3686_IV_SIZE); iv[12] = iv[13] = iv[14] = 0; iv[15] = 1;
Fix typo in memcpy() where size should be CTR_RFC3686_NONCE_SIZE. Fixes: 030f4e968741 ("crypto: nx - Fix reentrancy bugs") Cc: stable@vger.kernel.org Signed-off-by: Kees Cook <keescook@chromium.org> --- drivers/crypto/nx/nx-aes-ctr.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)