Re: [PATCH] KEYS: reject NULL restriction string when type is specified

[Mat Martineau]

Eric,

On Thu, 30 Nov 2017, Eric Biggers wrote:


From: Eric Biggers 

keyctl_restrict_keyring() allows through a NULL restriction when the
"type" is non-NULL, which causes a NULL pointer dereference in
asymmetric_lookup_restriction() when it calls strcmp() on the
restriction string.

But no key types actually use a "NULL restriction" to mean anything, so
update keyctl_restrict_keyring() to reject it with EINVAL.


Since this fixes the bug for the asymmetric key type and ensures that 
other key types won't make the same mistake, I agree this is the way to 
fix it. I did not find any issues in the patch.


Thanks,

Mat



Reported-by: syzbot 
Fixes: 97d3aa0f3134 ("KEYS: Add a lookup_restriction function for the asymmetric key 
type")
Cc:  # v4.12+
Signed-off-by: Eric Biggers 
---
security/keys/keyctl.c | 24 ++--
1 file changed, 10 insertions(+), 14 deletions(-)

diff --git a/security/keys/keyctl.c b/security/keys/keyctl.c
index 76d22f726ae4..1ffe60bb2845 100644
--- a/security/keys/keyctl.c
+++ b/security/keys/keyctl.c
@@ -1588,9 +1588,8 @@ long keyctl_session_to_parent(void)
 * The caller must have Setattr permission to change keyring restrictions.
 *
 * The requested type name may be a NULL pointer to reject all attempts
- * to link to the keyring. If _type is non-NULL, _restriction can be
- * NULL or a pointer to a string describing the restriction. If _type is
- * NULL, _restriction must also be NULL.
+ * to link to the keyring.  In this case, _restriction must also be NULL.
+ * Otherwise, both _type and _restriction must be non-NULL.
 *
 * Returns 0 if successful.
 */
@@ -1598,7 +1597,6 @@ long keyctl_restrict_keyring(key_serial_t id, const char 
__user *_type,
 const char __user *_restriction)
{
key_ref_t key_ref;
-   bool link_reject = !_type;
char type[32];
char *restriction = NULL;
long ret;
@@ -1607,31 +1605,29 @@ long keyctl_restrict_keyring(key_serial_t id, const 
char __user *_type,
if (IS_ERR(key_ref))
return PTR_ERR(key_ref);

+   ret = -EINVAL;
if (_type) {
-   ret = key_get_type_from_user(type, _type, sizeof(type));
-   if (ret < 0)
+   if (!_restriction)
goto error;
-   }

-   if (_restriction) {
-   if (!_type) {
-   ret = -EINVAL;
+   ret = key_get_type_from_user(type, _type, sizeof(type));
+   if (ret < 0)
goto error;
-   }

restriction = strndup_user(_restriction, PAGE_SIZE);
if (IS_ERR(restriction)) {
ret = PTR_ERR(restriction);
goto error;
}
+   } else {
+   if (_restriction)
+   goto error;
}

-   ret = keyring_restrict(key_ref, link_reject ? NULL : type, restriction);
+   ret = keyring_restrict(key_ref, _type ? type : NULL, restriction);
kfree(restriction);
-
error:
key_ref_put(key_ref);
-
return ret;
}

--
2.15.0.531.g2ccb3012c9-goog




--
Mat Martineau
Intel OTC

Re: [PATCH v2 3/4] crypto: inside-secure - only update the result buffer when provided

[Herbert Xu]

On Thu, Nov 30, 2017 at 10:19:26AM +0100, Kamil Konieczny wrote:
>
> can the driver get request for final/finup/digest with null req->result ?
> If yes (?), such checks can be done before any hardware processing, saving 
> time,
> for example:

This should not be possible through any user-space facing API.

If a kernel API user did this then they're just shooting themselves
in the foot.

So unless there is a valida call path that leads to this then I
would say that there is nothing to fix.

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

[PATCH] KEYS: reject NULL restriction string when type is specified

[Eric Biggers]

From: Eric Biggers 

keyctl_restrict_keyring() allows through a NULL restriction when the
"type" is non-NULL, which causes a NULL pointer dereference in
asymmetric_lookup_restriction() when it calls strcmp() on the
restriction string.

But no key types actually use a "NULL restriction" to mean anything, so
update keyctl_restrict_keyring() to reject it with EINVAL.

Reported-by: syzbot 
Fixes: 97d3aa0f3134 ("KEYS: Add a lookup_restriction function for the 
asymmetric key type")
Cc:  # v4.12+
Signed-off-by: Eric Biggers 
---
 security/keys/keyctl.c | 24 ++--
 1 file changed, 10 insertions(+), 14 deletions(-)

diff --git a/security/keys/keyctl.c b/security/keys/keyctl.c
index 76d22f726ae4..1ffe60bb2845 100644
--- a/security/keys/keyctl.c
+++ b/security/keys/keyctl.c
@@ -1588,9 +1588,8 @@ long keyctl_session_to_parent(void)
  * The caller must have Setattr permission to change keyring restrictions.
  *
  * The requested type name may be a NULL pointer to reject all attempts
- * to link to the keyring. If _type is non-NULL, _restriction can be
- * NULL or a pointer to a string describing the restriction. If _type is
- * NULL, _restriction must also be NULL.
+ * to link to the keyring.  In this case, _restriction must also be NULL.
+ * Otherwise, both _type and _restriction must be non-NULL.
  *
  * Returns 0 if successful.
  */
@@ -1598,7 +1597,6 @@ long keyctl_restrict_keyring(key_serial_t id, const char 
__user *_type,
 const char __user *_restriction)
 {
key_ref_t key_ref;
-   bool link_reject = !_type;
char type[32];
char *restriction = NULL;
long ret;
@@ -1607,31 +1605,29 @@ long keyctl_restrict_keyring(key_serial_t id, const 
char __user *_type,
if (IS_ERR(key_ref))
return PTR_ERR(key_ref);
 
+   ret = -EINVAL;
if (_type) {
-   ret = key_get_type_from_user(type, _type, sizeof(type));
-   if (ret < 0)
+   if (!_restriction)
goto error;
-   }
 
-   if (_restriction) {
-   if (!_type) {
-   ret = -EINVAL;
+   ret = key_get_type_from_user(type, _type, sizeof(type));
+   if (ret < 0)
goto error;
-   }
 
restriction = strndup_user(_restriction, PAGE_SIZE);
if (IS_ERR(restriction)) {
ret = PTR_ERR(restriction);
goto error;
}
+   } else {
+   if (_restriction)
+   goto error;
}
 
-   ret = keyring_restrict(key_ref, link_reject ? NULL : type, restriction);
+   ret = keyring_restrict(key_ref, _type ? type : NULL, restriction);
kfree(restriction);
-
 error:
key_ref_put(key_ref);
-
return ret;
 }
 
-- 
2.15.0.531.g2ccb3012c9-goog

[PATCH v5 06/27] x86: crypto, annotate local functions

[Jiri Slaby]

Use the newly added SYM_FUNC_START_LOCAL to annotate starts of all
functions which do not have ".globl" annotation, but their ends are
annotated by ENDPROC. This is needed to balance ENDPROC for tools that
are about to generate debuginfo.

To be symmetric, we also convert their ENDPROCs to the new SYM_FUNC_END.

Signed-off-by: Jiri Slaby 
Cc: Herbert Xu 
Cc: "David S. Miller" 
Cc: Thomas Gleixner 
Cc: Ingo Molnar 
Cc: "H. Peter Anvin" 
Cc: 
Cc: 
---
 arch/x86/crypto/aesni-intel_asm.S| 49 
 arch/x86/crypto/camellia-aesni-avx-asm_64.S  | 20 ++--
 arch/x86/crypto/camellia-aesni-avx2-asm_64.S | 20 ++--
 arch/x86/crypto/cast5-avx-x86_64-asm_64.S|  8 ++---
 arch/x86/crypto/cast6-avx-x86_64-asm_64.S|  8 ++---
 arch/x86/crypto/ghash-clmulni-intel_asm.S|  4 +--
 arch/x86/crypto/serpent-avx-x86_64-asm_64.S  |  8 ++---
 arch/x86/crypto/serpent-avx2-asm_64.S|  8 ++---
 arch/x86/crypto/twofish-avx-x86_64-asm_64.S  |  8 ++---
 9 files changed, 62 insertions(+), 71 deletions(-)

diff --git a/arch/x86/crypto/aesni-intel_asm.S 
b/arch/x86/crypto/aesni-intel_asm.S
index 16627fec80b2..1d34d5a14682 100644
--- a/arch/x86/crypto/aesni-intel_asm.S
+++ b/arch/x86/crypto/aesni-intel_asm.S
@@ -1875,7 +1875,7 @@ ENDPROC(aesni_gcm_enc)
 
 .align 4
 _key_expansion_128:
-_key_expansion_256a:
+SYM_FUNC_START_LOCAL(_key_expansion_256a)
pshufd $0b, %xmm1, %xmm1
shufps $0b0001, %xmm0, %xmm4
pxor %xmm4, %xmm0
@@ -1886,10 +1886,9 @@ _key_expansion_256a:
add $0x10, TKEYP
ret
 ENDPROC(_key_expansion_128)
-ENDPROC(_key_expansion_256a)
+SYM_FUNC_END(_key_expansion_256a)
 
-.align 4
-_key_expansion_192a:
+SYM_FUNC_START_LOCAL(_key_expansion_192a)
pshufd $0b01010101, %xmm1, %xmm1
shufps $0b0001, %xmm0, %xmm4
pxor %xmm4, %xmm0
@@ -1911,10 +1910,9 @@ _key_expansion_192a:
movaps %xmm1, 0x10(TKEYP)
add $0x20, TKEYP
ret
-ENDPROC(_key_expansion_192a)
+SYM_FUNC_END(_key_expansion_192a)
 
-.align 4
-_key_expansion_192b:
+SYM_FUNC_START_LOCAL(_key_expansion_192b)
pshufd $0b01010101, %xmm1, %xmm1
shufps $0b0001, %xmm0, %xmm4
pxor %xmm4, %xmm0
@@ -1931,10 +1929,9 @@ _key_expansion_192b:
movaps %xmm0, (TKEYP)
add $0x10, TKEYP
ret
-ENDPROC(_key_expansion_192b)
+SYM_FUNC_END(_key_expansion_192b)
 
-.align 4
-_key_expansion_256b:
+SYM_FUNC_START_LOCAL(_key_expansion_256b)
pshufd $0b10101010, %xmm1, %xmm1
shufps $0b0001, %xmm2, %xmm4
pxor %xmm4, %xmm2
@@ -1944,7 +1941,7 @@ _key_expansion_256b:
movaps %xmm2, (TKEYP)
add $0x10, TKEYP
ret
-ENDPROC(_key_expansion_256b)
+SYM_FUNC_END(_key_expansion_256b)
 
 /*
  * int aesni_set_key(struct crypto_aes_ctx *ctx, const u8 *in_key,
@@ -2097,8 +2094,7 @@ ENDPROC(aesni_enc)
  * KEY
  * TKEYP (T1)
  */
-.align 4
-_aesni_enc1:
+SYM_FUNC_START_LOCAL(_aesni_enc1)
movaps (KEYP), KEY  # key
mov KEYP, TKEYP
pxor KEY, STATE # round 0
@@ -2141,7 +2137,7 @@ _aesni_enc1:
movaps 0x70(TKEYP), KEY
AESENCLAST KEY STATE
ret
-ENDPROC(_aesni_enc1)
+SYM_FUNC_END(_aesni_enc1)
 
 /*
  * _aesni_enc4:internal ABI
@@ -2161,8 +2157,7 @@ ENDPROC(_aesni_enc1)
  * KEY
  * TKEYP (T1)
  */
-.align 4
-_aesni_enc4:
+SYM_FUNC_START_LOCAL(_aesni_enc4)
movaps (KEYP), KEY  # key
mov KEYP, TKEYP
pxor KEY, STATE1# round 0
@@ -2250,7 +2245,7 @@ _aesni_enc4:
AESENCLAST KEY STATE3
AESENCLAST KEY STATE4
ret
-ENDPROC(_aesni_enc4)
+SYM_FUNC_END(_aesni_enc4)
 
 /*
  * void aesni_dec (struct crypto_aes_ctx *ctx, u8 *dst, const u8 *src)
@@ -2289,8 +2284,7 @@ ENDPROC(aesni_dec)
  * KEY
  * TKEYP (T1)
  */
-.align 4
-_aesni_dec1:
+SYM_FUNC_START_LOCAL(_aesni_dec1)
movaps (KEYP), KEY  # key
mov KEYP, TKEYP
pxor KEY, STATE # round 0
@@ -2333,7 +2327,7 @@ _aesni_dec1:
movaps 0x70(TKEYP), KEY
AESDECLAST KEY STATE
ret
-ENDPROC(_aesni_dec1)
+SYM_FUNC_END(_aesni_dec1)
 
 /*
  * _aesni_dec4:internal ABI
@@ -2353,8 +2347,7 @@ ENDPROC(_aesni_dec1)
  * KEY
  * TKEYP (T1)
  */
-.align 4
-_aesni_dec4:
+SYM_FUNC_START_LOCAL(_aesni_dec4)
movaps (KEYP), KEY  # key
mov KEYP, TKEYP
pxor KEY, STATE1# round 0
@@ -2442,7 +2435,7 @@ _aesni_dec4:
AESDECLAST KEY STATE3
AESDECLAST KEY STATE4
ret
-ENDPROC(_aesni_dec4)
+SYM_FUNC_END(_aesni_dec4)
 
 /*
  * void aesni_ecb_enc(struct crypto_aes_ctx *ctx, const u8 *dst, u8 *src,
@@ -2720,8 +2713,7 @@ ENDPROC(aesni_cbc_dec)
  * INC:== 1, in little endian
  * BSWAP_MASK 

[PATCH v5 08/27] x86: assembly, annotate aliases

[Jiri Slaby]

_key_expansion_128 is an alias to _key_expansion_256a, __memcpy to
memcpy, xen_syscall32_target to xen_sysenter_target, and so on. Annotate
them all using the new SYM_FUNC_START_ALIAS, SYM_FUNC_START_LOCAL_ALIAS,
and SYM_FUNC_END_ALIAS. This will make the tools generating the
debuginfo happy.

Signed-off-by: Jiri Slaby 
Cc: Herbert Xu 
Cc: "David S. Miller" 
Cc: Thomas Gleixner 
Cc: Ingo Molnar 
Cc: "H. Peter Anvin" 
Cc: 
Cc: Boris Ostrovsky 
Cc: Juergen Gross 
Reviewed-by: Juergen Gross  [xen parts]
Cc: 
Cc: 
---
 arch/x86/crypto/aesni-intel_asm.S | 5 ++---
 arch/x86/lib/memcpy_64.S  | 4 ++--
 arch/x86/lib/memmove_64.S | 4 ++--
 arch/x86/lib/memset_64.S  | 4 ++--
 arch/x86/xen/xen-asm_64.S | 4 ++--
 5 files changed, 10 insertions(+), 11 deletions(-)

diff --git a/arch/x86/crypto/aesni-intel_asm.S 
b/arch/x86/crypto/aesni-intel_asm.S
index 1d34d5a14682..426a22192d69 100644
--- a/arch/x86/crypto/aesni-intel_asm.S
+++ b/arch/x86/crypto/aesni-intel_asm.S
@@ -1873,8 +1873,7 @@ ENDPROC(aesni_gcm_enc)
 #endif
 
 
-.align 4
-_key_expansion_128:
+SYM_FUNC_START_LOCAL_ALIAS(_key_expansion_128)
 SYM_FUNC_START_LOCAL(_key_expansion_256a)
pshufd $0b, %xmm1, %xmm1
shufps $0b0001, %xmm0, %xmm4
@@ -1885,8 +1884,8 @@ SYM_FUNC_START_LOCAL(_key_expansion_256a)
movaps %xmm0, (TKEYP)
add $0x10, TKEYP
ret
-ENDPROC(_key_expansion_128)
 SYM_FUNC_END(_key_expansion_256a)
+SYM_FUNC_END_ALIAS(_key_expansion_128)
 
 SYM_FUNC_START_LOCAL(_key_expansion_192a)
pshufd $0b01010101, %xmm1, %xmm1
diff --git a/arch/x86/lib/memcpy_64.S b/arch/x86/lib/memcpy_64.S
index 9a53a06e5a3e..4911b1c61aa8 100644
--- a/arch/x86/lib/memcpy_64.S
+++ b/arch/x86/lib/memcpy_64.S
@@ -26,7 +26,7 @@
  * Output:
  * rax original destination
  */
-ENTRY(__memcpy)
+SYM_FUNC_START_ALIAS(__memcpy)
 ENTRY(memcpy)
ALTERNATIVE_2 "jmp memcpy_orig", "", X86_FEATURE_REP_GOOD, \
  "jmp memcpy_erms", X86_FEATURE_ERMS
@@ -40,7 +40,7 @@ ENTRY(memcpy)
rep movsb
ret
 ENDPROC(memcpy)
-ENDPROC(__memcpy)
+SYM_FUNC_END_ALIAS(__memcpy)
 EXPORT_SYMBOL(memcpy)
 EXPORT_SYMBOL(__memcpy)
 
diff --git a/arch/x86/lib/memmove_64.S b/arch/x86/lib/memmove_64.S
index bbec69d8223b..50c1648311b3 100644
--- a/arch/x86/lib/memmove_64.S
+++ b/arch/x86/lib/memmove_64.S
@@ -26,7 +26,7 @@
  */
 .weak memmove
 
-ENTRY(memmove)
+SYM_FUNC_START_ALIAS(memmove)
 ENTRY(__memmove)
 
/* Handle more 32 bytes in loop */
@@ -208,6 +208,6 @@ ENTRY(__memmove)
 13:
retq
 ENDPROC(__memmove)
-ENDPROC(memmove)
+SYM_FUNC_END_ALIAS(memmove)
 EXPORT_SYMBOL(__memmove)
 EXPORT_SYMBOL(memmove)
diff --git a/arch/x86/lib/memset_64.S b/arch/x86/lib/memset_64.S
index 9bc861c71e75..927ac44d34aa 100644
--- a/arch/x86/lib/memset_64.S
+++ b/arch/x86/lib/memset_64.S
@@ -19,7 +19,7 @@
  *
  * rax   original destination
  */
-ENTRY(memset)
+SYM_FUNC_START_ALIAS(memset)
 ENTRY(__memset)
/*
 * Some CPUs support enhanced REP MOVSB/STOSB feature. It is recommended
@@ -43,8 +43,8 @@ ENTRY(__memset)
rep stosb
movq %r9,%rax
ret
-ENDPROC(memset)
 ENDPROC(__memset)
+SYM_FUNC_END_ALIAS(memset)
 EXPORT_SYMBOL(memset)
 EXPORT_SYMBOL(__memset)
 
diff --git a/arch/x86/xen/xen-asm_64.S b/arch/x86/xen/xen-asm_64.S
index 8a10c9a9e2b5..0820db159053 100644
--- a/arch/x86/xen/xen-asm_64.S
+++ b/arch/x86/xen/xen-asm_64.S
@@ -150,13 +150,13 @@ ENDPROC(xen_sysenter_target)
 
 #else /* !CONFIG_IA32_EMULATION */
 
-ENTRY(xen_syscall32_target)
+SYM_FUNC_START_ALIAS(xen_syscall32_target)
 ENTRY(xen_sysenter_target)
lea 16(%rsp), %rsp  /* strip %rcx, %r11 */
mov $-ENOSYS, %rax
pushq $0
jmp hypercall_iret
-ENDPROC(xen_syscall32_target)
 ENDPROC(xen_sysenter_target)
+SYM_FUNC_END_ALIAS(xen_syscall32_target)
 
 #endif /* CONFIG_IA32_EMULATION */
-- 
2.15.0

[PATCH v5 23/27] x86_64: assembly, change all ENTRY+ENDPROC to SYM_FUNC_*

[Jiri Slaby]

These are all functions which are invoked from elsewhere, so we annotate
them as global using the new SYM_FUNC_START. And their ENDPROC's by
SYM_FUNC_END.

And make sure ENTRY/ENDPROC is not defined on X86_64, given these were
the last users.

Signed-off-by: Jiri Slaby 
Reviewed-by: Rafael J. Wysocki  [hibernate]
Reviewed-by: Boris Ostrovsky  [xen bits]
Cc: "H. Peter Anvin" 
Cc: Thomas Gleixner 
Cc: Ingo Molnar 
Cc: x...@kernel.org
Cc: Herbert Xu 
Cc: "David S. Miller" 
Cc: "Rafael J. Wysocki" 
Cc: Len Brown 
Cc: Pavel Machek 
Cc: Matt Fleming 
Cc: Ard Biesheuvel 
Cc: Boris Ostrovsky 
Cc: Juergen Gross 
Cc: linux-crypto@vger.kernel.org
Cc: linux...@vger.kernel.org
Cc: linux-...@vger.kernel.org
Cc: xen-de...@lists.xenproject.org
---
 arch/x86/boot/compressed/efi_thunk_64.S|  4 +-
 arch/x86/boot/compressed/head_64.S | 16 
 arch/x86/boot/compressed/mem_encrypt.S |  8 ++--
 arch/x86/crypto/aes-i586-asm_32.S  |  8 ++--
 arch/x86/crypto/aes-x86_64-asm_64.S|  4 +-
 arch/x86/crypto/aes_ctrby8_avx-x86_64.S| 12 +++---
 arch/x86/crypto/aesni-intel_asm.S  | 44 +++---
 arch/x86/crypto/aesni-intel_avx-x86_64.S   | 24 ++--
 arch/x86/crypto/blowfish-x86_64-asm_64.S   | 16 
 arch/x86/crypto/camellia-aesni-avx-asm_64.S| 24 ++--
 arch/x86/crypto/camellia-aesni-avx2-asm_64.S   | 24 ++--
 arch/x86/crypto/camellia-x86_64-asm_64.S   | 16 
 arch/x86/crypto/cast5-avx-x86_64-asm_64.S  | 16 
 arch/x86/crypto/cast6-avx-x86_64-asm_64.S  | 24 ++--
 arch/x86/crypto/chacha20-avx2-x86_64.S |  4 +-
 arch/x86/crypto/chacha20-ssse3-x86_64.S|  8 ++--
 arch/x86/crypto/crc32-pclmul_asm.S |  4 +-
 arch/x86/crypto/crc32c-pcl-intel-asm_64.S  |  4 +-
 arch/x86/crypto/crct10dif-pcl-asm_64.S |  4 +-
 arch/x86/crypto/des3_ede-asm_64.S  |  8 ++--
 arch/x86/crypto/ghash-clmulni-intel_asm.S  |  8 ++--
 arch/x86/crypto/poly1305-avx2-x86_64.S |  4 +-
 arch/x86/crypto/poly1305-sse2-x86_64.S |  8 ++--
 arch/x86/crypto/salsa20-x86_64-asm_64.S| 12 +++---
 arch/x86/crypto/serpent-avx-x86_64-asm_64.S| 24 ++--
 arch/x86/crypto/serpent-avx2-asm_64.S  | 24 ++--
 arch/x86/crypto/serpent-sse2-x86_64-asm_64.S   |  8 ++--
 arch/x86/crypto/sha1-mb/sha1_mb_mgr_flush_avx2.S   |  8 ++--
 arch/x86/crypto/sha1-mb/sha1_mb_mgr_submit_avx2.S  |  4 +-
 arch/x86/crypto/sha1-mb/sha1_x8_avx2.S |  4 +-
 arch/x86/crypto/sha1_avx2_x86_64_asm.S |  4 +-
 arch/x86/crypto/sha1_ni_asm.S  |  4 +-
 arch/x86/crypto/sha1_ssse3_asm.S   |  4 +-
 arch/x86/crypto/sha256-avx-asm.S   |  4 +-
 arch/x86/crypto/sha256-avx2-asm.S  |  4 +-
 .../crypto/sha256-mb/sha256_mb_mgr_flush_avx2.S|  8 ++--
 .../crypto/sha256-mb/sha256_mb_mgr_submit_avx2.S   |  4 +-
 arch/x86/crypto/sha256-mb/sha256_x8_avx2.S |  4 +-
 arch/x86/crypto/sha256-ssse3-asm.S |  4 +-
 arch/x86/crypto/sha256_ni_asm.S|  4 +-
 arch/x86/crypto/sha512-avx-asm.S   |  4 +-
 arch/x86/crypto/sha512-avx2-asm.S  |  4 +-
 .../crypto/sha512-mb/sha512_mb_mgr_flush_avx2.S|  8 ++--
 .../crypto/sha512-mb/sha512_mb_mgr_submit_avx2.S   |  4 +-
 arch/x86/crypto/sha512-mb/sha512_x4_avx2.S |  4 +-
 arch/x86/crypto/sha512-ssse3-asm.S |  4 +-
 arch/x86/crypto/twofish-avx-x86_64-asm_64.S| 24 ++--
 arch/x86/crypto/twofish-x86_64-asm_64-3way.S   |  8 ++--
 arch/x86/crypto/twofish-x86_64-asm_64.S|  8 ++--
 arch/x86/entry/entry_64.S  | 10 ++---
 arch/x86/entry/entry_64_compat.S   |  8 ++--
 arch/x86/kernel/acpi/wakeup_64.S   |  8 ++--
 arch/x86/kernel/head_64.S  | 12 +++---
 arch/x86/lib/checksum_32.S |  8 ++--
 arch/x86/lib/clear_page_64.S   | 12 +++---
 arch/x86/lib/cmpxchg16b_emu.S  |  4 +-
 arch/x86/lib/cmpxchg8b_emu.S   |  4 +-
 arch/x86/lib/copy_page_64.S|  4 +-
 arch/x86/lib/copy_user_64.S| 16 
 arch/x86/lib/csum-copy_64.S|  4 +-
 arch/x86/lib/getuser.S | 16 
 arch/x86/lib/hweight.S |  8 ++--
 arch/x86/lib/iomap_copy_64.S  

[PATCH v5 26/27] x86_32: assembly, change all ENTRY+ENDPROC to SYM_FUNC_*

[Jiri Slaby]

These are all functions which are invoked from elsewhere, so we annotate
them as global using the new SYM_FUNC_START (and their ENDPROC's by
SYM_FUNC_END.)

Now, we can finally force ENTRY/ENDPROC to be undefined on X86.

Signed-off-by: Jiri Slaby 
Cc: "H. Peter Anvin" 
Cc: Thomas Gleixner 
Cc: Ingo Molnar 
Cc: x...@kernel.org
Cc: Herbert Xu 
Cc: "David S. Miller" 
Cc: Bill Metzenthen 
Cc: Matt Fleming 
Cc: Ard Biesheuvel 
Cc: linux-crypto@vger.kernel.org
Cc: linux-...@vger.kernel.org
---
 arch/x86/boot/compressed/efi_stub_32.S |  4 ++--
 arch/x86/boot/compressed/head_32.S | 12 +--
 arch/x86/crypto/salsa20-i586-asm_32.S  | 12 +--
 arch/x86/crypto/serpent-sse2-i586-asm_32.S |  8 
 arch/x86/crypto/twofish-i586-asm_32.S  |  8 
 arch/x86/entry/entry_32.S  | 24 +++---
 arch/x86/kernel/head_32.S  | 16 +++
 arch/x86/lib/atomic64_386_32.S |  4 ++--
 arch/x86/lib/atomic64_cx8_32.S | 32 +++---
 arch/x86/lib/checksum_32.S |  8 
 arch/x86/math-emu/div_Xsig.S   |  4 ++--
 arch/x86/math-emu/div_small.S  |  4 ++--
 arch/x86/math-emu/mul_Xsig.S   | 12 +--
 arch/x86/math-emu/polynom_Xsig.S   |  4 ++--
 arch/x86/math-emu/reg_norm.S   |  8 
 arch/x86/math-emu/reg_round.S  |  4 ++--
 arch/x86/math-emu/reg_u_add.S  |  4 ++--
 arch/x86/math-emu/reg_u_div.S  |  4 ++--
 arch/x86/math-emu/reg_u_mul.S  |  4 ++--
 arch/x86/math-emu/reg_u_sub.S  |  4 ++--
 arch/x86/math-emu/round_Xsig.S |  8 
 arch/x86/math-emu/shr_Xsig.S   |  4 ++--
 arch/x86/math-emu/wm_shrx.S|  8 
 arch/x86/math-emu/wm_sqrt.S|  4 ++--
 arch/x86/platform/efi/efi_stub_32.S|  4 ++--
 include/linux/linkage.h|  8 +++-
 26 files changed, 107 insertions(+), 109 deletions(-)

diff --git a/arch/x86/boot/compressed/efi_stub_32.S 
b/arch/x86/boot/compressed/efi_stub_32.S
index 257e341fd2c8..ed6c351d34ed 100644
--- a/arch/x86/boot/compressed/efi_stub_32.S
+++ b/arch/x86/boot/compressed/efi_stub_32.S
@@ -24,7 +24,7 @@
  */
 
 .text
-ENTRY(efi_call_phys)
+SYM_FUNC_START(efi_call_phys)
/*
 * 0. The function can only be called in Linux kernel. So CS has been
 * set to 0x0010, DS and SS have been set to 0x0018. In EFI, I found
@@ -77,7 +77,7 @@ ENTRY(efi_call_phys)
movlsaved_return_addr(%edx), %ecx
pushl   %ecx
ret
-ENDPROC(efi_call_phys)
+SYM_FUNC_END(efi_call_phys)
 .previous
 
 .data
diff --git a/arch/x86/boot/compressed/head_32.S 
b/arch/x86/boot/compressed/head_32.S
index 7e8ab0bb6968..3fa36496af12 100644
--- a/arch/x86/boot/compressed/head_32.S
+++ b/arch/x86/boot/compressed/head_32.S
@@ -61,7 +61,7 @@
.hidden _egot
 
__HEAD
-ENTRY(startup_32)
+SYM_FUNC_START(startup_32)
cld
/*
 * Test KEEP_SEGMENTS flag to see if the bootloader is asking
@@ -142,14 +142,14 @@ ENTRY(startup_32)
  */
lealrelocated(%ebx), %eax
jmp *%eax
-ENDPROC(startup_32)
+SYM_FUNC_END(startup_32)
 
 #ifdef CONFIG_EFI_STUB
 /*
  * We don't need the return address, so set up the stack so efi_main() can find
  * its arguments.
  */
-ENTRY(efi_pe_entry)
+SYM_FUNC_START(efi_pe_entry)
add $0x4, %esp
 
call1f
@@ -174,9 +174,9 @@ ENTRY(efi_pe_entry)
pushl   %eax
pushl   %ecx
jmp 2f  /* Skip efi_config initialization */
-ENDPROC(efi_pe_entry)
+SYM_FUNC_END(efi_pe_entry)
 
-ENTRY(efi32_stub_entry)
+SYM_FUNC_START(efi32_stub_entry)
add $0x4, %esp
popl%ecx
popl%edx
@@ -205,7 +205,7 @@ fail:
movlBP_code32_start(%esi), %eax
lealstartup_32(%eax), %eax
jmp *%eax
-ENDPROC(efi32_stub_entry)
+SYM_FUNC_END(efi32_stub_entry)
 #endif
 
.text
diff --git a/arch/x86/crypto/salsa20-i586-asm_32.S 
b/arch/x86/crypto/salsa20-i586-asm_32.S
index 329452b8f794..e9a6703056fc 100644
--- a/arch/x86/crypto/salsa20-i586-asm_32.S
+++ b/arch/x86/crypto/salsa20-i586-asm_32.S
@@ -7,7 +7,7 @@
 .text
 
 # enter salsa20_encrypt_bytes
-ENTRY(salsa20_encrypt_bytes)
+SYM_FUNC_START(salsa20_encrypt_bytes)
mov %esp,%eax
and $31,%eax
add $256,%eax
@@ -934,10 +934,10 @@ ENTRY(salsa20_encrypt_bytes)
add $64,%esi
# goto bytesatleast1
jmp ._bytesatleast1
-ENDPROC(salsa20_encrypt_bytes)
+SYM_FUNC_END(salsa20_encrypt_bytes)
 
 # enter salsa20_keysetup
-ENTRY(salsa20_keysetup)
+SYM_FUNC_START(salsa20_keysetup)
mov %esp,%eax
   

Re: [PATCH v2 3/4] crypto: inside-secure - only update the result buffer when provided

[Antoine Tenart]

Hi Kamil,

On Thu, Nov 30, 2017 at 03:10:28PM +0100, Kamil Konieczny wrote:
> On 30.11.2017 13:41, Antoine Tenart wrote:
> > 
> > No, if we do this we'll lose the ability to export the current state.
> 
> So maybe save it into request context:
> 
>   result_sz = crypto_ahash_digestsize(ahash);
>   ctx = ahash_request_ctx(areq);
> 
>   if (sreq->finish)
>   memcpy(sreq->state, areq->result, result_sz);
>   else
>   memcpy(sreq->state, ctx->state, result_sz);

Storing the digest into a driver own buffer could improve the export
ability in some *very rare* cases. If so I would suggest not to deal
with the kind of test you proposed, but to have your own buffer used
each time.

Anyway, this has nothing to do with the fix I'm proposing here, as it
would change the driver's logic, and would solve a complete different
(rare) issue.

The proposal here is to have a simple fix (which is similar to what can
be found in some other drivers), that can easily be backported to avoid
NULL pointer dereferences in older versions of the kernel.

Thanks,
Antoine

-- 
Antoine Ténart, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

[PATCH] chcr: ensure cntrl is initialized to fix bit-wise or'ing of garabage data

[Colin King]

From: Colin Ian King 

In the case where skb->ip_summed != CHECKSUM_PARTIAL then cntrl contains
garbage value and this is possibly being bit-wise or'd and stored into
cpl->ctrl1.  Fix this by initializing cntrl to zero.

Cleans up clang warning:
drivers/crypto/chelsio/chcr_ipsec.c:374:9: warning: The left expression
of the compound assignment is an uninitialized value. The computed value
will also be garbage

Fixes: 6dad4e8ab3ec ("chcr: Add support for Inline IPSec")
Signed-off-by: Colin Ian King 
---
 drivers/crypto/chelsio/chcr_ipsec.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/crypto/chelsio/chcr_ipsec.c 
b/drivers/crypto/chelsio/chcr_ipsec.c
index a0f003ffd1f4..64a030f8fb21 100644
--- a/drivers/crypto/chelsio/chcr_ipsec.c
+++ b/drivers/crypto/chelsio/chcr_ipsec.c
@@ -350,7 +350,7 @@ inline void *copy_cpltx_pktxt(struct sk_buff *skb,
struct port_info *pi;
struct sge_eth_txq *q;
struct cpl_tx_pkt_core *cpl;
-   u64 cntrl;
+   u64 cntrl = 0;
u32 ctrl0, qidx;
 
pi = netdev_priv(dev);
-- 
2.14.1

[PATCH] chcr: remove unused variables net_device, pi, adap and cntrl

[Colin King]

From: Colin Ian King 

Variables adap, pi and cntrl are assigned but are never read, hence
they are redundant and can be removed.

Cleans up various clang build warnings.

Signed-off-by: Colin Ian King 
---
 drivers/crypto/chelsio/chcr_ipsec.c | 8 +---
 1 file changed, 1 insertion(+), 7 deletions(-)

diff --git a/drivers/crypto/chelsio/chcr_ipsec.c 
b/drivers/crypto/chelsio/chcr_ipsec.c
index f90f991c133f..a0f003ffd1f4 100644
--- a/drivers/crypto/chelsio/chcr_ipsec.c
+++ b/drivers/crypto/chelsio/chcr_ipsec.c
@@ -199,14 +199,9 @@ static inline int chcr_ipsec_setkey(struct xfrm_state *x,
  */
 static int chcr_xfrm_add_state(struct xfrm_state *x)
 {
-   struct net_device *netdev = x->xso.dev;
-   struct port_info *pi  = netdev_priv(netdev);
struct ipsec_sa_entry *sa_entry;
-   struct adapter *adap;
int res = 0;
 
-   adap = pi->adapter;
-
if (x->props.aalgo != SADB_AALG_NONE) {
pr_debug("CHCR: Cannot offload authenticated xfrm states\n");
return -EINVAL;
@@ -570,7 +565,7 @@ int chcr_ipsec_xmit(struct sk_buff *skb, struct net_device 
*dev)
 {
struct xfrm_state *x = xfrm_input_state(skb);
struct ipsec_sa_entry *sa_entry;
-   u64 *pos, *end, *before, cntrl, *sgl;
+   u64 *pos, *end, *before, *sgl;
int qidx, left, credits;
unsigned int flits = 0, ndesc, kctx_len;
struct adapter *adap;
@@ -596,7 +591,6 @@ out_free:   dev_kfree_skb_any(skb);
q = >sge.ethtxq[qidx + pi->first_qset];
 
cxgb4_reclaim_completed_tx(adap, >q, true);
-   cntrl = TXPKT_L4CSUM_DIS_F | TXPKT_IPCSUM_DIS_F;
 
flits = calc_tx_sec_flits(skb, sa_entry->kctx_len);
ndesc = flits_to_desc(flits);
-- 
2.14.1

Re: [PATCH v2 3/4] crypto: inside-secure - only update the result buffer when provided

[Antoine Tenart]

On Thu, Nov 30, 2017 at 12:52:42PM +0100, Kamil Konieczny wrote:
> On 30.11.2017 10:29, Antoine Tenart wrote:
> > On Thu, Nov 30, 2017 at 10:19:26AM +0100, Kamil Konieczny wrote:
> >> can the driver get request for final/finup/digest with null req->result ?
> > 
> > I don't think that can happen. But having an update called without
> > req->result provided is a valid call (though it's not well documented).
> 
> so maybe:
> 
>   if (sreq->finish) {
>   result_sz = crypto_ahash_digestsize(ahash);
>   memcpy(sreq->state, areq->result, result_sz);
>   }

No, if we do this we'll lose the ability to export the current state.

Thanks,
Antoine

-- 
Antoine Ténart, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

[PATCH] crypto: mcryptd: protect the per-CPU queue with a lock

[Sebastian Andrzej Siewior]

mcryptd_enqueue_request() grabs the per-CPU queue struct and protects
access to it with disabled preemption. Then it schedules a worker on the
same CPU. The worker in mcryptd_queue_worker() guards access to the same
per-CPU variable with disabled preemption.

If we take CPU-hotplug into account then it is possible that between
queue_work_on() and the actual invocation of the worker the CPU goes
down and the worker will be scheduled on _another_ CPU. And here the
preempt_disable() protection does not work anymore. The easiest thing is
to add a spin_lock() to guard access to the list.

Another detail: mcryptd_queue_worker() is not processing more than
MCRYPTD_BATCH invocation in a row. If there are still items left, then
it will invoke queue_work() to proceed with more later. *I* would
suggest to simply drop that check because it does not use a system
workqueue and the workqueue is already marked as "CPU_INTENSIVE". And if
preemption is required then the scheduler should do it.
However if queue_work() is used then the work item is marked as CPU
unbound. That means it will try to run on the local CPU but it may run
on another CPU as well. Especially with CONFIG_DEBUG_WQ_FORCE_RR_CPU=y.
Again, the preempt_disable() won't work here but lock which was
introduced will help.
In order to keep work-item on the local CPU (and avoid RR) I changed it
to queue_work_on().

Cc: sta...@vger.kernel.org
Signed-off-by: Sebastian Andrzej Siewior 
---
 crypto/mcryptd.c | 23 ++-
 include/crypto/mcryptd.h |  1 +
 2 files changed, 11 insertions(+), 13 deletions(-)

diff --git a/crypto/mcryptd.c b/crypto/mcryptd.c
index 4e6472658852..eca04d3729b3 100644
--- a/crypto/mcryptd.c
+++ b/crypto/mcryptd.c
@@ -81,6 +81,7 @@ static int mcryptd_init_queue(struct mcryptd_queue *queue,
pr_debug("cpu_queue #%d %p\n", cpu, queue->cpu_queue);
crypto_init_queue(_queue->queue, max_cpu_qlen);
INIT_WORK(_queue->work, mcryptd_queue_worker);
+   spin_lock_init(_queue->q_lock);
}
return 0;
 }
@@ -104,15 +105,16 @@ static int mcryptd_enqueue_request(struct mcryptd_queue 
*queue,
int cpu, err;
struct mcryptd_cpu_queue *cpu_queue;
 
-   cpu = get_cpu();
-   cpu_queue = this_cpu_ptr(queue->cpu_queue);
-   rctx->tag.cpu = cpu;
+   cpu_queue = raw_cpu_ptr(queue->cpu_queue);
+   spin_lock(_queue->q_lock);
+   cpu = smp_processor_id();
+   rctx->tag.cpu = smp_processor_id();
 
err = crypto_enqueue_request(_queue->queue, request);
pr_debug("enqueue request: cpu %d cpu_queue %p request %p\n",
 cpu, cpu_queue, request);
+   spin_unlock(_queue->q_lock);
queue_work_on(cpu, kcrypto_wq, _queue->work);
-   put_cpu();
 
return err;
 }
@@ -161,16 +163,11 @@ static void mcryptd_queue_worker(struct work_struct *work)
cpu_queue = container_of(work, struct mcryptd_cpu_queue, work);
i = 0;
while (i < MCRYPTD_BATCH || single_task_running()) {
-   /*
-* preempt_disable/enable is used to prevent
-* being preempted by mcryptd_enqueue_request()
-*/
-   local_bh_disable();
-   preempt_disable();
+
+   spin_lock_bh(_queue->q_lock);
backlog = crypto_get_backlog(_queue->queue);
req = crypto_dequeue_request(_queue->queue);
-   preempt_enable();
-   local_bh_enable();
+   spin_unlock_bh(_queue->q_lock);
 
if (!req) {
mcryptd_opportunistic_flush();
@@ -185,7 +182,7 @@ static void mcryptd_queue_worker(struct work_struct *work)
++i;
}
if (cpu_queue->queue.qlen)
-   queue_work(kcrypto_wq, _queue->work);
+   queue_work_on(smp_processor_id(), kcrypto_wq, _queue->work);
 }
 
 void mcryptd_flusher(struct work_struct *__work)
diff --git a/include/crypto/mcryptd.h b/include/crypto/mcryptd.h
index cceafa01f907..b67404fc4b34 100644
--- a/include/crypto/mcryptd.h
+++ b/include/crypto/mcryptd.h
@@ -27,6 +27,7 @@ static inline struct mcryptd_ahash *__mcryptd_ahash_cast(
 
 struct mcryptd_cpu_queue {
struct crypto_queue queue;
+   spinlock_t q_lock;
struct work_struct work;
 };
 
-- 
2.15.0

Re: [PATCH v2 3/4] crypto: inside-secure - only update the result buffer when provided

[Kamil Konieczny]

Hi Antoine,

On 30.11.2017 10:29, Antoine Tenart wrote:
> Hi Kamil,
> 
> On Thu, Nov 30, 2017 at 10:19:26AM +0100, Kamil Konieczny wrote:
>> On 28.11.2017 16:42, Antoine Tenart wrote:
>>> The patch fixes the ahash support by only updating the result buffer
>>> when provided. Otherwise the driver could crash with NULL pointer
>>> exceptions, because the ahash caller isn't required to supply a result
>>> buffer on all calls.
>>
>> Can you point to bug crush report ?
> 
> Do you want the crash dump? (It'll only be a "normal" NULL pointer
> dereference).

Ah I see, in this case not.

>>> Fixes: 1b44c5a60c13 ("crypto: inside-secure - add SafeXcel EIP197 crypto 
>>> engine driver")
>>> Signed-off-by: Antoine Tenart 
>>> ---
>>>  drivers/crypto/inside-secure/safexcel_hash.c | 7 ++-
>>>  1 file changed, 6 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/crypto/inside-secure/safexcel_hash.c 
>>> b/drivers/crypto/inside-secure/safexcel_hash.c
>>> index 6135c9f5742c..424f4c5d4d25 100644
>>> --- a/drivers/crypto/inside-secure/safexcel_hash.c
>>> +++ b/drivers/crypto/inside-secure/safexcel_hash.c
>>> @@ -150,7 +150,12 @@ static int safexcel_handle_req_result(struct 
>>> safexcel_crypto_priv *priv, int rin
>>>  
>>> if (sreq->finish)
>>> result_sz = crypto_ahash_digestsize(ahash);
>>> -   memcpy(sreq->state, areq->result, result_sz);
>> [...]
>> can the driver get request for final/finup/digest with null req->result ?
> 
> I don't think that can happen. But having an update called without
> req->result provided is a valid call (though it's not well documented).

so maybe:

if (sreq->finish) {
result_sz = crypto_ahash_digestsize(ahash);
memcpy(sreq->state, areq->result, result_sz);
}

-- 
Best regards,
Kamil Konieczny
Samsung R Institute Poland

[PATCH] crypto: chelsio: make arrays sgl_ent_len and dsgl_ent_len static

[Colin King]

From: Colin Ian King 

The arrays sgl_ent_len and dsgl_ent_len are local to the source and do
not need to be in global scope, so make them static. Also re-format the
declarations to match the following round_constant array declaration
style.

Cleans up sparse warnings:
drivers/crypto/chelsio/chcr_algo.c:76:14: warning: symbol 'sgl_ent_len'
was not declared. Should it be static?
drivers/crypto/chelsio/chcr_algo.c:81:14: warning: symbol 'dsgl_ent_len'
was not declared. Should it be static?

Signed-off-by: Colin Ian King 
---
 drivers/crypto/chelsio/chcr_algo.c | 22 +-
 1 file changed, 13 insertions(+), 9 deletions(-)

diff --git a/drivers/crypto/chelsio/chcr_algo.c 
b/drivers/crypto/chelsio/chcr_algo.c
index e61ec8a46340..b663b93b7d01 100644
--- a/drivers/crypto/chelsio/chcr_algo.c
+++ b/drivers/crypto/chelsio/chcr_algo.c
@@ -73,15 +73,19 @@
 
 #define IV AES_BLOCK_SIZE
 
-unsigned int sgl_ent_len[] = {0, 0, 16, 24, 40, 48, 64, 72, 88,
-   96, 112, 120, 136, 144, 160, 168, 184,
-   192, 208, 216, 232, 240, 256, 264, 280,
-   288, 304, 312, 328, 336, 352, 360, 376};
-
-unsigned int dsgl_ent_len[] = {0, 32, 32, 48, 48, 64, 64, 80, 80,
-   112, 112, 128, 128, 144, 144, 160, 160,
-   192, 192, 208, 208, 224, 224, 240, 240,
-   272, 272, 288, 288, 304, 304, 320, 320};
+static unsigned int sgl_ent_len[] = {
+   0, 0, 16, 24, 40, 48, 64, 72, 88,
+   96, 112, 120, 136, 144, 160, 168, 184,
+   192, 208, 216, 232, 240, 256, 264, 280,
+   288, 304, 312, 328, 336, 352, 360, 376
+};
+
+static unsigned int dsgl_ent_len[] = {
+   0, 32, 32, 48, 48, 64, 64, 80, 80,
+   112, 112, 128, 128, 144, 144, 160, 160,
+   192, 192, 208, 208, 224, 224, 240, 240,
+   272, 272, 288, 288, 304, 304, 320, 320
+};
 
 static u32 round_constant[11] = {
0x0100, 0x0200, 0x0400, 0x0800,
-- 
2.14.1

[PATCH] crypto: cryptd: make cryptd_max_cpu_qlen module parameter static

[Colin King]

From: Colin Ian King 

The cryptd_max_cpu_qlen module parameter is local to the source and does
not need to be in global scope, so make it static.

Cleans up sparse warning:
crypto/cryptd.c:35:14: warning: symbol 'cryptd_max_cpu_qlen' was not
declared. Should it be static?

Signed-off-by: Colin Ian King 
---
 crypto/cryptd.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/crypto/cryptd.c b/crypto/cryptd.c
index b1eb131c0f10..552e3a86e829 100644
--- a/crypto/cryptd.c
+++ b/crypto/cryptd.c
@@ -32,7 +32,7 @@
 #include 
 #include 
 
-unsigned int cryptd_max_cpu_qlen = 1000;
+static unsigned int cryptd_max_cpu_qlen = 1000;
 module_param(cryptd_max_cpu_qlen, uint, 0);
 MODULE_PARM_DESC(cryptd_max_cpu_qlen, "Set cryptd Max queue depth");
 
-- 
2.14.1

Re: [PATCH] crypto: stm32: fix module device table name

[Fabien DESSENNE]

Hi Corentin


Thank you for the patch.


On 30/11/17 12:04, Corentin Labbe wrote:
> This patch fix the following build failure:
>CC [M]  drivers/crypto/stm32/stm32-cryp.o
> In file included from drivers/crypto/stm32/stm32-cryp.c:11:0:
> drivers/crypto/stm32/stm32-cryp.c:1049:25: error: 'sti_dt_ids' undeclared 
> here (not in a function)
>   MODULE_DEVICE_TABLE(of, sti_dt_ids);
>
> Let's replace sti_dt_ids with stm32_dt_ids which is just declared
> before.
>
> Signed-off-by: Corentin Labbe 

Reviewed-by: Fabien Dessenne 

> ---
>   drivers/crypto/stm32/stm32-cryp.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/crypto/stm32/stm32-cryp.c 
> b/drivers/crypto/stm32/stm32-cryp.c
> index 098fad266c41..1807789b23f5 100644
> --- a/drivers/crypto/stm32/stm32-cryp.c
> +++ b/drivers/crypto/stm32/stm32-cryp.c
> @@ -1046,7 +1046,7 @@ static const struct of_device_id stm32_dt_ids[] = {
>   { .compatible = "st,stm32f756-cryp", },
>   {},
>   };
> -MODULE_DEVICE_TABLE(of, sti_dt_ids);
> +MODULE_DEVICE_TABLE(of, stm32_dt_ids);
>   
>   static int stm32_cryp_probe(struct platform_device *pdev)
>   {

[PATCH] crypto: stm32: fix module device table name

[Corentin Labbe]

This patch fix the following build failure:
  CC [M]  drivers/crypto/stm32/stm32-cryp.o
In file included from drivers/crypto/stm32/stm32-cryp.c:11:0:
drivers/crypto/stm32/stm32-cryp.c:1049:25: error: 'sti_dt_ids' undeclared here 
(not in a function)
 MODULE_DEVICE_TABLE(of, sti_dt_ids);

Let's replace sti_dt_ids with stm32_dt_ids which is just declared
before.

Signed-off-by: Corentin Labbe 
---
 drivers/crypto/stm32/stm32-cryp.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/crypto/stm32/stm32-cryp.c 
b/drivers/crypto/stm32/stm32-cryp.c
index 098fad266c41..1807789b23f5 100644
--- a/drivers/crypto/stm32/stm32-cryp.c
+++ b/drivers/crypto/stm32/stm32-cryp.c
@@ -1046,7 +1046,7 @@ static const struct of_device_id stm32_dt_ids[] = {
{ .compatible = "st,stm32f756-cryp", },
{},
 };
-MODULE_DEVICE_TABLE(of, sti_dt_ids);
+MODULE_DEVICE_TABLE(of, stm32_dt_ids);
 
 static int stm32_cryp_probe(struct platform_device *pdev)
 {
-- 
2.13.6

[PATCH 2/6] crypto: tcrypt: fix AEAD decryption speed test

[Gilad Ben-Yossef]

The AEAD speed test pretended to support decryption, however that support
was broken as decryption requires a valid auth field which the test did
not provide.

Fix this by running the encryption path once with inout/output sgls
switched to calculate the auth field prior to performing decryption
speed tests.

Signed-off-by: Gilad Ben-Yossef 
---
 crypto/tcrypt.c | 30 +-
 1 file changed, 29 insertions(+), 1 deletion(-)

diff --git a/crypto/tcrypt.c b/crypto/tcrypt.c
index d1fd033..56fb6a6 100644
--- a/crypto/tcrypt.c
+++ b/crypto/tcrypt.c
@@ -327,10 +327,30 @@ static void test_aead_speed(const char *algo, int enc, 
unsigned int secs,
sg_set_buf([0], assoc, aad_size);
sg_set_buf([0], assoc, aad_size);
 
+   aead_request_set_ad(req, aad_size);
+
+   if (!enc) {
+
+   /*
+* For decryption we need a proper auth so
+* we do the encryption path once with buffers
+* reversed (input <-> output) to calculate it
+*/
+   aead_request_set_crypt(req, sgout, sg,
+  *b_size, iv);
+   ret = do_one_aead_op(req,
+crypto_aead_encrypt(req));
+
+   if (ret) {
+   pr_err("calculating auth failed failed 
(%d)\n",
+  ret);
+   break;
+   }
+   }
+
aead_request_set_crypt(req, sg, sgout,
   *b_size + (enc ? 0 : authsize),
   iv);
-   aead_request_set_ad(req, aad_size);
 
if (secs)
ret = test_aead_jiffies(req, enc, *b_size,
@@ -1564,16 +1584,24 @@ static int do_test(const char *alg, u32 type, u32 mask, 
int m)
NULL, 0, 16, 16, aead_speed_template_20);
test_aead_speed("gcm(aes)", ENCRYPT, sec,
NULL, 0, 16, 8, speed_template_16_24_32);
+   test_aead_speed("rfc4106(gcm(aes))", DECRYPT, sec,
+   NULL, 0, 16, 16, aead_speed_template_20);
+   test_aead_speed("gcm(aes)", DECRYPT, sec,
+   NULL, 0, 16, 8, speed_template_16_24_32);
break;
 
case 212:
test_aead_speed("rfc4309(ccm(aes))", ENCRYPT, sec,
NULL, 0, 16, 16, aead_speed_template_19);
+   test_aead_speed("rfc4309(ccm(aes))", DECRYPT, sec,
+   NULL, 0, 16, 16, aead_speed_template_19);
break;
 
case 213:
test_aead_speed("rfc7539esp(chacha20,poly1305)", ENCRYPT, sec,
NULL, 0, 16, 8, aead_speed_template_36);
+   test_aead_speed("rfc7539esp(chacha20,poly1305)", DECRYPT, sec,
+   NULL, 0, 16, 8, aead_speed_template_36);
break;
 
case 214:
-- 
2.7.4

[PATCH 1/6] crypto: tcrypt: use multi buf for ahash mb test

[Gilad Ben-Yossef]

The multi buffer ahash speed test was allocating multiple
buffers for use with the multiple outstanding requests
it was starting but never actually using them (except
to free them), instead using a different single statically
allocated buffer for all requests.

Fix this by actually using the allocated buffers for the test.

It is noted that it may seem tempting to instead remove the
allocation and free of the multiple buffers and leave things as
they are since this is a hash test where the input is read
only. However, after consideration I believe that multiple
buffers better reflect real life scenario with regard
to data cache and TLB behaviours etc.

Signed-off-by: Gilad Ben-Yossef 
---
 crypto/tcrypt.c | 13 +
 1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/crypto/tcrypt.c b/crypto/tcrypt.c
index 3ced1ba..d1fd033 100644
--- a/crypto/tcrypt.c
+++ b/crypto/tcrypt.c
@@ -383,7 +383,7 @@ static inline int do_one_ahash_op(struct ahash_request 
*req, int ret)
 }
 
 struct test_mb_ahash_data {
-   struct scatterlist sg[TVMEMSIZE];
+   struct scatterlist sg[XBUFSIZE];
char result[64];
struct ahash_request *req;
struct crypto_wait wait;
@@ -426,7 +426,12 @@ static void test_mb_ahash_speed(const char *algo, unsigned 
int sec,
 
ahash_request_set_callback(data[i].req, 0, crypto_req_done,
   [i].wait);
-   test_hash_sg_init(data[i].sg);
+
+   sg_init_table(data[i].sg, XBUFSIZE);
+   for (j = 0; j < XBUFSIZE; j++) {
+   sg_set_buf(data[i].sg + j, data[i].xbuf[j], PAGE_SIZE);
+   memset(data[i].xbuf[j], 0xff, PAGE_SIZE);
+   }
}
 
pr_info("\ntesting speed of multibuffer %s (%s)\n", algo,
@@ -437,9 +442,9 @@ static void test_mb_ahash_speed(const char *algo, unsigned 
int sec,
if (speed[i].blen != speed[i].plen)
continue;
 
-   if (speed[i].blen > TVMEMSIZE * PAGE_SIZE) {
+   if (speed[i].blen > XBUFSIZE * PAGE_SIZE) {
pr_err("template (%u) too big for tvmem (%lu)\n",
-  speed[i].blen, TVMEMSIZE * PAGE_SIZE);
+  speed[i].blen, XBUFSIZE * PAGE_SIZE);
goto out;
}
 
-- 
2.7.4

[PATCH 4/6] crypto: tcrypt: add multi buf ahash jiffies test

[Gilad Ben-Yossef]

The multi buffer concurrent requests ahash speed test only
supported the cycles mode. Add support for the so called
jiffies mode that test performance of bytes/sec.

We only add support for digest mode at the moment.

Signed-off-by: Gilad Ben-Yossef 
---
 crypto/tcrypt.c | 112 +---
 1 file changed, 82 insertions(+), 30 deletions(-)

diff --git a/crypto/tcrypt.c b/crypto/tcrypt.c
index 447b18c..cf9b44f 100644
--- a/crypto/tcrypt.c
+++ b/crypto/tcrypt.c
@@ -411,13 +411,87 @@ struct test_mb_ahash_data {
char *xbuf[XBUFSIZE];
 };
 
-static void test_mb_ahash_speed(const char *algo, unsigned int sec,
+static inline int do_mult_ahash_op(struct test_mb_ahash_data *data, u32 num_mb)
+{
+   int i, rc[num_mb], err = 0;
+
+   /* Fire up a bunch of concurrent requests */
+   for (i = 0; i < num_mb; i++)
+   rc[i] = crypto_ahash_digest(data[i].req);
+
+   /* Wait for all requests to finish */
+   for (i = 0; i < num_mb; i++) {
+   rc[i] = crypto_wait_req(rc[i], [i].wait);
+
+   if (rc[i]) {
+   pr_info("concurrent request %d error %d\n", i, rc[i]);
+   err = rc[i];
+   }
+   }
+
+   return err;
+}
+
+static int test_mb_ahash_jiffies(struct test_mb_ahash_data *data, int blen,
+int secs, u32 num_mb)
+{
+   unsigned long start, end;
+   int bcount;
+   int ret;
+
+   for (start = jiffies, end = start + secs * HZ, bcount = 0;
+time_before(jiffies, end); bcount++) {
+   ret = do_mult_ahash_op(data, num_mb);
+   if (ret)
+   return ret;
+   }
+
+   pr_cont("%d operations in %d seconds (%ld bytes)\n",
+   bcount * num_mb, secs, (long)bcount * blen * num_mb);
+   return 0;
+}
+
+static int test_mb_ahash_cycles(struct test_mb_ahash_data *data, int blen,
+   u32 num_mb)
+{
+   unsigned long cycles = 0;
+   int ret = 0;
+   int i;
+
+   /* Warm-up run. */
+   for (i = 0; i < 4; i++) {
+   ret = do_mult_ahash_op(data, num_mb);
+   if (ret)
+   goto out;
+   }
+
+   /* The real thing. */
+   for (i = 0; i < 8; i++) {
+   cycles_t start, end;
+
+   start = get_cycles();
+   ret = do_mult_ahash_op(data, num_mb);
+   end = get_cycles();
+
+   if (ret)
+   goto out;
+
+   cycles += end - start;
+   }
+
+out:
+   if (ret == 0)
+   pr_cont("1 operation in %lu cycles (%d bytes)\n",
+   (cycles + 4) / (8 * num_mb), blen);
+
+   return ret;
+}
+
+static void test_mb_ahash_speed(const char *algo, unsigned int secs,
struct hash_speed *speed, u32 num_mb)
 {
struct test_mb_ahash_data *data;
struct crypto_ahash *tfm;
-   unsigned long start, end;
-   unsigned long cycles;
unsigned int i, j, k;
int ret;
 
@@ -481,34 +555,12 @@ static void test_mb_ahash_speed(const char *algo, 
unsigned int sec,
i, speed[i].blen, speed[i].plen,
speed[i].blen / speed[i].plen);
 
-   start = get_cycles();
-
-   for (k = 0; k < num_mb; k++) {
-   ret = crypto_ahash_digest(data[k].req);
-   if (ret == -EINPROGRESS) {
-   ret = 0;
-   continue;
-   }
-
-   if (ret)
-   break;
-
-   crypto_req_done([k].req->base, 0);
-   }
-
-   for (j = 0; j < k; j++) {
-   struct crypto_wait *wait = [j].wait;
-   int wait_ret;
-
-   wait_ret = crypto_wait_req(-EINPROGRESS, wait);
-   if (wait_ret)
-   ret = wait_ret;
-   }
+   if (secs)
+   ret = test_mb_ahash_jiffies(data, speed[i].blen, secs,
+   num_mb);
+   else
+   ret = test_mb_ahash_cycles(data, speed[i].blen, num_mb);
 
-   end = get_cycles();
-   cycles = end - start;
-   pr_cont("%6lu cycles/operation, %4lu cycles/byte\n",
-   cycles, cycles / (num_mb * speed[i].blen));
 
if (ret) {
pr_err("At least one hashing failed ret=%d\n", ret);
-- 
2.7.4

[PATCH 3/6] crypto: tcrypt: allow setting num of bufs

[Gilad Ben-Yossef]

For multiple buffers speed tests, the number of buffers, or
requests, used actually sets the level of parallelism a tfm
provider may utilize to hide latency. The existing number
(of 8) is good for some software based providers but not
enough for many HW providers with deep FIFOs.

Add a module parameter that allows setting the number of
multiple buffers/requests used, leaving the default at 8.

Signed-off-by: Gilad Ben-Yossef 
---
 crypto/tcrypt.c | 31 +++
 1 file changed, 19 insertions(+), 12 deletions(-)

diff --git a/crypto/tcrypt.c b/crypto/tcrypt.c
index 56fb6a6..447b18c 100644
--- a/crypto/tcrypt.c
+++ b/crypto/tcrypt.c
@@ -67,6 +67,7 @@ static char *alg = NULL;
 static u32 type;
 static u32 mask;
 static int mode;
+static u32 num_mb = 8;
 static char *tvmem[TVMEMSIZE];
 
 static char *check[] = {
@@ -411,7 +412,7 @@ struct test_mb_ahash_data {
 };
 
 static void test_mb_ahash_speed(const char *algo, unsigned int sec,
-   struct hash_speed *speed)
+   struct hash_speed *speed, u32 num_mb)
 {
struct test_mb_ahash_data *data;
struct crypto_ahash *tfm;
@@ -420,7 +421,7 @@ static void test_mb_ahash_speed(const char *algo, unsigned 
int sec,
unsigned int i, j, k;
int ret;
 
-   data = kzalloc(sizeof(*data) * 8, GFP_KERNEL);
+   data = kcalloc(num_mb, sizeof(*data), GFP_KERNEL);
if (!data)
return;
 
@@ -431,7 +432,7 @@ static void test_mb_ahash_speed(const char *algo, unsigned 
int sec,
goto free_data;
}
 
-   for (i = 0; i < 8; ++i) {
+   for (i = 0; i < num_mb; ++i) {
if (testmgr_alloc_buf(data[i].xbuf))
goto out;
 
@@ -471,7 +472,7 @@ static void test_mb_ahash_speed(const char *algo, unsigned 
int sec,
if (speed[i].klen)
crypto_ahash_setkey(tfm, tvmem[0], speed[i].klen);
 
-   for (k = 0; k < 8; k++)
+   for (k = 0; k < num_mb; k++)
ahash_request_set_crypt(data[k].req, data[k].sg,
data[k].result, speed[i].blen);
 
@@ -482,7 +483,7 @@ static void test_mb_ahash_speed(const char *algo, unsigned 
int sec,
 
start = get_cycles();
 
-   for (k = 0; k < 8; k++) {
+   for (k = 0; k < num_mb; k++) {
ret = crypto_ahash_digest(data[k].req);
if (ret == -EINPROGRESS) {
ret = 0;
@@ -507,7 +508,7 @@ static void test_mb_ahash_speed(const char *algo, unsigned 
int sec,
end = get_cycles();
cycles = end - start;
pr_cont("%6lu cycles/operation, %4lu cycles/byte\n",
-   cycles, cycles / (8 * speed[i].blen));
+   cycles, cycles / (num_mb * speed[i].blen));
 
if (ret) {
pr_err("At least one hashing failed ret=%d\n", ret);
@@ -516,10 +517,10 @@ static void test_mb_ahash_speed(const char *algo, 
unsigned int sec,
}
 
 out:
-   for (k = 0; k < 8; ++k)
+   for (k = 0; k < num_mb; ++k)
ahash_request_free(data[k].req);
 
-   for (k = 0; k < 8; ++k)
+   for (k = 0; k < num_mb; ++k)
testmgr_free_buf(data[k].xbuf);
 
crypto_free_ahash(tfm);
@@ -1813,19 +1814,23 @@ static int do_test(const char *alg, u32 type, u32 mask, 
int m)
if (mode > 400 && mode < 500) break;
/* fall through */
case 422:
-   test_mb_ahash_speed("sha1", sec, generic_hash_speed_template);
+   test_mb_ahash_speed("sha1", sec, generic_hash_speed_template,
+   num_mb);
if (mode > 400 && mode < 500) break;
/* fall through */
case 423:
-   test_mb_ahash_speed("sha256", sec, generic_hash_speed_template);
+   test_mb_ahash_speed("sha256", sec, generic_hash_speed_template,
+   num_mb);
if (mode > 400 && mode < 500) break;
/* fall through */
case 424:
-   test_mb_ahash_speed("sha512", sec, generic_hash_speed_template);
+   test_mb_ahash_speed("sha512", sec, generic_hash_speed_template,
+   num_mb);
if (mode > 400 && mode < 500) break;
/* fall through */
case 425:
-   test_mb_ahash_speed("sm3", sec, generic_hash_speed_template);
+   test_mb_ahash_speed("sm3", sec, generic_hash_speed_template,
+   num_mb);
if (mode > 400 && mode < 500) break;
/* fall through */
case 499:
@@ -2104,6 +2109,8 @@ module_param(mode, int, 0);
 module_param(sec, uint, 0);
 MODULE_PARM_DESC(sec, "Length in 

[PATCH 5/6] crypto: tcrypt: add multibuf skcipher speed test

[Gilad Ben-Yossef]

The performance of some skcipher tfm providers is affected by
the amount of parallelism possible with the processing.

Introduce an async skcipher concurrent multiple buffer
processing speed test to be able to test performance of such
tfm providers.

Signed-off-by: Gilad Ben-Yossef 
---
 crypto/tcrypt.c | 460 
 1 file changed, 460 insertions(+)

diff --git a/crypto/tcrypt.c b/crypto/tcrypt.c
index cf9b44f..f1417e3 100644
--- a/crypto/tcrypt.c
+++ b/crypto/tcrypt.c
@@ -816,6 +816,254 @@ static void test_hash_speed(const char *algo, unsigned 
int secs,
return test_ahash_speed_common(algo, secs, speed, CRYPTO_ALG_ASYNC);
 }
 
+struct test_mb_skcipher_data {
+   struct scatterlist sg[XBUFSIZE];
+   struct skcipher_request *req;
+   struct crypto_wait wait;
+   char *xbuf[XBUFSIZE];
+};
+
+static int do_mult_acipher_op(struct test_mb_skcipher_data *data, int enc,
+   u32 num_mb)
+{
+   int i, rc[num_mb], err = 0;
+
+   /* Fire up a bunch of concurrent requests */
+   for (i = 0; i < num_mb; i++) {
+   if (enc == ENCRYPT)
+   rc[i] = crypto_skcipher_encrypt(data[i].req);
+   else
+   rc[i] = crypto_skcipher_decrypt(data[i].req);
+   }
+
+   /* Wait for all requests to finish */
+   for (i = 0; i < num_mb; i++) {
+   rc[i] = crypto_wait_req(rc[i], [i].wait);
+
+   if (rc[i]) {
+   pr_info("concurrent request %d error %d\n", i, rc[i]);
+   err = rc[i];
+   }
+   }
+
+   return err;
+}
+
+static int test_mb_acipher_jiffies(struct test_mb_skcipher_data *data, int enc,
+   int blen, int secs, u32 num_mb)
+{
+   unsigned long start, end;
+   int bcount;
+   int ret;
+
+   for (start = jiffies, end = start + secs * HZ, bcount = 0;
+time_before(jiffies, end); bcount++) {
+   ret = do_mult_acipher_op(data, enc, num_mb);
+   if (ret)
+   return ret;
+   }
+
+   pr_cont("%d operations in %d seconds (%ld bytes)\n",
+   bcount * num_mb, secs, (long)bcount * blen * num_mb);
+   return 0;
+}
+
+static int test_mb_acipher_cycles(struct test_mb_skcipher_data *data, int enc,
+  int blen, u32 num_mb)
+{
+   unsigned long cycles = 0;
+   int ret = 0;
+   int i;
+
+   /* Warm-up run. */
+   for (i = 0; i < 4; i++) {
+   ret = do_mult_acipher_op(data, enc, num_mb);
+   if (ret)
+   goto out;
+   }
+
+   /* The real thing. */
+   for (i = 0; i < 8; i++) {
+   cycles_t start, end;
+
+   start = get_cycles();
+   ret = do_mult_acipher_op(data, enc, num_mb);
+   end = get_cycles();
+
+   if (ret)
+   goto out;
+
+   cycles += end - start;
+   }
+
+out:
+   if (ret == 0)
+   pr_cont("1 operation in %lu cycles (%d bytes)\n",
+   (cycles + 4) / (8 * num_mb), blen);
+
+   return ret;
+}
+
+static void test_mb_skcipher_speed(const char *algo, int enc, int secs,
+  struct cipher_speed_template *template,
+  unsigned int tcount, u8 *keysize, u32 num_mb)
+{
+   struct test_mb_skcipher_data *data;
+   struct crypto_skcipher *tfm;
+   unsigned int i, j, iv_len;
+   const char *key;
+   const char *e;
+   u32 *b_size;
+   char iv[128];
+   int ret;
+
+   if (enc == ENCRYPT)
+   e = "encryption";
+   else
+   e = "decryption";
+
+   data = kcalloc(num_mb, sizeof(*data), GFP_KERNEL);
+   if (!data)
+   return;
+
+   tfm = crypto_alloc_skcipher(algo, 0, 0);
+   if (IS_ERR(tfm)) {
+   pr_err("failed to load transform for %s: %ld\n",
+   algo, PTR_ERR(tfm));
+   goto out_free_data;
+   }
+
+   for (i = 0; i < num_mb; ++i)
+   if (testmgr_alloc_buf(data[i].xbuf)) {
+   while (i--)
+   testmgr_free_buf(data[i].xbuf);
+   goto out_free_tfm;
+   }
+
+
+   for (i = 0; i < num_mb; ++i)
+   if (testmgr_alloc_buf(data[i].xbuf)) {
+   while (i--)
+   testmgr_free_buf(data[i].xbuf);
+   goto out_free_tfm;
+   }
+
+
+   for (i = 0; i < num_mb; ++i) {
+   data[i].req = skcipher_request_alloc(tfm, GFP_KERNEL);
+   if (!data[i].req) {
+   pr_err("alg: skcipher: Failed to allocate request for 
%s\n",
+  algo);
+   while (i--)
+  

[PATCH 0/6] crypto: tcrypt: fix and add multi buf speed tests

[Gilad Ben-Yossef]

The performance of some crypto tfm providers is affected by
the amount of parallelism possible with the processing.

We already had some support for speed test of multiple concurrent
requests, dubbed multi buffer, in ahash speed tests.

This patch set extends said support and add similar support for skcipher
and AEAD, as well as fixes some odd bugs discovered along the way.

It is noted that it is possible to consolidate some of the none multi buffer
speed test code better, but given that tcrypt as a whole is a developer testing
harness rather than production code, the value of this activity seems
questionable. Do let me know if you disagree.

The patch set is built on top of Robert Baronescu's patch titled
"crypto: tcrypt - fix S/G table for test_aead_speed()" which was posted on
the list, as without it the AEAD tests fails.

Gilad Ben-Yossef (6):
  crypto: tcrypt: use multi buf for ahash mb test
  crypto: tcrypt: fix AEAD decryption speed test
  crypto: tcrypt: allow setting num of bufs
  crypto: tcrypt: add multi buf ahash jiffies test
  crypto: tcrypt: add multibuf skcipher speed test
  crypto: tcrypt: add multibuf aead speed test

 crypto/tcrypt.c | 1074 +--
 1 file changed, 973 insertions(+), 101 deletions(-)

-- 
2.7.4

[PATCH 6/6] crypto: tcrypt: add multibuf aead speed test

[Gilad Ben-Yossef]

The performance of some aead tfm providers is affected by
the amount of parallelism possible with the processing.

Introduce an async aead concurrent multiple buffer
processing speed test to be able to test performance of such
tfm providers.

Signed-off-by: Gilad Ben-Yossef 
---
 crypto/tcrypt.c | 432 
 1 file changed, 376 insertions(+), 56 deletions(-)

diff --git a/crypto/tcrypt.c b/crypto/tcrypt.c
index f1417e3..a05648e 100644
--- a/crypto/tcrypt.c
+++ b/crypto/tcrypt.c
@@ -80,6 +80,62 @@ static char *check[] = {
NULL
 };
 
+static u32 block_sizes[] = { 16, 64, 256, 1024, 8192, 0 };
+static u32 aead_sizes[] = { 16, 64, 256, 512, 1024, 2048, 4096, 8192, 0 };
+
+#define XBUFSIZE 8
+#define MAX_IVLEN 32
+
+static int testmgr_alloc_buf(char *buf[XBUFSIZE])
+{
+   int i;
+
+   for (i = 0; i < XBUFSIZE; i++) {
+   buf[i] = (void *)__get_free_page(GFP_KERNEL);
+   if (!buf[i])
+   goto err_free_buf;
+   }
+
+   return 0;
+
+err_free_buf:
+   while (i-- > 0)
+   free_page((unsigned long)buf[i]);
+
+   return -ENOMEM;
+}
+
+static void testmgr_free_buf(char *buf[XBUFSIZE])
+{
+   int i;
+
+   for (i = 0; i < XBUFSIZE; i++)
+   free_page((unsigned long)buf[i]);
+}
+
+static void sg_init_aead(struct scatterlist *sg, char *xbuf[XBUFSIZE],
+   unsigned int buflen)
+{
+   int np = (buflen + PAGE_SIZE - 1)/PAGE_SIZE;
+   int k, rem;
+
+   if (np > XBUFSIZE) {
+   rem = PAGE_SIZE;
+   np = XBUFSIZE;
+   } else {
+   rem = buflen % PAGE_SIZE;
+   }
+
+   sg_init_table(sg, np + 1);
+   if (rem)
+   np--;
+   for (k = 0; k < np; k++)
+   sg_set_buf([k + 1], xbuf[k], PAGE_SIZE);
+
+   if (rem)
+   sg_set_buf([k + 1], xbuf[k], rem);
+}
+
 static inline int do_one_aead_op(struct aead_request *req, int ret)
 {
struct crypto_wait *wait = req->base.data;
@@ -87,6 +143,299 @@ static inline int do_one_aead_op(struct aead_request *req, 
int ret)
return crypto_wait_req(ret, wait);
 }
 
+struct test_mb_aead_data {
+   struct scatterlist sg[XBUFSIZE];
+   struct scatterlist sgout[XBUFSIZE];
+   struct aead_request *req;
+   struct crypto_wait wait;
+   char *xbuf[XBUFSIZE];
+   char *xoutbuf[XBUFSIZE];
+   char *axbuf[XBUFSIZE];
+};
+
+static int do_mult_aead_op(struct test_mb_aead_data *data, int enc,
+   u32 num_mb)
+{
+   int i, rc[num_mb], err = 0;
+
+   /* Fire up a bunch of concurrent requests */
+   for (i = 0; i < num_mb; i++) {
+   if (enc == ENCRYPT)
+   rc[i] = crypto_aead_encrypt(data[i].req);
+   else
+   rc[i] = crypto_aead_decrypt(data[i].req);
+   }
+
+   /* Wait for all requests to finish */
+   for (i = 0; i < num_mb; i++) {
+   rc[i] = crypto_wait_req(rc[i], [i].wait);
+
+   if (rc[i]) {
+   pr_info("concurrent request %d error %d\n", i, rc[i]);
+   err = rc[i];
+   }
+   }
+
+   return err;
+}
+
+static int test_mb_aead_jiffies(struct test_mb_aead_data *data, int enc,
+   int blen, int secs, u32 num_mb)
+{
+   unsigned long start, end;
+   int bcount;
+   int ret;
+
+   for (start = jiffies, end = start + secs * HZ, bcount = 0;
+time_before(jiffies, end); bcount++) {
+   ret = do_mult_aead_op(data, enc, num_mb);
+   if (ret)
+   return ret;
+   }
+
+   pr_cont("%d operations in %d seconds (%ld bytes)\n",
+   bcount * num_mb, secs, (long)bcount * blen * num_mb);
+   return 0;
+}
+
+static int test_mb_aead_cycles(struct test_mb_aead_data *data, int enc,
+  int blen, u32 num_mb)
+{
+   unsigned long cycles = 0;
+   int ret = 0;
+   int i;
+
+   /* Warm-up run. */
+   for (i = 0; i < 4; i++) {
+   ret = do_mult_aead_op(data, enc, num_mb);
+   if (ret)
+   goto out;
+   }
+
+   /* The real thing. */
+   for (i = 0; i < 8; i++) {
+   cycles_t start, end;
+
+   start = get_cycles();
+   ret = do_mult_aead_op(data, enc, num_mb);
+   end = get_cycles();
+
+   if (ret)
+   goto out;
+
+   cycles += end - start;
+   }
+
+out:
+   if (ret == 0)
+   pr_cont("1 operation in %lu cycles (%d bytes)\n",
+   (cycles + 4) / (8 * num_mb), blen);
+
+   return ret;
+}
+
+static void test_mb_aead_speed(const char *algo, int enc, int secs,
+  struct aead_speed_template *template,
+   

Re: [PATCH v2 3/4] crypto: inside-secure - only update the result buffer when provided

[Antoine Tenart]

Hi Kamil,

On Thu, Nov 30, 2017 at 10:19:26AM +0100, Kamil Konieczny wrote:
> On 28.11.2017 16:42, Antoine Tenart wrote:
> > The patch fixes the ahash support by only updating the result buffer
> > when provided. Otherwise the driver could crash with NULL pointer
> > exceptions, because the ahash caller isn't required to supply a result
> > buffer on all calls.
> 
> Can you point to bug crush report ?

Do you want the crash dump? (It'll only be a "normal" NULL pointer
dereference).

> > Fixes: 1b44c5a60c13 ("crypto: inside-secure - add SafeXcel EIP197 crypto 
> > engine driver")
> > Signed-off-by: Antoine Tenart 
> > ---
> >  drivers/crypto/inside-secure/safexcel_hash.c | 7 ++-
> >  1 file changed, 6 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/crypto/inside-secure/safexcel_hash.c 
> > b/drivers/crypto/inside-secure/safexcel_hash.c
> > index 6135c9f5742c..424f4c5d4d25 100644
> > --- a/drivers/crypto/inside-secure/safexcel_hash.c
> > +++ b/drivers/crypto/inside-secure/safexcel_hash.c
> > @@ -150,7 +150,12 @@ static int safexcel_handle_req_result(struct 
> > safexcel_crypto_priv *priv, int rin
> >  
> > if (sreq->finish)
> > result_sz = crypto_ahash_digestsize(ahash);
> > -   memcpy(sreq->state, areq->result, result_sz);
> > +
> > +   /* The called isn't required to supply a result buffer. Updated it only
> > +* when provided.
> > +*/
> > +   if (areq->result)
> > +   memcpy(sreq->state, areq->result, result_sz);
> >  
> > dma_unmap_sg(priv->dev, areq->src,
> >  sg_nents_for_len(areq->src, areq->nbytes), DMA_TO_DEVICE);
> > 
> 
> can the driver get request for final/finup/digest with null req->result ?

I don't think that can happen. But having an update called without
req->result provided is a valid call (though it's not well documented).

Thanks,
Antoine

-- 
Antoine Ténart, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

Re: [PATCH v2 3/4] crypto: inside-secure - only update the result buffer when provided

[Kamil Konieczny]

Hi Antoine,

On 28.11.2017 16:42, Antoine Tenart wrote:
> The patch fixes the ahash support by only updating the result buffer
> when provided. Otherwise the driver could crash with NULL pointer
> exceptions, because the ahash caller isn't required to supply a result
> buffer on all calls.

Can you point to bug crush report ?

> Fixes: 1b44c5a60c13 ("crypto: inside-secure - add SafeXcel EIP197 crypto 
> engine driver")
> Signed-off-by: Antoine Tenart 
> ---
>  drivers/crypto/inside-secure/safexcel_hash.c | 7 ++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/crypto/inside-secure/safexcel_hash.c 
> b/drivers/crypto/inside-secure/safexcel_hash.c
> index 6135c9f5742c..424f4c5d4d25 100644
> --- a/drivers/crypto/inside-secure/safexcel_hash.c
> +++ b/drivers/crypto/inside-secure/safexcel_hash.c
> @@ -150,7 +150,12 @@ static int safexcel_handle_req_result(struct 
> safexcel_crypto_priv *priv, int rin
>  
>   if (sreq->finish)
>   result_sz = crypto_ahash_digestsize(ahash);
> - memcpy(sreq->state, areq->result, result_sz);
> +
> + /* The called isn't required to supply a result buffer. Updated it only
> +  * when provided.
> +  */
> + if (areq->result)
> + memcpy(sreq->state, areq->result, result_sz);
>  
>   dma_unmap_sg(priv->dev, areq->src,
>sg_nents_for_len(areq->src, areq->nbytes), DMA_TO_DEVICE);
> 

can the driver get request for final/finup/digest with null req->result ?
If yes (?), such checks can be done before any hardware processing, saving time,
for example:

int hash_final(struct ahash_request *areq)
{
if (!areq->result)
return -EINVAL;
/* normal processing here */
}


-- 
Best regards,
Kamil Konieczny
Samsung R Institute Poland

Re: WARNING: kernel stack regs has bad 'bp' value (2)

[Eric Biggers]

On Tue, Nov 28, 2017 at 10:36:01AM -0800, syzbot wrote:
> WARNING: kernel stack regs at 8801c1e5f468 in syzkaller196611:6199 has
> bad 'bp' value 0001
> unwind stack type:0 next_sp:  (null) mask:0x6 graph_idx:0
> 8801db4075a8: 8801db407630 (0x8801db407630)
> 8801db4075b0: 8128a84e (__save_stack_trace+0x6e/0xd0)
> 8801db4075b8:  ...
> 8801db4075c0: 8801c1e58000 (0x8801c1e58000)
> 8801db4075c8: 8801c1e6 (0x8801c1e6)
> 8801db4075d0:  ...
> 8801db4075d8: 0006 (0x6)
> 8801db4075e0: 8801c1e4e000 (0x8801c1e4e000)
> 8801db4075e8: 0101 (0x101)
> 8801db4075f0:  ...
> 8801db4075f8: 8801db4075a8 (0x8801db4075a8)
> 8801db407600: 8134ff7d (__twofish_enc_blk_3way+0x1b1d/0x1b30)

Looks like the x86_64 "3 way" version of Twofish (twofish-x86_64-asm_64-3way.S)
needs to be updated to not use %rbp.

Eric

Re: KASAN: slab-out-of-bounds Read in crypto_chacha20_crypt

[Eric Biggers]

On Thu, Nov 30, 2017 at 12:37:01AM -0800, syzbot wrote:
> ==
> BUG: KASAN: slab-out-of-bounds in __le32_to_cpup
> include/uapi/linux/byteorder/little_endian.h:58 [inline]
> BUG: KASAN: slab-out-of-bounds in le32_to_cpuvp crypto/chacha20_generic.c:19
> [inline]
> BUG: KASAN: slab-out-of-bounds in crypto_chacha20_init
> crypto/chacha20_generic.c:58 [inline]
> BUG: KASAN: slab-out-of-bounds in crypto_chacha20_crypt+0xaf1/0xbd0
> crypto/chacha20_generic.c:91
> Read of size 4 at addr 88010006 by task syz-executor1/15417

#syz dup: general protection fault in crypto_chacha20_crypt

Re: KASAN: use-after-free Read in blkcipher_walk_virt

[Eric Biggers]

On Thu, Nov 30, 2017 at 12:37:01AM -0800, syzbot wrote:
> ==
> BUG: KASAN: use-after-free in crypto_tfm_alg_blocksize
> include/linux/crypto.h:671 [inline]
> BUG: KASAN: use-after-free in crypto_blkcipher_blocksize
> include/linux/crypto.h:1214 [inline]
> BUG: KASAN: use-after-free in blkcipher_walk_virt+0x286/0x2a0
> crypto/blkcipher.c:304
> Read of size 8 at addr 8801ccba7f38 by task syz-executor5/4473
> 

#syz dup: KASAN: use-after-free Read in aead_recvmsg