* [PATCH] lib/crypto: arm64: Fix lost Poly1305 carry when resuming NEON state
@ 2026-10-07 22:02 Jérémy Jean
2026-10-07 22:37 ` Eric Biggers
0 siblings, 1 reply; 3+ messages in thread
From: Jérémy Jean @ 2026-10-07 22:02 UTC (permalink / raw)
To: Eric Biggers, Jason A. Donenfeld, Ard Biesheuvel
Cc: linux-crypto, linux-kernel, Jérémy Jean, stable
When poly1305_blocks_neon() resumes from a radix-2^26 state with an odd
number of input blocks, it converts the accumulator back to radix-2^64
before consuming the first block. The final ADC stores the carry into d2,
but the following accumulation and poly1305_mult() use h2. If the
conversion overflows across bit 128, the carry is dropped and the emitted
tag is wrong.
Store the carry back into h2. This matches the other conversion paths and
preserves the represented accumulator.
Fixes: f569ca164751 ("crypto: arm64/poly1305 - incorporate OpenSSL/CRYPTOGAMS NEON implementation")
Cc: stable@vger.kernel.org
Assisted-by: LLM
Signed-off-by: Jérémy Jean <Jeremy.Jean@oss.cyber.gouv.fr>
---
lib/crypto/arm64/poly1305-armv8.pl | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/lib/crypto/arm64/poly1305-armv8.pl b/lib/crypto/arm64/poly1305-armv8.pl
index f1930c6..234398f 100644
--- a/lib/crypto/arm64/poly1305-armv8.pl
+++ b/lib/crypto/arm64/poly1305-armv8.pl
@@ -375,7 +375,7 @@ poly1305_blocks_neon:
adc $h1,$h1,xzr
lsr $h2,x14,#24
adds $h1,$h1,x14,lsl#40
- adc $d2,$h2,xzr // can be partially reduced...
+ adc $h2,$h2,xzr // preserve carry into top limb
ldp $d0,$d1,[$inp],#16 // load input
sub $len,$len,#16
--
2.47.3
^ permalink raw reply related [flat|nested] 3+ messages in thread* Re: [PATCH] lib/crypto: arm64: Fix lost Poly1305 carry when resuming NEON state
2026-10-07 22:02 [PATCH] lib/crypto: arm64: Fix lost Poly1305 carry when resuming NEON state Jérémy Jean
@ 2026-10-07 22:37 ` Eric Biggers
2026-10-08 20:09 ` Jérémy Jean
0 siblings, 1 reply; 3+ messages in thread
From: Eric Biggers @ 2026-10-07 22:37 UTC (permalink / raw)
To: Jérémy Jean
Cc: Jason A. Donenfeld, Ard Biesheuvel, linux-crypto, linux-kernel,
stable
On Wed, Oct 07, 2026 at 10:02:36PM +0000, Jérémy Jean wrote:
> When poly1305_blocks_neon() resumes from a radix-2^26 state with an odd
> number of input blocks, it converts the accumulator back to radix-2^64
> before consuming the first block. The final ADC stores the carry into d2,
> but the following accumulation and poly1305_mult() use h2. If the
> conversion overflows across bit 128, the carry is dropped and the emitted
> tag is wrong.
>
> Store the carry back into h2. This matches the other conversion paths and
> preserves the represented accumulator.
>
> Fixes: f569ca164751 ("crypto: arm64/poly1305 - incorporate OpenSSL/CRYPTOGAMS NEON implementation")
> Cc: stable@vger.kernel.org
> Assisted-by: LLM
> Signed-off-by: Jérémy Jean <Jeremy.Jean@oss.cyber.gouv.fr>
> ---
> lib/crypto/arm64/poly1305-armv8.pl | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/lib/crypto/arm64/poly1305-armv8.pl b/lib/crypto/arm64/poly1305-armv8.pl
> index f1930c6..234398f 100644
> --- a/lib/crypto/arm64/poly1305-armv8.pl
> +++ b/lib/crypto/arm64/poly1305-armv8.pl
> @@ -375,7 +375,7 @@ poly1305_blocks_neon:
> adc $h1,$h1,xzr
> lsr $h2,x14,#24
> adds $h1,$h1,x14,lsl#40
> - adc $d2,$h2,xzr // can be partially reduced...
> + adc $h2,$h2,xzr // preserve carry into top limb
This needs a regression test in lib/crypto/tests/poly1305_kunit.c.
Please include more details in the commit message about under what
circumstances that carry bit could be nonzero.
It seems this was introduced by commit 03dc4adf91c8bc of
https://github.com/dot-asm/cryptogams just before the kernel imported
the code. The bug never reached the copy of this file in OpenSSL. Do
you agree? If so, please mention this information in the commit message
too, and also open an issue at https://github.com/dot-asm/cryptogams so
that it can be fixed there as well.
- Eric
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH] lib/crypto: arm64: Fix lost Poly1305 carry when resuming NEON state
2026-10-07 22:37 ` Eric Biggers
@ 2026-10-08 20:09 ` Jérémy Jean
0 siblings, 0 replies; 3+ messages in thread
From: Jérémy Jean @ 2026-10-08 20:09 UTC (permalink / raw)
To: Eric Biggers
Cc: Jason A. Donenfeld, Ard Biesheuvel, linux-crypto, linux-kernel,
stable
On 2026-10-08 00:37, Eric Biggers wrote:
> On Wed, Oct 07, 2026 at 10:02:36PM +0000, Jérémy Jean wrote:
>> When poly1305_blocks_neon() resumes from a radix-2^26 state with an
>> odd
>> number of input blocks, it converts the accumulator back to radix-2^64
>> before consuming the first block. The final ADC stores the carry into
>> d2,
>> but the following accumulation and poly1305_mult() use h2. If the
>> conversion overflows across bit 128, the carry is dropped and the
>> emitted
>> tag is wrong.
>>
>> Store the carry back into h2. This matches the other conversion paths
>> and
>> preserves the represented accumulator.
>>
>> Fixes: f569ca164751 ("crypto: arm64/poly1305 - incorporate
>> OpenSSL/CRYPTOGAMS NEON implementation")
>> Cc: stable@vger.kernel.org
>> Assisted-by: LLM
>> Signed-off-by: Jérémy Jean <Jeremy.Jean@oss.cyber.gouv.fr>
>> ---
>> lib/crypto/arm64/poly1305-armv8.pl | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/lib/crypto/arm64/poly1305-armv8.pl
>> b/lib/crypto/arm64/poly1305-armv8.pl
>> index f1930c6..234398f 100644
>> --- a/lib/crypto/arm64/poly1305-armv8.pl
>> +++ b/lib/crypto/arm64/poly1305-armv8.pl
>> @@ -375,7 +375,7 @@ poly1305_blocks_neon:
>> adc $h1,$h1,xzr
>> lsr $h2,x14,#24
>> adds $h1,$h1,x14,lsl#40
>> - adc $d2,$h2,xzr // can be partially reduced...
>> + adc $h2,$h2,xzr // preserve carry into top limb
Hello Eric,
> This needs a regression test in lib/crypto/tests/poly1305_kunit.c.
I have prepared one, and will send it along in the v2.
> Please include more details in the commit message about under what
> circumstances that carry bit could be nonzero.
I have expanded the commit message of the bugfix: it should be clearer
in the v2. In particular, I have added an explicit example and provided
probability estimates for which this bug would trigger under nominal
use.
The probability is so low that this should never happen for normal use;
the fix merely fixes correctness of the algorithm.
> It seems this was introduced by commit 03dc4adf91c8bc of
> https://github.com/dot-asm/cryptogams just before the kernel imported
> the code. The bug never reached the copy of this file in OpenSSL. Do
> you agree? If so, please mention this information in the commit
> message
> too, and also open an issue at https://github.com/dot-asm/cryptogams so
> that it can be fixed there as well.
Agreed. I have added a short paragraph about that in the commit message
as well. I will see later to open an issue on
https://github.com/dot-asm/cryptogams.
Regards,
Jérémy
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2026-10-08 20:10 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-10-07 22:02 [PATCH] lib/crypto: arm64: Fix lost Poly1305 carry when resuming NEON state Jérémy Jean
2026-10-07 22:37 ` Eric Biggers
2026-10-08 20:09 ` Jérémy Jean
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox