[PATCH] lib/crypto/poly1305: Fix arm64's poly1305_blocks_arch()

[PATCH] lib/crypto/poly1305: Fix arm64's poly1305_blocks_arch()

[Eric Biggers]

From: Eric Biggers <ebiggers@google.com>

For some reason arm64's Poly1305 code got changed to ignore the padbit
argument.  As a result, the output is incorrect when the message length
is not a multiple of 16 (which is not reached with the standard
ChaCha20Poly1305, but bcachefs could reach this).  Fix this.

Fixes: a59e5468a921 ("crypto: arm64/poly1305 - Add block-only interface")
Reported-by: Kent Overstreet <kent.overstreet@linux.dev>
Signed-off-by: Eric Biggers <ebiggers@google.com>
---
 arch/arm64/lib/crypto/poly1305-glue.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/lib/crypto/poly1305-glue.c b/arch/arm64/lib/crypto/poly1305-glue.c
index 6a661cf048213..c9a74766785bd 100644
--- a/arch/arm64/lib/crypto/poly1305-glue.c
+++ b/arch/arm64/lib/crypto/poly1305-glue.c
@@ -36,18 +36,18 @@ void poly1305_blocks_arch(struct poly1305_block_state *state, const u8 *src,
 	if (static_branch_likely(&have_neon)) {
 		do {
 			unsigned int todo = min_t(unsigned int, len, SZ_4K);
 
 			kernel_neon_begin();
-			poly1305_blocks_neon(state, src, todo, 1);
+			poly1305_blocks_neon(state, src, todo, padbit);
 			kernel_neon_end();
 
 			len -= todo;
 			src += todo;
 		} while (len);
 	} else
-		poly1305_blocks(state, src, len, 1);
+		poly1305_blocks(state, src, len, padbit);
 }
 EXPORT_SYMBOL_GPL(poly1305_blocks_arch);
 
 bool poly1305_is_arch_optimized(void)
 {

base-commit: 19272b37aa4f83ca52bdf9c16d5d81bdd1354494
-- 
2.49.0

Re: [PATCH] lib/crypto/poly1305: Fix arm64's poly1305_blocks_arch()

[Kent Overstreet]

On Sun, Jun 15, 2025 at 06:06:54PM -0700, Eric Biggers wrote:
> From: Eric Biggers <ebiggers@google.com>
> 
> For some reason arm64's Poly1305 code got changed to ignore the padbit
> argument.  As a result, the output is incorrect when the message length
> is not a multiple of 16 (which is not reached with the standard
> ChaCha20Poly1305, but bcachefs could reach this).  Fix this.
> 
> Fixes: a59e5468a921 ("crypto: arm64/poly1305 - Add block-only interface")
> Reported-by: Kent Overstreet <kent.overstreet@linux.dev>
> Signed-off-by: Eric Biggers <ebiggers@google.com>

Thanks for the quick fix :)

Tested-by: Kent Overstreet <kent.overstreet@linux.dev>

> ---
>  arch/arm64/lib/crypto/poly1305-glue.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/arm64/lib/crypto/poly1305-glue.c b/arch/arm64/lib/crypto/poly1305-glue.c
> index 6a661cf048213..c9a74766785bd 100644
> --- a/arch/arm64/lib/crypto/poly1305-glue.c
> +++ b/arch/arm64/lib/crypto/poly1305-glue.c
> @@ -36,18 +36,18 @@ void poly1305_blocks_arch(struct poly1305_block_state *state, const u8 *src,
>  	if (static_branch_likely(&have_neon)) {
>  		do {
>  			unsigned int todo = min_t(unsigned int, len, SZ_4K);
>  
>  			kernel_neon_begin();
> -			poly1305_blocks_neon(state, src, todo, 1);
> +			poly1305_blocks_neon(state, src, todo, padbit);
>  			kernel_neon_end();
>  
>  			len -= todo;
>  			src += todo;
>  		} while (len);
>  	} else
> -		poly1305_blocks(state, src, len, 1);
> +		poly1305_blocks(state, src, len, padbit);
>  }
>  EXPORT_SYMBOL_GPL(poly1305_blocks_arch);
>  
>  bool poly1305_is_arch_optimized(void)
>  {
> 
> base-commit: 19272b37aa4f83ca52bdf9c16d5d81bdd1354494
> -- 
> 2.49.0
> 

Re: [PATCH] lib/crypto/poly1305: Fix arm64's poly1305_blocks_arch()

[Herbert Xu]

Eric Biggers <ebiggers@kernel.org> wrote:
> From: Eric Biggers <ebiggers@google.com>
> 
> For some reason arm64's Poly1305 code got changed to ignore the padbit
> argument.  As a result, the output is incorrect when the message length
> is not a multiple of 16 (which is not reached with the standard
> ChaCha20Poly1305, but bcachefs could reach this).  Fix this.

Sorry, it was a cut-n-paste error since I copy the code from
the update function where the padbit is always 1.

> diff --git a/arch/arm64/lib/crypto/poly1305-glue.c b/arch/arm64/lib/crypto/poly1305-glue.c
> index 6a661cf048213..c9a74766785bd 100644
> --- a/arch/arm64/lib/crypto/poly1305-glue.c
> +++ b/arch/arm64/lib/crypto/poly1305-glue.c
> @@ -36,18 +36,18 @@ void poly1305_blocks_arch(struct poly1305_block_state *state, const u8 *src,
>        if (static_branch_likely(&have_neon)) {
>                do {
>                        unsigned int todo = min_t(unsigned int, len, SZ_4K);
> 
>                        kernel_neon_begin();
> -                       poly1305_blocks_neon(state, src, todo, 1);
> +                       poly1305_blocks_neon(state, src, todo, padbit);

This would do the wrong thing if someone ever tried to pad a
message more than 4K and called the block function with padbit == 0.
Fortunately it can't happen today as there is no digest interface
to poly1305.

Looking around it seems that this pattern is replicated across
all of our poly1305 implementations so it isn't a big deal.

I presume you will be picking this up via the lib/crypto tree?

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

Re: [PATCH] lib/crypto/poly1305: Fix arm64's poly1305_blocks_arch()

[Eric Biggers]

On Mon, Jun 16, 2025 at 11:27:09AM +0800, Herbert Xu wrote:
> Eric Biggers <ebiggers@kernel.org> wrote:
> > From: Eric Biggers <ebiggers@google.com>
> > 
> > For some reason arm64's Poly1305 code got changed to ignore the padbit
> > argument.  As a result, the output is incorrect when the message length
> > is not a multiple of 16 (which is not reached with the standard
> > ChaCha20Poly1305, but bcachefs could reach this).  Fix this.
> 
> Sorry, it was a cut-n-paste error since I copy the code from
> the update function where the padbit is always 1.
> 
> > diff --git a/arch/arm64/lib/crypto/poly1305-glue.c b/arch/arm64/lib/crypto/poly1305-glue.c
> > index 6a661cf048213..c9a74766785bd 100644
> > --- a/arch/arm64/lib/crypto/poly1305-glue.c
> > +++ b/arch/arm64/lib/crypto/poly1305-glue.c
> > @@ -36,18 +36,18 @@ void poly1305_blocks_arch(struct poly1305_block_state *state, const u8 *src,
> >        if (static_branch_likely(&have_neon)) {
> >                do {
> >                        unsigned int todo = min_t(unsigned int, len, SZ_4K);
> > 
> >                        kernel_neon_begin();
> > -                       poly1305_blocks_neon(state, src, todo, 1);
> > +                       poly1305_blocks_neon(state, src, todo, padbit);
> 
> This would do the wrong thing if someone ever tried to pad a
> message more than 4K and called the block function with padbit == 0.
> Fortunately it can't happen today as there is no digest interface
> to poly1305.

The final partial block is (and needs to be) processed with its own call to
poly1305_blocks().

> Looking around it seems that this pattern is replicated across
> all of our poly1305 implementations so it isn't a big deal.
> 
> I presume you will be picking this up via the lib/crypto tree?

Yes.

- Eric

Re: [PATCH] lib/crypto/poly1305: Fix arm64's poly1305_blocks_arch()

[Eric Biggers]

On Sun, Jun 15, 2025 at 06:06:54PM -0700, Eric Biggers wrote:
> From: Eric Biggers <ebiggers@google.com>
> 
> For some reason arm64's Poly1305 code got changed to ignore the padbit
> argument.  As a result, the output is incorrect when the message length
> is not a multiple of 16 (which is not reached with the standard
> ChaCha20Poly1305, but bcachefs could reach this).  Fix this.
> 
> Fixes: a59e5468a921 ("crypto: arm64/poly1305 - Add block-only interface")
> Reported-by: Kent Overstreet <kent.overstreet@linux.dev>
> Signed-off-by: Eric Biggers <ebiggers@google.com>
> ---
>  arch/arm64/lib/crypto/poly1305-glue.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)

Applied to https://git.kernel.org/pub/scm/linux/kernel/git/ebiggers/linux.git/log/?h=libcrypto-fixes

- Eric