Re: [PATCH 2/2] crypto/algapi - make crypto_xor() take separate dst and src arguments

[Ard Biesheuvel]

On 18 July 2017 at 09:39, Herbert Xu  wrote:
> On Mon, Jul 10, 2017 at 02:45:48PM +0100, Ard Biesheuvel wrote:
>> There are quite a number of occurrences in the kernel of the pattern
>>
>> if (dst != src)
>> memcpy(dst, src, walk.total % AES_BLOCK_SIZE);
>> crypto_xor(dst, final, walk.total % AES_BLOCK_SIZE);
>>
>> or
>>
>> crypto_xor(keystream, src, nbytes);
>> memcpy(dst, keystream, nbytes);
>
> What keeping crypto_xor as it is and adding a new entry point for
> the 4-argument case?
>

Also fine.

Re: [PATCH 2/2] crypto/algapi - make crypto_xor() take separate dst and src arguments

[Herbert Xu]

On Mon, Jul 10, 2017 at 02:45:48PM +0100, Ard Biesheuvel wrote:
> There are quite a number of occurrences in the kernel of the pattern
> 
> if (dst != src)
> memcpy(dst, src, walk.total % AES_BLOCK_SIZE);
> crypto_xor(dst, final, walk.total % AES_BLOCK_SIZE);
> 
> or
> 
> crypto_xor(keystream, src, nbytes);
> memcpy(dst, keystream, nbytes);

What keeping crypto_xor as it is and adding a new entry point for
the 4-argument case?

Cheers,
-- 
Email: Herbert Xu 
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

[PATCH 2/2] crypto/algapi - make crypto_xor() take separate dst and src arguments

[Ard Biesheuvel]

There are quite a number of occurrences in the kernel of the pattern

if (dst != src)
memcpy(dst, src, walk.total % AES_BLOCK_SIZE);
crypto_xor(dst, final, walk.total % AES_BLOCK_SIZE);

or

crypto_xor(keystream, src, nbytes);
memcpy(dst, keystream, nbytes);

where crypto_xor() is preceded or followed by a memcpy() invocation
that is only there because crypto_xor() uses its output parameter as
one of the inputs. To avoid having to add new instances of this pattern
in the arm64 code, which will be refactored to implement non-SIMD
fallbacks, split the output and first input operands in crypto_xor().

While we're at it, fold in the memcpy()s that can now be made redundant.

Signed-off-by: Ard Biesheuvel 
---
 arch/arm/crypto/aes-ce-glue.c   |  4 +---
 arch/arm/crypto/aes-neonbs-glue.c   |  9 +++--
 arch/arm64/crypto/aes-glue.c|  8 
 arch/arm64/crypto/aes-neonbs-glue.c |  9 +++--
 arch/sparc/crypto/aes_glue.c|  3 +--
 arch/x86/crypto/aesni-intel_glue.c  |  4 ++--
 arch/x86/crypto/blowfish_glue.c |  3 +--
 arch/x86/crypto/cast5_avx_glue.c|  3 +--
 arch/x86/crypto/des3_ede_glue.c |  3 +--
 crypto/ccm.c|  2 +-
 crypto/chacha20_generic.c   |  4 ++--
 crypto/cmac.c   |  8 
 crypto/ctr.c|  7 +++
 crypto/cts.c|  4 ++--
 crypto/gcm.c|  4 ++--
 crypto/ghash-generic.c  |  2 +-
 crypto/keywrap.c|  4 ++--
 crypto/pcbc.c   | 20 
 crypto/salsa20_generic.c|  4 ++--
 crypto/seqiv.c  |  2 +-
 crypto/xcbc.c   |  8 
 drivers/crypto/vmx/aes_ctr.c|  3 +--
 drivers/md/dm-crypt.c   | 19 +--
 include/crypto/algapi.h | 10 ++
 include/crypto/cbc.h| 10 +-
 net/mac80211/fils_aead.c|  6 +++---
 26 files changed, 73 insertions(+), 90 deletions(-)

diff --git a/arch/arm/crypto/aes-ce-glue.c b/arch/arm/crypto/aes-ce-glue.c
index 0f966a8ca1ce..10374324ab25 100644
--- a/arch/arm/crypto/aes-ce-glue.c
+++ b/arch/arm/crypto/aes-ce-glue.c
@@ -285,9 +285,7 @@ static int ctr_encrypt(struct skcipher_request *req)
 
ce_aes_ctr_encrypt(tail, NULL, (u8 *)ctx->key_enc,
   num_rounds(ctx), blocks, walk.iv);
-   if (tdst != tsrc)
-   memcpy(tdst, tsrc, nbytes);
-   crypto_xor(tdst, tail, nbytes);
+   crypto_xor(tdst, tsrc, tail, nbytes);
err = skcipher_walk_done(, 0);
}
kernel_neon_end();
diff --git a/arch/arm/crypto/aes-neonbs-glue.c 
b/arch/arm/crypto/aes-neonbs-glue.c
index c76377961444..86f3c2c0d179 100644
--- a/arch/arm/crypto/aes-neonbs-glue.c
+++ b/arch/arm/crypto/aes-neonbs-glue.c
@@ -218,12 +218,9 @@ static int ctr_encrypt(struct skcipher_request *req)
  ctx->rk, ctx->rounds, blocks, walk.iv, final);
 
if (final) {
-   u8 *dst = walk.dst.virt.addr + blocks * AES_BLOCK_SIZE;
-   u8 *src = walk.src.virt.addr + blocks * AES_BLOCK_SIZE;
-
-   if (dst != src)
-   memcpy(dst, src, walk.total % AES_BLOCK_SIZE);
-   crypto_xor(dst, final, walk.total % AES_BLOCK_SIZE);
+   crypto_xor(walk.dst.virt.addr + blocks * AES_BLOCK_SIZE,
+  walk.src.virt.addr + blocks * AES_BLOCK_SIZE,
+  final, walk.total % AES_BLOCK_SIZE);
 
err = skcipher_walk_done(, 0);
break;
diff --git a/arch/arm64/crypto/aes-glue.c b/arch/arm64/crypto/aes-glue.c
index bcf596b0197e..eb93eccda503 100644
--- a/arch/arm64/crypto/aes-glue.c
+++ b/arch/arm64/crypto/aes-glue.c
@@ -241,9 +241,7 @@ static int ctr_encrypt(struct skcipher_request *req)
 
aes_ctr_encrypt(tail, NULL, (u8 *)ctx->key_enc, rounds,
blocks, walk.iv, first);
-   if (tdst != tsrc)
-   memcpy(tdst, tsrc, nbytes);
-   crypto_xor(tdst, tail, nbytes);
+   crypto_xor(tdst, tsrc, tail, nbytes);
err = skcipher_walk_done(, 0);
}
kernel_neon_end();
@@ -493,7 +491,9 @@ static int mac_update(struct shash_desc *desc, const u8 *p, 
unsigned int len)
l = min(len, AES_BLOCK_SIZE - ctx->len);
 
if (l <= AES_BLOCK_SIZE) {
-   crypto_xor(ctx->dg + ctx->len, p, l);
+   u8 *dg = ctx->dg + ctx->len;
+
+   crypto_xor(dg, dg, p, l);
ctx->len += l;
len -= l;
p += l;
diff --git