* [PATCH 01/16] crypto: cfb - add missing 'chunksize' property
[not found] <20190104041625.3259-1-ebiggers@kernel.org>
@ 2019-01-04 4:16 ` Eric Biggers
[not found] ` <20190104210311.5AC542087F@mail.kernel.org>
2019-01-04 4:16 ` [PATCH 02/16] crypto: cfb - remove bogus memcpy() with src == dest Eric Biggers
` (2 subsequent siblings)
3 siblings, 1 reply; 10+ messages in thread
From: Eric Biggers @ 2019-01-04 4:16 UTC (permalink / raw)
To: linux-crypto, Herbert Xu; +Cc: stable, James Bottomley
From: Eric Biggers <ebiggers@google.com>
Like some other block cipher mode implementations, the CFB
implementation assumes that while walking through the scatterlist, a
partial block does not occur until the end. But the walk is incorrectly
being done with a blocksize of 1, as 'cra_blocksize' is set to 1 (since
CFB is a stream cipher) but no 'chunksize' is set. This bug causes
incorrect encryption/decryption for some scatterlist layouts.
Fix it by setting the 'chunksize'. Also extend the CFB test vectors to
cover this bug as well as cases where the message length is not a
multiple of the block size.
Fixes: a7d85e06ed80 ("crypto: cfb - add support for Cipher FeedBack mode")
Cc: <stable@vger.kernel.org> # v4.17+
Cc: James Bottomley <James.Bottomley@HansenPartnership.com>
Signed-off-by: Eric Biggers <ebiggers@google.com>
---
crypto/cfb.c | 6 ++++++
crypto/testmgr.h | 25 +++++++++++++++++++++++++
2 files changed, 31 insertions(+)
diff --git a/crypto/cfb.c b/crypto/cfb.c
index e81e456734985..183e8a9c33128 100644
--- a/crypto/cfb.c
+++ b/crypto/cfb.c
@@ -298,6 +298,12 @@ static int crypto_cfb_create(struct crypto_template *tmpl, struct rtattr **tb)
inst->alg.base.cra_blocksize = 1;
inst->alg.base.cra_alignmask = alg->cra_alignmask;
+ /*
+ * To simplify the implementation, configure the skcipher walk to only
+ * give a partial block at the very end, never earlier.
+ */
+ inst->alg.chunksize = alg->cra_blocksize;
+
inst->alg.ivsize = alg->cra_blocksize;
inst->alg.min_keysize = alg->cra_cipher.cia_min_keysize;
inst->alg.max_keysize = alg->cra_cipher.cia_max_keysize;
diff --git a/crypto/testmgr.h b/crypto/testmgr.h
index e8f47d7b92cdd..7f4dae7a57a1c 100644
--- a/crypto/testmgr.h
+++ b/crypto/testmgr.h
@@ -12870,6 +12870,31 @@ static const struct cipher_testvec aes_cfb_tv_template[] = {
"\x75\xa3\x85\x74\x1a\xb9\xce\xf8"
"\x20\x31\x62\x3d\x55\xb1\xe4\x71",
.len = 64,
+ .also_non_np = 1,
+ .np = 2,
+ .tap = { 31, 33 },
+ }, { /* > 16 bytes, not a multiple of 16 bytes */
+ .key = "\x2b\x7e\x15\x16\x28\xae\xd2\xa6"
+ "\xab\xf7\x15\x88\x09\xcf\x4f\x3c",
+ .klen = 16,
+ .iv = "\x00\x01\x02\x03\x04\x05\x06\x07"
+ "\x08\x09\x0a\x0b\x0c\x0d\x0e\x0f",
+ .ptext = "\x6b\xc1\xbe\xe2\x2e\x40\x9f\x96"
+ "\xe9\x3d\x7e\x11\x73\x93\x17\x2a"
+ "\xae",
+ .ctext = "\x3b\x3f\xd9\x2e\xb7\x2d\xad\x20"
+ "\x33\x34\x49\xf8\xe8\x3c\xfb\x4a"
+ "\xc8",
+ .len = 17,
+ }, { /* < 16 bytes */
+ .key = "\x2b\x7e\x15\x16\x28\xae\xd2\xa6"
+ "\xab\xf7\x15\x88\x09\xcf\x4f\x3c",
+ .klen = 16,
+ .iv = "\x00\x01\x02\x03\x04\x05\x06\x07"
+ "\x08\x09\x0a\x0b\x0c\x0d\x0e\x0f",
+ .ptext = "\x6b\xc1\xbe\xe2\x2e\x40\x9f",
+ .ctext = "\x3b\x3f\xd9\x2e\xb7\x2d\xad",
+ .len = 7,
},
};
--
2.20.1
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH 02/16] crypto: cfb - remove bogus memcpy() with src == dest
[not found] <20190104041625.3259-1-ebiggers@kernel.org>
2019-01-04 4:16 ` [PATCH 01/16] crypto: cfb - add missing 'chunksize' property Eric Biggers
@ 2019-01-04 4:16 ` Eric Biggers
2019-01-04 4:16 ` [PATCH 03/16] crypto: ofb - fix handling partial blocks and make thread-safe Eric Biggers
2019-01-04 4:16 ` [PATCH 04/16] crypto: pcbc - remove bogus memcpy()s with src == dest Eric Biggers
3 siblings, 0 replies; 10+ messages in thread
From: Eric Biggers @ 2019-01-04 4:16 UTC (permalink / raw)
To: linux-crypto, Herbert Xu; +Cc: stable, James Bottomley
From: Eric Biggers <ebiggers@google.com>
The memcpy() in crypto_cfb_decrypt_inplace() uses walk->iv as both the
source and destination, which has undefined behavior. It is unneeded
because walk->iv is already used to hold the previous ciphertext block;
thus, walk->iv is already updated to its final value. So, remove it.
Also, note that in-place decryption is the only case where the previous
ciphertext block is not directly available. Therefore, as a related
cleanup I also updated crypto_cfb_encrypt_segment() to directly use the
previous ciphertext block rather than save it into walk->iv. This makes
it consistent with in-place encryption and out-of-place decryption; now
only in-place decryption is different, because it has to be.
Fixes: a7d85e06ed80 ("crypto: cfb - add support for Cipher FeedBack mode")
Cc: <stable@vger.kernel.org> # v4.17+
Cc: James Bottomley <James.Bottomley@HansenPartnership.com>
Signed-off-by: Eric Biggers <ebiggers@google.com>
---
crypto/cfb.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/crypto/cfb.c b/crypto/cfb.c
index 183e8a9c33128..4abfe32ff8451 100644
--- a/crypto/cfb.c
+++ b/crypto/cfb.c
@@ -77,12 +77,14 @@ static int crypto_cfb_encrypt_segment(struct skcipher_walk *walk,
do {
crypto_cfb_encrypt_one(tfm, iv, dst);
crypto_xor(dst, src, bsize);
- memcpy(iv, dst, bsize);
+ iv = dst;
src += bsize;
dst += bsize;
} while ((nbytes -= bsize) >= bsize);
+ memcpy(walk->iv, iv, bsize);
+
return nbytes;
}
@@ -162,7 +164,7 @@ static int crypto_cfb_decrypt_inplace(struct skcipher_walk *walk,
const unsigned int bsize = crypto_cfb_bsize(tfm);
unsigned int nbytes = walk->nbytes;
u8 *src = walk->src.virt.addr;
- u8 *iv = walk->iv;
+ u8 * const iv = walk->iv;
u8 tmp[MAX_CIPHER_BLOCKSIZE];
do {
@@ -172,8 +174,6 @@ static int crypto_cfb_decrypt_inplace(struct skcipher_walk *walk,
src += bsize;
} while ((nbytes -= bsize) >= bsize);
- memcpy(walk->iv, iv, bsize);
-
return nbytes;
}
--
2.20.1
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH 03/16] crypto: ofb - fix handling partial blocks and make thread-safe
[not found] <20190104041625.3259-1-ebiggers@kernel.org>
2019-01-04 4:16 ` [PATCH 01/16] crypto: cfb - add missing 'chunksize' property Eric Biggers
2019-01-04 4:16 ` [PATCH 02/16] crypto: cfb - remove bogus memcpy() with src == dest Eric Biggers
@ 2019-01-04 4:16 ` Eric Biggers
2019-01-06 10:38 ` Gilad Ben-Yossef
2019-01-04 4:16 ` [PATCH 04/16] crypto: pcbc - remove bogus memcpy()s with src == dest Eric Biggers
3 siblings, 1 reply; 10+ messages in thread
From: Eric Biggers @ 2019-01-04 4:16 UTC (permalink / raw)
To: linux-crypto, Herbert Xu; +Cc: stable, Gilad Ben-Yossef
From: Eric Biggers <ebiggers@google.com>
Fix multiple bugs in the OFB implementation:
1. It stored the per-request state 'cnt' in the tfm context, which can be
used by multiple threads concurrently (e.g. via AF_ALG).
2. It didn't support messages not a multiple of the block cipher size,
despite being a stream cipher.
3. It didn't set cra_blocksize to 1 to indicate it is a stream cipher.
To fix these, set the 'chunksize' property to the cipher block size to
guarantee that when walking through the scatterlist, a partial block can
only occur at the end. Then change the implementation to XOR a block at
a time at first, then XOR the partial block at the end if needed. This
is the same way CTR and CFB are implemented. As a bonus, this also
improves performance in most cases over the current approach.
Fixes: e497c51896b3 ("crypto: ofb - add output feedback mode")
Cc: <stable@vger.kernel.org> # v4.20+
Cc: Gilad Ben-Yossef <gilad@benyossef.com>
Signed-off-by: Eric Biggers <ebiggers@google.com>
---
crypto/ofb.c | 91 ++++++++++++++++++++----------------------------
crypto/testmgr.h | 28 +++++++++++++--
2 files changed, 63 insertions(+), 56 deletions(-)
diff --git a/crypto/ofb.c b/crypto/ofb.c
index 886631708c5e9..cab0b80953fed 100644
--- a/crypto/ofb.c
+++ b/crypto/ofb.c
@@ -5,9 +5,6 @@
*
* Copyright (C) 2018 ARM Limited or its affiliates.
* All rights reserved.
- *
- * Based loosely on public domain code gleaned from libtomcrypt
- * (https://github.com/libtom/libtomcrypt).
*/
#include <crypto/algapi.h>
@@ -21,7 +18,6 @@
struct crypto_ofb_ctx {
struct crypto_cipher *child;
- int cnt;
};
@@ -41,58 +37,40 @@ static int crypto_ofb_setkey(struct crypto_skcipher *parent, const u8 *key,
return err;
}
-static int crypto_ofb_encrypt_segment(struct crypto_ofb_ctx *ctx,
- struct skcipher_walk *walk,
- struct crypto_cipher *tfm)
+static int crypto_ofb_crypt(struct skcipher_request *req)
{
- int bsize = crypto_cipher_blocksize(tfm);
- int nbytes = walk->nbytes;
-
- u8 *src = walk->src.virt.addr;
- u8 *dst = walk->dst.virt.addr;
- u8 *iv = walk->iv;
-
- do {
- if (ctx->cnt == bsize) {
- if (nbytes < bsize)
- break;
- crypto_cipher_encrypt_one(tfm, iv, iv);
- ctx->cnt = 0;
- }
- *dst = *src ^ iv[ctx->cnt];
- src++;
- dst++;
- ctx->cnt++;
- } while (--nbytes);
- return nbytes;
-}
-
-static int crypto_ofb_encrypt(struct skcipher_request *req)
-{
- struct skcipher_walk walk;
struct crypto_skcipher *tfm = crypto_skcipher_reqtfm(req);
- unsigned int bsize;
struct crypto_ofb_ctx *ctx = crypto_skcipher_ctx(tfm);
- struct crypto_cipher *child = ctx->child;
- int ret = 0;
+ struct crypto_cipher *cipher = ctx->child;
+ const unsigned int bsize = crypto_cipher_blocksize(cipher);
+ struct skcipher_walk walk;
+ int err;
- bsize = crypto_cipher_blocksize(child);
- ctx->cnt = bsize;
+ err = skcipher_walk_virt(&walk, req, false);
- ret = skcipher_walk_virt(&walk, req, false);
+ while (walk.nbytes >= bsize) {
+ const u8 *src = walk.src.virt.addr;
+ u8 *dst = walk.dst.virt.addr;
+ u8 * const iv = walk.iv;
+ unsigned int nbytes = walk.nbytes;
- while (walk.nbytes) {
- ret = crypto_ofb_encrypt_segment(ctx, &walk, child);
- ret = skcipher_walk_done(&walk, ret);
- }
+ do {
+ crypto_cipher_encrypt_one(cipher, iv, iv);
+ crypto_xor_cpy(dst, src, iv, bsize);
+ dst += bsize;
+ src += bsize;
+ } while ((nbytes -= bsize) >= bsize);
- return ret;
-}
+ err = skcipher_walk_done(&walk, nbytes);
+ }
-/* OFB encrypt and decrypt are identical */
-static int crypto_ofb_decrypt(struct skcipher_request *req)
-{
- return crypto_ofb_encrypt(req);
+ if (walk.nbytes) {
+ crypto_cipher_encrypt_one(cipher, walk.iv, walk.iv);
+ crypto_xor_cpy(walk.dst.virt.addr, walk.src.virt.addr, walk.iv,
+ walk.nbytes);
+ err = skcipher_walk_done(&walk, 0);
+ }
+ return err;
}
static int crypto_ofb_init_tfm(struct crypto_skcipher *tfm)
@@ -165,13 +143,18 @@ static int crypto_ofb_create(struct crypto_template *tmpl, struct rtattr **tb)
if (err)
goto err_drop_spawn;
+ /* OFB mode is a stream cipher. */
+ inst->alg.base.cra_blocksize = 1;
+
+ /*
+ * To simplify the implementation, configure the skcipher walk to only
+ * give a partial block at the very end, never earlier.
+ */
+ inst->alg.chunksize = alg->cra_blocksize;
+
inst->alg.base.cra_priority = alg->cra_priority;
- inst->alg.base.cra_blocksize = alg->cra_blocksize;
inst->alg.base.cra_alignmask = alg->cra_alignmask;
- /* We access the data as u32s when xoring. */
- inst->alg.base.cra_alignmask |= __alignof__(u32) - 1;
-
inst->alg.ivsize = alg->cra_blocksize;
inst->alg.min_keysize = alg->cra_cipher.cia_min_keysize;
inst->alg.max_keysize = alg->cra_cipher.cia_max_keysize;
@@ -182,8 +165,8 @@ static int crypto_ofb_create(struct crypto_template *tmpl, struct rtattr **tb)
inst->alg.exit = crypto_ofb_exit_tfm;
inst->alg.setkey = crypto_ofb_setkey;
- inst->alg.encrypt = crypto_ofb_encrypt;
- inst->alg.decrypt = crypto_ofb_decrypt;
+ inst->alg.encrypt = crypto_ofb_crypt;
+ inst->alg.decrypt = crypto_ofb_crypt;
inst->free = crypto_ofb_free;
diff --git a/crypto/testmgr.h b/crypto/testmgr.h
index 7f4dae7a57a1c..ca8e8ebef309d 100644
--- a/crypto/testmgr.h
+++ b/crypto/testmgr.h
@@ -16681,8 +16681,7 @@ static const struct cipher_testvec aes_ctr_rfc3686_tv_template[] = {
};
static const struct cipher_testvec aes_ofb_tv_template[] = {
- /* From NIST Special Publication 800-38A, Appendix F.5 */
- {
+ { /* From NIST Special Publication 800-38A, Appendix F.5 */
.key = "\x2b\x7e\x15\x16\x28\xae\xd2\xa6"
"\xab\xf7\x15\x88\x09\xcf\x4f\x3c",
.klen = 16,
@@ -16705,6 +16704,31 @@ static const struct cipher_testvec aes_ofb_tv_template[] = {
"\x30\x4c\x65\x28\xf6\x59\xc7\x78"
"\x66\xa5\x10\xd9\xc1\xd6\xae\x5e",
.len = 64,
+ .also_non_np = 1,
+ .np = 2,
+ .tap = { 31, 33 },
+ }, { /* > 16 bytes, not a multiple of 16 bytes */
+ .key = "\x2b\x7e\x15\x16\x28\xae\xd2\xa6"
+ "\xab\xf7\x15\x88\x09\xcf\x4f\x3c",
+ .klen = 16,
+ .iv = "\x00\x01\x02\x03\x04\x05\x06\x07"
+ "\x08\x09\x0a\x0b\x0c\x0d\x0e\x0f",
+ .ptext = "\x6b\xc1\xbe\xe2\x2e\x40\x9f\x96"
+ "\xe9\x3d\x7e\x11\x73\x93\x17\x2a"
+ "\xae",
+ .ctext = "\x3b\x3f\xd9\x2e\xb7\x2d\xad\x20"
+ "\x33\x34\x49\xf8\xe8\x3c\xfb\x4a"
+ "\x77",
+ .len = 17,
+ }, { /* < 16 bytes */
+ .key = "\x2b\x7e\x15\x16\x28\xae\xd2\xa6"
+ "\xab\xf7\x15\x88\x09\xcf\x4f\x3c",
+ .klen = 16,
+ .iv = "\x00\x01\x02\x03\x04\x05\x06\x07"
+ "\x08\x09\x0a\x0b\x0c\x0d\x0e\x0f",
+ .ptext = "\x6b\xc1\xbe\xe2\x2e\x40\x9f",
+ .ctext = "\x3b\x3f\xd9\x2e\xb7\x2d\xad",
+ .len = 7,
}
};
--
2.20.1
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [PATCH 03/16] crypto: ofb - fix handling partial blocks and make thread-safe
2019-01-04 4:16 ` [PATCH 03/16] crypto: ofb - fix handling partial blocks and make thread-safe Eric Biggers
@ 2019-01-06 10:38 ` Gilad Ben-Yossef
0 siblings, 0 replies; 10+ messages in thread
From: Gilad Ben-Yossef @ 2019-01-06 10:38 UTC (permalink / raw)
To: Eric Biggers; +Cc: Linux Crypto Mailing List, Herbert Xu, stable
On Fri, Jan 4, 2019 at 6:20 AM Eric Biggers <ebiggers@kernel.org> wrote:
>
> From: Eric Biggers <ebiggers@google.com>
>
> Fix multiple bugs in the OFB implementation:
>
> 1. It stored the per-request state 'cnt' in the tfm context, which can be
> used by multiple threads concurrently (e.g. via AF_ALG).
> 2. It didn't support messages not a multiple of the block cipher size,
> despite being a stream cipher.
> 3. It didn't set cra_blocksize to 1 to indicate it is a stream cipher.
>
> To fix these, set the 'chunksize' property to the cipher block size to
> guarantee that when walking through the scatterlist, a partial block can
> only occur at the end. Then change the implementation to XOR a block at
> a time at first, then XOR the partial block at the end if needed. This
> is the same way CTR and CFB are implemented. As a bonus, this also
> improves performance in most cases over the current approach.
Well, it certainly looks like my implementation had a lot of room for
improvement :-)
Thank you for doing this, Eric
Reviewed-by: Gilad Ben-Yossef <gilad@benyossef.com>
Gilad
--
Gilad Ben-Yossef
Chief Coffee Drinker
values of β will give rise to dom!
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 04/16] crypto: pcbc - remove bogus memcpy()s with src == dest
[not found] <20190104041625.3259-1-ebiggers@kernel.org>
` (2 preceding siblings ...)
2019-01-04 4:16 ` [PATCH 03/16] crypto: ofb - fix handling partial blocks and make thread-safe Eric Biggers
@ 2019-01-04 4:16 ` Eric Biggers
2019-01-04 9:57 ` David Howells
3 siblings, 1 reply; 10+ messages in thread
From: Eric Biggers @ 2019-01-04 4:16 UTC (permalink / raw)
To: linux-crypto, Herbert Xu; +Cc: stable, David Howells
From: Eric Biggers <ebiggers@google.com>
The memcpy()s in the PCBC implementation use walk->iv as both the source
and destination, which has undefined behavior. These memcpy()'s are
actually unneeded, because walk->iv is already used to hold the previous
plaintext block XOR'd with the previous ciphertext block. Thus,
walk->iv is already updated to its final value.
So remove the broken and unnecessary memcpy()s.
Fixes: 91652be5d1b9 ("[CRYPTO] pcbc: Add Propagated CBC template")
Cc: <stable@vger.kernel.org> # v2.6.21+
Cc: David Howells <dhowells@redhat.com>
Signed-off-by: Eric Biggers <ebiggers@google.com>
---
crypto/pcbc.c | 14 ++++----------
1 file changed, 4 insertions(+), 10 deletions(-)
diff --git a/crypto/pcbc.c b/crypto/pcbc.c
index 8aa10144407c0..1b182dfedc948 100644
--- a/crypto/pcbc.c
+++ b/crypto/pcbc.c
@@ -51,7 +51,7 @@ static int crypto_pcbc_encrypt_segment(struct skcipher_request *req,
unsigned int nbytes = walk->nbytes;
u8 *src = walk->src.virt.addr;
u8 *dst = walk->dst.virt.addr;
- u8 *iv = walk->iv;
+ u8 * const iv = walk->iv;
do {
crypto_xor(iv, src, bsize);
@@ -72,7 +72,7 @@ static int crypto_pcbc_encrypt_inplace(struct skcipher_request *req,
int bsize = crypto_cipher_blocksize(tfm);
unsigned int nbytes = walk->nbytes;
u8 *src = walk->src.virt.addr;
- u8 *iv = walk->iv;
+ u8 * const iv = walk->iv;
u8 tmpbuf[MAX_CIPHER_BLOCKSIZE];
do {
@@ -84,8 +84,6 @@ static int crypto_pcbc_encrypt_inplace(struct skcipher_request *req,
src += bsize;
} while ((nbytes -= bsize) >= bsize);
- memcpy(walk->iv, iv, bsize);
-
return nbytes;
}
@@ -121,7 +119,7 @@ static int crypto_pcbc_decrypt_segment(struct skcipher_request *req,
unsigned int nbytes = walk->nbytes;
u8 *src = walk->src.virt.addr;
u8 *dst = walk->dst.virt.addr;
- u8 *iv = walk->iv;
+ u8 * const iv = walk->iv;
do {
crypto_cipher_decrypt_one(tfm, dst, src);
@@ -132,8 +130,6 @@ static int crypto_pcbc_decrypt_segment(struct skcipher_request *req,
dst += bsize;
} while ((nbytes -= bsize) >= bsize);
- memcpy(walk->iv, iv, bsize);
-
return nbytes;
}
@@ -144,7 +140,7 @@ static int crypto_pcbc_decrypt_inplace(struct skcipher_request *req,
int bsize = crypto_cipher_blocksize(tfm);
unsigned int nbytes = walk->nbytes;
u8 *src = walk->src.virt.addr;
- u8 *iv = walk->iv;
+ u8 * const iv = walk->iv;
u8 tmpbuf[MAX_CIPHER_BLOCKSIZE] __aligned(__alignof__(u32));
do {
@@ -156,8 +152,6 @@ static int crypto_pcbc_decrypt_inplace(struct skcipher_request *req,
src += bsize;
} while ((nbytes -= bsize) >= bsize);
- memcpy(walk->iv, iv, bsize);
-
return nbytes;
}
--
2.20.1
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [PATCH 04/16] crypto: pcbc - remove bogus memcpy()s with src == dest
2019-01-04 4:16 ` [PATCH 04/16] crypto: pcbc - remove bogus memcpy()s with src == dest Eric Biggers
@ 2019-01-04 9:57 ` David Howells
2019-01-04 17:07 ` Eric Biggers
0 siblings, 1 reply; 10+ messages in thread
From: David Howells @ 2019-01-04 9:57 UTC (permalink / raw)
To: Eric Biggers; +Cc: dhowells, linux-crypto, Herbert Xu, stable
Eric Biggers <ebiggers@kernel.org> wrote:
> - u8 *iv = walk->iv;
> + u8 * const iv = walk->iv;
Does adding this const actually gain anything? (this is done twice)
David
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 04/16] crypto: pcbc - remove bogus memcpy()s with src == dest
2019-01-04 9:57 ` David Howells
@ 2019-01-04 17:07 ` Eric Biggers
2019-01-04 17:24 ` David Howells
0 siblings, 1 reply; 10+ messages in thread
From: Eric Biggers @ 2019-01-04 17:07 UTC (permalink / raw)
To: David Howells; +Cc: linux-crypto, Herbert Xu, stable
On Fri, Jan 04, 2019 at 09:57:13AM +0000, David Howells wrote:
> Eric Biggers <ebiggers@kernel.org> wrote:
>
> > - u8 *iv = walk->iv;
> > + u8 * const iv = walk->iv;
>
> Does adding this const actually gain anything? (this is done twice)
>
> David
It makes it clearer what's going on, especially since some modes update the 'iv'
pointer after each block (delaying the copy to 'walk.iv' until the end) but
others can't do that. The 'const' is helpful to further distinguish these two
cases, which were confused in both the pcbc and cfb implementations.
- Eric
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 04/16] crypto: pcbc - remove bogus memcpy()s with src == dest
2019-01-04 17:07 ` Eric Biggers
@ 2019-01-04 17:24 ` David Howells
0 siblings, 0 replies; 10+ messages in thread
From: David Howells @ 2019-01-04 17:24 UTC (permalink / raw)
To: Eric Biggers; +Cc: dhowells, linux-crypto, Herbert Xu, stable
Eric Biggers <ebiggers@kernel.org> wrote:
> It makes it clearer what's going on, especially since some modes update the
> 'iv' pointer after each block (delaying the copy to 'walk.iv' until the end)
> but others can't do that. The 'const' is helpful to further distinguish
> these two cases, which were confused in both the pcbc and cfb
> implementations.
I'm not sure I agree that it makes it clearer, but:
Reviewed-and-tested-by: David Howells <dhowells@redhat.com>
^ permalink raw reply [flat|nested] 10+ messages in thread