linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] crypto: poly1305: fix poly1305_core_setkey() declaration
@ 2021-03-22 17:05 Arnd Bergmann
  2021-03-22 18:51 ` Ard Biesheuvel
  2021-04-02  9:47 ` Herbert Xu
  0 siblings, 2 replies; 4+ messages in thread
From: Arnd Bergmann @ 2021-03-22 17:05 UTC (permalink / raw)
  To: Herbert Xu, David S. Miller, Jason A. Donenfeld
  Cc: Arnd Bergmann, Russell King, Catalin Marinas, Will Deacon,
	Thomas Bogendoerfer, Thomas Gleixner, Ingo Molnar,
	Borislav Petkov, x86, H. Peter Anvin, Ard Biesheuvel,
	Arvind Sankar, Masahiro Yamada, Eric Biggers, linux-crypto,
	linux-arm-kernel, linux-kernel, linux-mips

From: Arnd Bergmann <arnd@arndb.de>

gcc-11 points out a mismatch between the declaration and the definition
of poly1305_core_setkey():

lib/crypto/poly1305-donna32.c:13:67: error: argument 2 of type ‘const u8[16]’ {aka ‘const unsigned char[16]’} with mismatched bound [-Werror=array-parameter=]
   13 | void poly1305_core_setkey(struct poly1305_core_key *key, const u8 raw_key[16])
      |                                                          ~~~~~~~~~^~~~~~~~~~~
In file included from lib/crypto/poly1305-donna32.c:11:
include/crypto/internal/poly1305.h:21:68: note: previously declared as ‘const u8 *’ {aka ‘const unsigned char *’}
   21 | void poly1305_core_setkey(struct poly1305_core_key *key, const u8 *raw_key);

This is harmless in principle, as the calling conventions are the same,
but the more specific prototype allows better type checking in the
caller.

Change the declaration to match the actual function definition.
The poly1305_simd_init() is a bit suspicious here, as it previously
had a 32-byte argument type, but looks like it needs to take the
16-byte POLY1305_BLOCK_SIZE array instead.

Fixes: 1c08a104360f ("crypto: poly1305 - add new 32 and 64-bit generic versions")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 arch/arm/crypto/poly1305-glue.c    | 2 +-
 arch/arm64/crypto/poly1305-glue.c  | 2 +-
 arch/mips/crypto/poly1305-glue.c   | 2 +-
 arch/x86/crypto/poly1305_glue.c    | 6 +++---
 include/crypto/internal/poly1305.h | 3 ++-
 include/crypto/poly1305.h          | 6 ++++--
 lib/crypto/poly1305-donna32.c      | 3 ++-
 lib/crypto/poly1305-donna64.c      | 3 ++-
 lib/crypto/poly1305.c              | 3 ++-
 9 files changed, 18 insertions(+), 12 deletions(-)

diff --git a/arch/arm/crypto/poly1305-glue.c b/arch/arm/crypto/poly1305-glue.c
index 3023c1acfa19..c31bd8f7c092 100644
--- a/arch/arm/crypto/poly1305-glue.c
+++ b/arch/arm/crypto/poly1305-glue.c
@@ -29,7 +29,7 @@ void __weak poly1305_blocks_neon(void *state, const u8 *src, u32 len, u32 hibit)
 
 static __ro_after_init DEFINE_STATIC_KEY_FALSE(have_neon);
 
-void poly1305_init_arch(struct poly1305_desc_ctx *dctx, const u8 *key)
+void poly1305_init_arch(struct poly1305_desc_ctx *dctx, const u8 key[POLY1305_KEY_SIZE])
 {
 	poly1305_init_arm(&dctx->h, key);
 	dctx->s[0] = get_unaligned_le32(key + 16);
diff --git a/arch/arm64/crypto/poly1305-glue.c b/arch/arm64/crypto/poly1305-glue.c
index 683de671741a..9c3d86e397bf 100644
--- a/arch/arm64/crypto/poly1305-glue.c
+++ b/arch/arm64/crypto/poly1305-glue.c
@@ -25,7 +25,7 @@ asmlinkage void poly1305_emit(void *state, u8 *digest, const u32 *nonce);
 
 static __ro_after_init DEFINE_STATIC_KEY_FALSE(have_neon);
 
-void poly1305_init_arch(struct poly1305_desc_ctx *dctx, const u8 *key)
+void poly1305_init_arch(struct poly1305_desc_ctx *dctx, const u8 key[POLY1305_KEY_SIZE])
 {
 	poly1305_init_arm64(&dctx->h, key);
 	dctx->s[0] = get_unaligned_le32(key + 16);
diff --git a/arch/mips/crypto/poly1305-glue.c b/arch/mips/crypto/poly1305-glue.c
index fc881b46d911..bc6110fb98e0 100644
--- a/arch/mips/crypto/poly1305-glue.c
+++ b/arch/mips/crypto/poly1305-glue.c
@@ -17,7 +17,7 @@ asmlinkage void poly1305_init_mips(void *state, const u8 *key);
 asmlinkage void poly1305_blocks_mips(void *state, const u8 *src, u32 len, u32 hibit);
 asmlinkage void poly1305_emit_mips(void *state, u8 *digest, const u32 *nonce);
 
-void poly1305_init_arch(struct poly1305_desc_ctx *dctx, const u8 *key)
+void poly1305_init_arch(struct poly1305_desc_ctx *dctx, const u8 key[POLY1305_KEY_SIZE])
 {
 	poly1305_init_mips(&dctx->h, key);
 	dctx->s[0] = get_unaligned_le32(key + 16);
diff --git a/arch/x86/crypto/poly1305_glue.c b/arch/x86/crypto/poly1305_glue.c
index 646da46e8d10..1dfb8af48a3c 100644
--- a/arch/x86/crypto/poly1305_glue.c
+++ b/arch/x86/crypto/poly1305_glue.c
@@ -16,7 +16,7 @@
 #include <asm/simd.h>
 
 asmlinkage void poly1305_init_x86_64(void *ctx,
-				     const u8 key[POLY1305_KEY_SIZE]);
+				     const u8 key[POLY1305_BLOCK_SIZE]);
 asmlinkage void poly1305_blocks_x86_64(void *ctx, const u8 *inp,
 				       const size_t len, const u32 padbit);
 asmlinkage void poly1305_emit_x86_64(void *ctx, u8 mac[POLY1305_DIGEST_SIZE],
@@ -81,7 +81,7 @@ static void convert_to_base2_64(void *ctx)
 	state->is_base2_26 = 0;
 }
 
-static void poly1305_simd_init(void *ctx, const u8 key[POLY1305_KEY_SIZE])
+static void poly1305_simd_init(void *ctx, const u8 key[POLY1305_BLOCK_SIZE])
 {
 	poly1305_init_x86_64(ctx, key);
 }
@@ -129,7 +129,7 @@ static void poly1305_simd_emit(void *ctx, u8 mac[POLY1305_DIGEST_SIZE],
 		poly1305_emit_avx(ctx, mac, nonce);
 }
 
-void poly1305_init_arch(struct poly1305_desc_ctx *dctx, const u8 *key)
+void poly1305_init_arch(struct poly1305_desc_ctx *dctx, const u8 key[POLY1305_KEY_SIZE])
 {
 	poly1305_simd_init(&dctx->h, key);
 	dctx->s[0] = get_unaligned_le32(&key[16]);
diff --git a/include/crypto/internal/poly1305.h b/include/crypto/internal/poly1305.h
index 064e52ca5248..196aa769f296 100644
--- a/include/crypto/internal/poly1305.h
+++ b/include/crypto/internal/poly1305.h
@@ -18,7 +18,8 @@
  * only the ε-almost-∆-universal hash function (not the full MAC) is computed.
  */
 
-void poly1305_core_setkey(struct poly1305_core_key *key, const u8 *raw_key);
+void poly1305_core_setkey(struct poly1305_core_key *key,
+			  const u8 raw_key[POLY1305_BLOCK_SIZE]);
 static inline void poly1305_core_init(struct poly1305_state *state)
 {
 	*state = (struct poly1305_state){};
diff --git a/include/crypto/poly1305.h b/include/crypto/poly1305.h
index f1f67fc749cf..090692ec3bc7 100644
--- a/include/crypto/poly1305.h
+++ b/include/crypto/poly1305.h
@@ -58,8 +58,10 @@ struct poly1305_desc_ctx {
 	};
 };
 
-void poly1305_init_arch(struct poly1305_desc_ctx *desc, const u8 *key);
-void poly1305_init_generic(struct poly1305_desc_ctx *desc, const u8 *key);
+void poly1305_init_arch(struct poly1305_desc_ctx *desc,
+			const u8 key[POLY1305_KEY_SIZE]);
+void poly1305_init_generic(struct poly1305_desc_ctx *desc,
+			   const u8 key[POLY1305_KEY_SIZE]);
 
 static inline void poly1305_init(struct poly1305_desc_ctx *desc, const u8 *key)
 {
diff --git a/lib/crypto/poly1305-donna32.c b/lib/crypto/poly1305-donna32.c
index 3cc77d94390b..7fb71845cc84 100644
--- a/lib/crypto/poly1305-donna32.c
+++ b/lib/crypto/poly1305-donna32.c
@@ -10,7 +10,8 @@
 #include <asm/unaligned.h>
 #include <crypto/internal/poly1305.h>
 
-void poly1305_core_setkey(struct poly1305_core_key *key, const u8 raw_key[16])
+void poly1305_core_setkey(struct poly1305_core_key *key,
+			  const u8 raw_key[POLY1305_BLOCK_SIZE])
 {
 	/* r &= 0xffffffc0ffffffc0ffffffc0fffffff */
 	key->key.r[0] = (get_unaligned_le32(&raw_key[0])) & 0x3ffffff;
diff --git a/lib/crypto/poly1305-donna64.c b/lib/crypto/poly1305-donna64.c
index 6ae181bb4345..d34cf4053668 100644
--- a/lib/crypto/poly1305-donna64.c
+++ b/lib/crypto/poly1305-donna64.c
@@ -12,7 +12,8 @@
 
 typedef __uint128_t u128;
 
-void poly1305_core_setkey(struct poly1305_core_key *key, const u8 raw_key[16])
+void poly1305_core_setkey(struct poly1305_core_key *key,
+			  const u8 raw_key[POLY1305_BLOCK_SIZE])
 {
 	u64 t0, t1;
 
diff --git a/lib/crypto/poly1305.c b/lib/crypto/poly1305.c
index 9d2d14df0fee..26d87fc3823e 100644
--- a/lib/crypto/poly1305.c
+++ b/lib/crypto/poly1305.c
@@ -12,7 +12,8 @@
 #include <linux/module.h>
 #include <asm/unaligned.h>
 
-void poly1305_init_generic(struct poly1305_desc_ctx *desc, const u8 *key)
+void poly1305_init_generic(struct poly1305_desc_ctx *desc,
+			   const u8 key[POLY1305_KEY_SIZE])
 {
 	poly1305_core_setkey(&desc->core_r, key);
 	desc->s[0] = get_unaligned_le32(key + 16);
-- 
2.29.2


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH] crypto: poly1305: fix poly1305_core_setkey() declaration
  2021-03-22 17:05 [PATCH] crypto: poly1305: fix poly1305_core_setkey() declaration Arnd Bergmann
@ 2021-03-22 18:51 ` Ard Biesheuvel
  2021-03-23  0:51   ` Eric Biggers
  2021-04-02  9:47 ` Herbert Xu
  1 sibling, 1 reply; 4+ messages in thread
From: Ard Biesheuvel @ 2021-03-22 18:51 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Herbert Xu, David S. Miller, Jason A. Donenfeld, Arnd Bergmann,
	Russell King, Catalin Marinas, Will Deacon, Thomas Bogendoerfer,
	Thomas Gleixner, Ingo Molnar, Borislav Petkov, X86 ML,
	H. Peter Anvin, Arvind Sankar, Masahiro Yamada, Eric Biggers,
	Linux Crypto Mailing List, Linux ARM, Linux Kernel Mailing List,
	open list:MIPS

On Mon, 22 Mar 2021 at 18:05, Arnd Bergmann <arnd@kernel.org> wrote:
>
> From: Arnd Bergmann <arnd@arndb.de>
>
> gcc-11 points out a mismatch between the declaration and the definition
> of poly1305_core_setkey():
>
> lib/crypto/poly1305-donna32.c:13:67: error: argument 2 of type ‘const u8[16]’ {aka ‘const unsigned char[16]’} with mismatched bound [-Werror=array-parameter=]
>    13 | void poly1305_core_setkey(struct poly1305_core_key *key, const u8 raw_key[16])
>       |                                                          ~~~~~~~~~^~~~~~~~~~~
> In file included from lib/crypto/poly1305-donna32.c:11:
> include/crypto/internal/poly1305.h:21:68: note: previously declared as ‘const u8 *’ {aka ‘const unsigned char *’}
>    21 | void poly1305_core_setkey(struct poly1305_core_key *key, const u8 *raw_key);
>
> This is harmless in principle, as the calling conventions are the same,
> but the more specific prototype allows better type checking in the
> caller.
>
> Change the declaration to match the actual function definition.
> The poly1305_simd_init() is a bit suspicious here, as it previously
> had a 32-byte argument type, but looks like it needs to take the
> 16-byte POLY1305_BLOCK_SIZE array instead.
>

This looks ok to me. For historical reasons, the Poly1305 integration
is based on an unkeyed shash, and both the Poly1305 key and nonce are
passed as ordinary input, prepended to the actual data.
poly1305_simd_init() takes only the key but not the nonce, so it
should only be passed 16 bytes.

> Fixes: 1c08a104360f ("crypto: poly1305 - add new 32 and 64-bit generic versions")
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>

Reviewed-by: Ard Biesheuvel <ardb@kernel.org>

> ---
>  arch/arm/crypto/poly1305-glue.c    | 2 +-
>  arch/arm64/crypto/poly1305-glue.c  | 2 +-
>  arch/mips/crypto/poly1305-glue.c   | 2 +-
>  arch/x86/crypto/poly1305_glue.c    | 6 +++---
>  include/crypto/internal/poly1305.h | 3 ++-
>  include/crypto/poly1305.h          | 6 ++++--
>  lib/crypto/poly1305-donna32.c      | 3 ++-
>  lib/crypto/poly1305-donna64.c      | 3 ++-
>  lib/crypto/poly1305.c              | 3 ++-
>  9 files changed, 18 insertions(+), 12 deletions(-)
>
> diff --git a/arch/arm/crypto/poly1305-glue.c b/arch/arm/crypto/poly1305-glue.c
> index 3023c1acfa19..c31bd8f7c092 100644
> --- a/arch/arm/crypto/poly1305-glue.c
> +++ b/arch/arm/crypto/poly1305-glue.c
> @@ -29,7 +29,7 @@ void __weak poly1305_blocks_neon(void *state, const u8 *src, u32 len, u32 hibit)
>
>  static __ro_after_init DEFINE_STATIC_KEY_FALSE(have_neon);
>
> -void poly1305_init_arch(struct poly1305_desc_ctx *dctx, const u8 *key)
> +void poly1305_init_arch(struct poly1305_desc_ctx *dctx, const u8 key[POLY1305_KEY_SIZE])
>  {
>         poly1305_init_arm(&dctx->h, key);
>         dctx->s[0] = get_unaligned_le32(key + 16);
> diff --git a/arch/arm64/crypto/poly1305-glue.c b/arch/arm64/crypto/poly1305-glue.c
> index 683de671741a..9c3d86e397bf 100644
> --- a/arch/arm64/crypto/poly1305-glue.c
> +++ b/arch/arm64/crypto/poly1305-glue.c
> @@ -25,7 +25,7 @@ asmlinkage void poly1305_emit(void *state, u8 *digest, const u32 *nonce);
>
>  static __ro_after_init DEFINE_STATIC_KEY_FALSE(have_neon);
>
> -void poly1305_init_arch(struct poly1305_desc_ctx *dctx, const u8 *key)
> +void poly1305_init_arch(struct poly1305_desc_ctx *dctx, const u8 key[POLY1305_KEY_SIZE])
>  {
>         poly1305_init_arm64(&dctx->h, key);
>         dctx->s[0] = get_unaligned_le32(key + 16);
> diff --git a/arch/mips/crypto/poly1305-glue.c b/arch/mips/crypto/poly1305-glue.c
> index fc881b46d911..bc6110fb98e0 100644
> --- a/arch/mips/crypto/poly1305-glue.c
> +++ b/arch/mips/crypto/poly1305-glue.c
> @@ -17,7 +17,7 @@ asmlinkage void poly1305_init_mips(void *state, const u8 *key);
>  asmlinkage void poly1305_blocks_mips(void *state, const u8 *src, u32 len, u32 hibit);
>  asmlinkage void poly1305_emit_mips(void *state, u8 *digest, const u32 *nonce);
>
> -void poly1305_init_arch(struct poly1305_desc_ctx *dctx, const u8 *key)
> +void poly1305_init_arch(struct poly1305_desc_ctx *dctx, const u8 key[POLY1305_KEY_SIZE])
>  {
>         poly1305_init_mips(&dctx->h, key);
>         dctx->s[0] = get_unaligned_le32(key + 16);
> diff --git a/arch/x86/crypto/poly1305_glue.c b/arch/x86/crypto/poly1305_glue.c
> index 646da46e8d10..1dfb8af48a3c 100644
> --- a/arch/x86/crypto/poly1305_glue.c
> +++ b/arch/x86/crypto/poly1305_glue.c
> @@ -16,7 +16,7 @@
>  #include <asm/simd.h>
>
>  asmlinkage void poly1305_init_x86_64(void *ctx,
> -                                    const u8 key[POLY1305_KEY_SIZE]);
> +                                    const u8 key[POLY1305_BLOCK_SIZE]);
>  asmlinkage void poly1305_blocks_x86_64(void *ctx, const u8 *inp,
>                                        const size_t len, const u32 padbit);
>  asmlinkage void poly1305_emit_x86_64(void *ctx, u8 mac[POLY1305_DIGEST_SIZE],
> @@ -81,7 +81,7 @@ static void convert_to_base2_64(void *ctx)
>         state->is_base2_26 = 0;
>  }
>
> -static void poly1305_simd_init(void *ctx, const u8 key[POLY1305_KEY_SIZE])
> +static void poly1305_simd_init(void *ctx, const u8 key[POLY1305_BLOCK_SIZE])
>  {
>         poly1305_init_x86_64(ctx, key);
>  }
> @@ -129,7 +129,7 @@ static void poly1305_simd_emit(void *ctx, u8 mac[POLY1305_DIGEST_SIZE],
>                 poly1305_emit_avx(ctx, mac, nonce);
>  }
>
> -void poly1305_init_arch(struct poly1305_desc_ctx *dctx, const u8 *key)
> +void poly1305_init_arch(struct poly1305_desc_ctx *dctx, const u8 key[POLY1305_KEY_SIZE])
>  {
>         poly1305_simd_init(&dctx->h, key);
>         dctx->s[0] = get_unaligned_le32(&key[16]);
> diff --git a/include/crypto/internal/poly1305.h b/include/crypto/internal/poly1305.h
> index 064e52ca5248..196aa769f296 100644
> --- a/include/crypto/internal/poly1305.h
> +++ b/include/crypto/internal/poly1305.h
> @@ -18,7 +18,8 @@
>   * only the ε-almost-∆-universal hash function (not the full MAC) is computed.
>   */
>
> -void poly1305_core_setkey(struct poly1305_core_key *key, const u8 *raw_key);
> +void poly1305_core_setkey(struct poly1305_core_key *key,
> +                         const u8 raw_key[POLY1305_BLOCK_SIZE]);
>  static inline void poly1305_core_init(struct poly1305_state *state)
>  {
>         *state = (struct poly1305_state){};
> diff --git a/include/crypto/poly1305.h b/include/crypto/poly1305.h
> index f1f67fc749cf..090692ec3bc7 100644
> --- a/include/crypto/poly1305.h
> +++ b/include/crypto/poly1305.h
> @@ -58,8 +58,10 @@ struct poly1305_desc_ctx {
>         };
>  };
>
> -void poly1305_init_arch(struct poly1305_desc_ctx *desc, const u8 *key);
> -void poly1305_init_generic(struct poly1305_desc_ctx *desc, const u8 *key);
> +void poly1305_init_arch(struct poly1305_desc_ctx *desc,
> +                       const u8 key[POLY1305_KEY_SIZE]);
> +void poly1305_init_generic(struct poly1305_desc_ctx *desc,
> +                          const u8 key[POLY1305_KEY_SIZE]);
>
>  static inline void poly1305_init(struct poly1305_desc_ctx *desc, const u8 *key)
>  {
> diff --git a/lib/crypto/poly1305-donna32.c b/lib/crypto/poly1305-donna32.c
> index 3cc77d94390b..7fb71845cc84 100644
> --- a/lib/crypto/poly1305-donna32.c
> +++ b/lib/crypto/poly1305-donna32.c
> @@ -10,7 +10,8 @@
>  #include <asm/unaligned.h>
>  #include <crypto/internal/poly1305.h>
>
> -void poly1305_core_setkey(struct poly1305_core_key *key, const u8 raw_key[16])
> +void poly1305_core_setkey(struct poly1305_core_key *key,
> +                         const u8 raw_key[POLY1305_BLOCK_SIZE])
>  {
>         /* r &= 0xffffffc0ffffffc0ffffffc0fffffff */
>         key->key.r[0] = (get_unaligned_le32(&raw_key[0])) & 0x3ffffff;
> diff --git a/lib/crypto/poly1305-donna64.c b/lib/crypto/poly1305-donna64.c
> index 6ae181bb4345..d34cf4053668 100644
> --- a/lib/crypto/poly1305-donna64.c
> +++ b/lib/crypto/poly1305-donna64.c
> @@ -12,7 +12,8 @@
>
>  typedef __uint128_t u128;
>
> -void poly1305_core_setkey(struct poly1305_core_key *key, const u8 raw_key[16])
> +void poly1305_core_setkey(struct poly1305_core_key *key,
> +                         const u8 raw_key[POLY1305_BLOCK_SIZE])
>  {
>         u64 t0, t1;
>
> diff --git a/lib/crypto/poly1305.c b/lib/crypto/poly1305.c
> index 9d2d14df0fee..26d87fc3823e 100644
> --- a/lib/crypto/poly1305.c
> +++ b/lib/crypto/poly1305.c
> @@ -12,7 +12,8 @@
>  #include <linux/module.h>
>  #include <asm/unaligned.h>
>
> -void poly1305_init_generic(struct poly1305_desc_ctx *desc, const u8 *key)
> +void poly1305_init_generic(struct poly1305_desc_ctx *desc,
> +                          const u8 key[POLY1305_KEY_SIZE])
>  {
>         poly1305_core_setkey(&desc->core_r, key);
>         desc->s[0] = get_unaligned_le32(key + 16);
> --
> 2.29.2
>

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] crypto: poly1305: fix poly1305_core_setkey() declaration
  2021-03-22 18:51 ` Ard Biesheuvel
@ 2021-03-23  0:51   ` Eric Biggers
  0 siblings, 0 replies; 4+ messages in thread
From: Eric Biggers @ 2021-03-23  0:51 UTC (permalink / raw)
  To: Ard Biesheuvel
  Cc: Arnd Bergmann, Herbert Xu, David S. Miller, Jason A. Donenfeld,
	Arnd Bergmann, Russell King, Catalin Marinas, Will Deacon,
	Thomas Bogendoerfer, Thomas Gleixner, Ingo Molnar,
	Borislav Petkov, X86 ML, H. Peter Anvin, Arvind Sankar,
	Masahiro Yamada, Linux Crypto Mailing List, Linux ARM,
	Linux Kernel Mailing List, open list:MIPS

On Mon, Mar 22, 2021 at 07:51:47PM +0100, Ard Biesheuvel wrote:
> On Mon, 22 Mar 2021 at 18:05, Arnd Bergmann <arnd@kernel.org> wrote:
> >
> > From: Arnd Bergmann <arnd@arndb.de>
> >
> > gcc-11 points out a mismatch between the declaration and the definition
> > of poly1305_core_setkey():
> >
> > lib/crypto/poly1305-donna32.c:13:67: error: argument 2 of type ‘const u8[16]’ {aka ‘const unsigned char[16]’} with mismatched bound [-Werror=array-parameter=]
> >    13 | void poly1305_core_setkey(struct poly1305_core_key *key, const u8 raw_key[16])
> >       |                                                          ~~~~~~~~~^~~~~~~~~~~
> > In file included from lib/crypto/poly1305-donna32.c:11:
> > include/crypto/internal/poly1305.h:21:68: note: previously declared as ‘const u8 *’ {aka ‘const unsigned char *’}
> >    21 | void poly1305_core_setkey(struct poly1305_core_key *key, const u8 *raw_key);
> >
> > This is harmless in principle, as the calling conventions are the same,
> > but the more specific prototype allows better type checking in the
> > caller.
> >
> > Change the declaration to match the actual function definition.
> > The poly1305_simd_init() is a bit suspicious here, as it previously
> > had a 32-byte argument type, but looks like it needs to take the
> > 16-byte POLY1305_BLOCK_SIZE array instead.
> >
> 
> This looks ok to me. For historical reasons, the Poly1305 integration
> is based on an unkeyed shash, and both the Poly1305 key and nonce are
> passed as ordinary input, prepended to the actual data.
> poly1305_simd_init() takes only the key but not the nonce, so it
> should only be passed 16 bytes.

Well to be more precise, there are two conventions for using Poly1305.  One
where it is invoked many times with the same 16-byte key and different 16-byte
nonces.  And one where every invocation uses a unique key *and* nonce,
interpreted as a 32-byte "one-time key".

So that's why there's a mix of 16 and 32 byte "keys".

The naming "POLY1305_KEY_SIZE" assumes the second convention, which is a bit
confusing; it really should be called something like POLY1305_ONETIME_KEY_SIZE.
I guess the idea was that the one-time key convention is the more common one.

Anyway, the patch seems to be fine, as it uses the correct length in each
location.  You can add:

Reviewed-by: Eric Biggers <ebiggers@google.com>

- Eric

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] crypto: poly1305: fix poly1305_core_setkey() declaration
  2021-03-22 17:05 [PATCH] crypto: poly1305: fix poly1305_core_setkey() declaration Arnd Bergmann
  2021-03-22 18:51 ` Ard Biesheuvel
@ 2021-04-02  9:47 ` Herbert Xu
  1 sibling, 0 replies; 4+ messages in thread
From: Herbert Xu @ 2021-04-02  9:47 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: David S. Miller, Jason A. Donenfeld, Arnd Bergmann, Russell King,
	Catalin Marinas, Will Deacon, Thomas Bogendoerfer,
	Thomas Gleixner, Ingo Molnar, Borislav Petkov, x86,
	H. Peter Anvin, Ard Biesheuvel, Arvind Sankar, Masahiro Yamada,
	Eric Biggers, linux-crypto, linux-arm-kernel, linux-kernel,
	linux-mips

On Mon, Mar 22, 2021 at 06:05:15PM +0100, Arnd Bergmann wrote:
> From: Arnd Bergmann <arnd@arndb.de>
> 
> gcc-11 points out a mismatch between the declaration and the definition
> of poly1305_core_setkey():
> 
> lib/crypto/poly1305-donna32.c:13:67: error: argument 2 of type ‘const u8[16]’ {aka ‘const unsigned char[16]’} with mismatched bound [-Werror=array-parameter=]
>    13 | void poly1305_core_setkey(struct poly1305_core_key *key, const u8 raw_key[16])
>       |                                                          ~~~~~~~~~^~~~~~~~~~~
> In file included from lib/crypto/poly1305-donna32.c:11:
> include/crypto/internal/poly1305.h:21:68: note: previously declared as ‘const u8 *’ {aka ‘const unsigned char *’}
>    21 | void poly1305_core_setkey(struct poly1305_core_key *key, const u8 *raw_key);
> 
> This is harmless in principle, as the calling conventions are the same,
> but the more specific prototype allows better type checking in the
> caller.
> 
> Change the declaration to match the actual function definition.
> The poly1305_simd_init() is a bit suspicious here, as it previously
> had a 32-byte argument type, but looks like it needs to take the
> 16-byte POLY1305_BLOCK_SIZE array instead.
> 
> Fixes: 1c08a104360f ("crypto: poly1305 - add new 32 and 64-bit generic versions")
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
>  arch/arm/crypto/poly1305-glue.c    | 2 +-
>  arch/arm64/crypto/poly1305-glue.c  | 2 +-
>  arch/mips/crypto/poly1305-glue.c   | 2 +-
>  arch/x86/crypto/poly1305_glue.c    | 6 +++---
>  include/crypto/internal/poly1305.h | 3 ++-
>  include/crypto/poly1305.h          | 6 ++++--
>  lib/crypto/poly1305-donna32.c      | 3 ++-
>  lib/crypto/poly1305-donna64.c      | 3 ++-
>  lib/crypto/poly1305.c              | 3 ++-
>  9 files changed, 18 insertions(+), 12 deletions(-)

Patch applied.  Thanks.
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2021-04-02  9:51 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-03-22 17:05 [PATCH] crypto: poly1305: fix poly1305_core_setkey() declaration Arnd Bergmann
2021-03-22 18:51 ` Ard Biesheuvel
2021-03-23  0:51   ` Eric Biggers
2021-04-02  9:47 ` Herbert Xu

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).