[PATCH 0/5] lib/crypto: Handle large lengths in poly1305 and chacha20poly1305

[PATCH 0/5] lib/crypto: Handle large lengths in poly1305 and chacha20poly1305

[Vladimir Kukharenko via B4 Relay]

This series makes the Poly1305 and ChaCha20-Poly1305 library code handle
large lengths:

- poly1305_update() can write past its 16-byte buffer if it holds a
  partial block and nbytes >= 2^32 - desc->buflen.
- The chacha20poly1305 functions use only the low 32 bits of their
  size_t message and associated data lengths.
- On 64-bit MIPS and RISC-V, the poly1305 assembly reads past its input
  for lengths of 2 GiB or more: these ABIs sign-extend its u32 length
  argument, and the assembly uses all 64 bits.  arm64 has the same
  prototype mismatch, with no known failure.

Patches 1-3 fix the assembly prototypes, one architecture per patch.
The assembly already treats the length as 64-bit, so they declare it as
size_t, as commit f3d6cb3dc039 ("lib/crypto: x86/sha1: Migrate
optimized code into library") did for the x86 SHA-1 assembly.
Patches 4 and 5 make poly1305_update() and chacha_crypt() take size_t
lengths.  The patches apply independently, except that on 64-bit MIPS
and RISC-V patch 4 relies on patches 1 and 2 for inputs of 2 GiB or more.

No in-tree caller passes inputs this large (WireGuard passes at most one
packet, big_key at most 1 MiB), so I consider these ABI robustness
improvements rather than fixes and have not added Fixes or Cc stable tags.

I tested with the lib/crypto KUnit suite on UML, x86_64, i386, arm64,
riscv64 and mips64, with a temporary 2 GiB KUnit test on arm64, riscv64
and mips64 (it oopses on riscv64 and mips64 without patches 1 and 2), and
in userspace against OpenSSL for inputs above 4 GiB.

I found the bugs fixed by patches 4 and 5 while writing Coq/VST proofs of
this code with LLM agents, and an LLM review of patch 4 found the one
fixed by patches 1-3.  I used an LLM-based assistant to write the patches
and descriptions, and reviewed its work.

---
Vladimir Kukharenko (5):
      lib/crypto: riscv/poly1305: Fix 32-bit length treated as 64-bit
      lib/crypto: mips/poly1305: Fix 32-bit length treated as 64-bit
      lib/crypto: arm64/poly1305: Fix 32-bit length treated as 64-bit
      lib/crypto: poly1305: Take a size_t length in poly1305_update()
      lib/crypto: chacha: Take a size_t length in chacha_crypt()

 include/crypto/chacha.h     |  4 ++--
 include/crypto/poly1305.h   |  2 +-
 lib/crypto/arm64/poly1305.h |  4 ++--
 lib/crypto/chacha.c         | 15 ++++++++++++++-
 lib/crypto/mips/poly1305.h  |  2 +-
 lib/crypto/poly1305.c       | 14 +++++++-------
 lib/crypto/riscv/poly1305.h |  2 +-
 7 files changed, 28 insertions(+), 15 deletions(-)
---
base-commit: 6a50ce0af5c91fd665d65d357af8ae55db1df7d0
change-id: 20261009-libcrypto-4gib-lengths-c3d762b42b0a

Best regards,
--  
Vladimir Kukharenko <vladimir@genproof.com>

[PATCH 1/5] lib/crypto: riscv/poly1305: Fix 32-bit length treated as 64-bit

[Vladimir Kukharenko via B4 Relay]

From: Vladimir Kukharenko <vladimir@genproof.com>

The C code declares the length argument of the assembly function
poly1305_blocks() as u32.  The assembly comes from OpenSSL/CRYPTOGAMS,
which declares the length as size_t, and on riscv64 it uses all 64 bits
of the register.  However, the RISC-V psABI sign-extends 32-bit
arguments, including unsigned ones, to 64 bits.  So for a length of
2 GiB or more, the assembly computes an end pointer 4 GiB lower than the
correct one, below the start of the input, and the loop reads past the
input until it faults.  poly1305_update() passes all complete blocks to
poly1305_blocks() in one call, so a single poly1305_update() of 2 GiB to
4 GiB triggers this.

Declare the length as size_t, as OpenSSL and the x86_64 code do, rather
than changing the imported assembly.  On riscv32, size_t is 32 bits
wide, so nothing changes there.

This bug was introduced by commit bef9c7559869 ("lib/crypto:
riscv/poly1305: Import OpenSSL/CRYPTOGAMS implementation").  No in-tree
caller passes 2 GiB or more in one call, so this is an ABI robustness
improvement.

I reproduced the fault on riscv64 in QEMU with a single 2 GiB
poly1305_update(), and this patch fixes it.

Assisted-by: LLM
Signed-off-by: Vladimir Kukharenko <vladimir@genproof.com>
---
 lib/crypto/riscv/poly1305.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/crypto/riscv/poly1305.h b/lib/crypto/riscv/poly1305.h
index 88f3df44e355..aaf24d0f6674 100644
--- a/lib/crypto/riscv/poly1305.h
+++ b/lib/crypto/riscv/poly1305.h
@@ -8,7 +8,7 @@
 asmlinkage void poly1305_block_init(struct poly1305_block_state *state,
 				    const u8 raw_key[POLY1305_BLOCK_SIZE]);
 asmlinkage void poly1305_blocks(struct poly1305_block_state *state,
-				const u8 *src, u32 len, u32 hibit);
+				const u8 *src, size_t len, u32 hibit);
 asmlinkage void poly1305_emit(const struct poly1305_state *state,
 			      u8 digest[POLY1305_DIGEST_SIZE],
 			      const u32 nonce[4]);

-- 
2.43.0

[PATCH 2/5] lib/crypto: mips/poly1305: Fix 32-bit length treated as 64-bit

[Vladimir Kukharenko via B4 Relay]

From: Vladimir Kukharenko <vladimir@genproof.com>

The C code declares the length argument of the assembly function
poly1305_blocks() as u32.  The assembly comes from OpenSSL/CRYPTOGAMS,
which declares the length as size_t, and its 64-bit flavour uses all
64 bits of the register.  However, the MIPS n64 ABI keeps 32-bit values,
including unsigned ones, sign-extended in 64-bit registers.  So for a
length of 2 GiB or more, the assembly computes an end pointer 4 GiB
lower than the correct one, below the start of the input, and the loop
reads past the input until it faults.  poly1305_update() passes all
complete blocks to poly1305_blocks() in one call, so a single
poly1305_update() of 2 GiB to 4 GiB triggers this on 64-bit kernels.

Declare the length as size_t, as OpenSSL and the x86_64 code do, rather
than changing the imported assembly.  32-bit kernels use the o32 flavour
of the assembly, and size_t is 32 bits wide there, so nothing changes
for them.

This bug was introduced by commit a11d055e7a64 ("crypto: mips/poly1305 -
incorporate OpenSSL/CRYPTOGAMS optimized implementation").  No in-tree
caller passes 2 GiB or more in one call, so this is an ABI robustness
improvement.

I reproduced the fault on mips64 in QEMU with a single 2 GiB
poly1305_update(), and this patch fixes it.

Assisted-by: LLM
Signed-off-by: Vladimir Kukharenko <vladimir@genproof.com>
---
 lib/crypto/mips/poly1305.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/crypto/mips/poly1305.h b/lib/crypto/mips/poly1305.h
index 85de450f1a93..9e9147f40d17 100644
--- a/lib/crypto/mips/poly1305.h
+++ b/lib/crypto/mips/poly1305.h
@@ -8,7 +8,7 @@
 asmlinkage void poly1305_block_init(struct poly1305_block_state *state,
 				    const u8 raw_key[POLY1305_BLOCK_SIZE]);
 asmlinkage void poly1305_blocks(struct poly1305_block_state *state,
-				const u8 *src, u32 len, u32 hibit);
+				const u8 *src, size_t len, u32 hibit);
 asmlinkage void poly1305_emit(const struct poly1305_state *state,
 			      u8 digest[POLY1305_DIGEST_SIZE],
 			      const u32 nonce[4]);

-- 
2.43.0

[PATCH 3/5] lib/crypto: arm64/poly1305: Fix 32-bit length treated as 64-bit

[Vladimir Kukharenko via B4 Relay]

From: Vladimir Kukharenko <vladimir@genproof.com>

The C code declares the length argument of poly1305_blocks_arm64() and
poly1305_blocks_neon() as u32, but the assembly, which comes from
OpenSSL/CRYPTOGAMS where the length is a size_t, reads it from the
64-bit register x2.  The arm64 ABI leaves the upper 32 bits of a 32-bit
argument unspecified, and gcc and clang don't zero-extend it.  In
practice the upper bits are zero in the current callers, so I don't
know of a failure.

Declare the length as size_t, as OpenSSL and the x86_64 code do, rather
than changing the imported assembly.  The assembly also reads hibit from
x3, but the callers always pass the constant 0 or 1.

This has been the case since commit f569ca164751 ("crypto:
arm64/poly1305 - incorporate OpenSSL/CRYPTOGAMS NEON implementation").

Assisted-by: LLM
Signed-off-by: Vladimir Kukharenko <vladimir@genproof.com>
---
 lib/crypto/arm64/poly1305.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/lib/crypto/arm64/poly1305.h b/lib/crypto/arm64/poly1305.h
index 3d4bde857699..205a7b5fcf47 100644
--- a/lib/crypto/arm64/poly1305.h
+++ b/lib/crypto/arm64/poly1305.h
@@ -14,9 +14,9 @@
 asmlinkage void poly1305_block_init(struct poly1305_block_state *state,
 				    const u8 raw_key[POLY1305_BLOCK_SIZE]);
 asmlinkage void poly1305_blocks_arm64(struct poly1305_block_state *state,
-				      const u8 *src, u32 len, u32 hibit);
+				      const u8 *src, size_t len, u32 hibit);
 asmlinkage void poly1305_blocks_neon(struct poly1305_block_state *state,
-				     const u8 *src, u32 len, u32 hibit);
+				     const u8 *src, size_t len, u32 hibit);
 asmlinkage void poly1305_emit(const struct poly1305_state *state,
 			      u8 digest[POLY1305_DIGEST_SIZE],
 			      const u32 nonce[4]);

-- 
2.43.0

[PATCH 4/5] lib/crypto: poly1305: Take a size_t length in poly1305_update()

[Vladimir Kukharenko via B4 Relay]

From: Vladimir Kukharenko <vladimir@genproof.com>

poly1305_update() adds desc->buflen and nbytes as unsigned int values
to check for a full block.  With 1 to 15 bytes in the buffer and
nbytes >= 2^32 - desc->buflen, the sum wraps.  The tail path then copies
nbytes bytes into the 16-byte desc->buf.

For example, these calls trigger the bug:

    poly1305_update(&desc, p, 1);
    poly1305_update(&desc, p + 1, 0xffffffff);

Commit b646b782e522 ("lib/crypto: poly1305: Consolidate into single
module") introduced this addition.  The previous BLOCK_HASH_UPDATE()
code used size_t for the addition.

The unsigned int nbytes parameter also truncates the size_t lengths
from lib/crypto/chacha20poly1305.c.  For associated data or messages of
4 GiB or more, poly1305_update() authenticates only the first
(length mod 2^32) bytes.  Other hash-like library functions, including
sha256_update() and blake2s_update(), take size_t lengths.

Change nbytes to size_t and compare it with the space left in the
buffer.  poly1305_blocks() takes an unsigned int length, so pass it at
most UINT_MAX bytes per call, rounded down to a whole number of blocks.
On 64-bit MIPS and RISC-V, this relies on the preceding patches, which
make their assembly handle lengths of 2 GiB or more.

No in-tree caller passes inputs this large, so this is an ABI robustness
improvement.  I found the bugs while writing a Coq/VST proof of this
code with LLM agents.  I reproduced the overflow in userspace with
AddressSanitizer, and the truncation by comparing MACs with OpenSSL.

Assisted-by: LLM
Signed-off-by: Vladimir Kukharenko <vladimir@genproof.com>
---
 include/crypto/poly1305.h |  2 +-
 lib/crypto/poly1305.c     | 14 +++++++-------
 2 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/include/crypto/poly1305.h b/include/crypto/poly1305.h
index 190beb427c6d..bf53b1df9aa3 100644
--- a/include/crypto/poly1305.h
+++ b/include/crypto/poly1305.h
@@ -61,7 +61,7 @@ struct poly1305_desc_ctx {
 void poly1305_init(struct poly1305_desc_ctx *desc,
 		   const u8 key[at_least POLY1305_KEY_SIZE]);
 void poly1305_update(struct poly1305_desc_ctx *desc,
-		     const u8 *src, unsigned int nbytes);
+		     const u8 *src, size_t nbytes);
 void poly1305_final(struct poly1305_desc_ctx *desc, u8 *digest);
 
 #endif
diff --git a/lib/crypto/poly1305.c b/lib/crypto/poly1305.c
index f313ccc4b4dd..f04c4cc2f55c 100644
--- a/lib/crypto/poly1305.c
+++ b/lib/crypto/poly1305.c
@@ -35,11 +35,9 @@ void poly1305_init(struct poly1305_desc_ctx *desc,
 EXPORT_SYMBOL(poly1305_init);
 
 void poly1305_update(struct poly1305_desc_ctx *desc,
-		     const u8 *src, unsigned int nbytes)
+		     const u8 *src, size_t nbytes)
 {
-	if (desc->buflen + nbytes >= POLY1305_BLOCK_SIZE) {
-		unsigned int bulk_len;
-
+	if (nbytes >= POLY1305_BLOCK_SIZE - desc->buflen) {
 		if (desc->buflen) {
 			unsigned int l = POLY1305_BLOCK_SIZE - desc->buflen;
 
@@ -52,12 +50,14 @@ void poly1305_update(struct poly1305_desc_ctx *desc,
 			desc->buflen = 0;
 		}
 
-		bulk_len = round_down(nbytes, POLY1305_BLOCK_SIZE);
-		nbytes %= POLY1305_BLOCK_SIZE;
+		while (nbytes >= POLY1305_BLOCK_SIZE) {
+			/* poly1305_blocks() takes an unsigned int length. */
+			unsigned int bulk_len = round_down(min(nbytes, UINT_MAX),
+							   POLY1305_BLOCK_SIZE);
 
-		if (bulk_len) {
 			poly1305_blocks(&desc->state, src, bulk_len, 1);
 			src += bulk_len;
+			nbytes -= bulk_len;
 		}
 	}
 	if (nbytes) {

-- 
2.43.0

[PATCH 5/5] lib/crypto: chacha: Take a size_t length in chacha_crypt()

[Vladimir Kukharenko via B4 Relay]

From: Vladimir Kukharenko <vladimir@genproof.com>

chacha20poly1305_encrypt(), chacha20poly1305_decrypt() and the
xchacha20poly1305 variants take size_t lengths.  They pass these lengths
to chacha20_crypt(), which takes an unsigned int.

For a message of 2^32 + k bytes, chacha20_crypt() processes only the
first k bytes.  In-place encryption leaves the remaining bytes as
plaintext.  Until poly1305_update() took a size_t, those bytes were not
authenticated either.  Commit ed20078b7e33 ("crypto: chacha20poly1305 -
import construction and selftest from Zinc") introduced this behavior.

Change chacha_crypt() and chacha20_crypt() to take size_t lengths, as
poly1305_update() and the other library functions do.  The architecture
code takes an unsigned int length.  Split inputs larger than UINT_MAX
into chunks of at most UINT_MAX bytes.  Round each non-final chunk down
to a multiple of CHACHA_BLOCK_SIZE to keep the keystream continuous.

Lengths up to UINT_MAX make the same single call as before.  The
RFC 8439 limit of 2^38 - 64 bytes per message still applies.

No in-tree caller passes messages this large, so this is an ABI
robustness improvement.  I found the bug while writing a Coq/VST proof
of this code with LLM agents, and reproduced it in userspace.

Assisted-by: LLM
Signed-off-by: Vladimir Kukharenko <vladimir@genproof.com>
---
 include/crypto/chacha.h |  4 ++--
 lib/crypto/chacha.c     | 15 ++++++++++++++-
 2 files changed, 16 insertions(+), 3 deletions(-)

diff --git a/include/crypto/chacha.h b/include/crypto/chacha.h
index 1cc301a48469..4c18e99eb4f7 100644
--- a/include/crypto/chacha.h
+++ b/include/crypto/chacha.h
@@ -86,10 +86,10 @@ static inline void chacha_init(struct chacha_state *state,
 }
 
 void chacha_crypt(struct chacha_state *state, u8 *dst, const u8 *src,
-		  unsigned int bytes, int nrounds);
+		  size_t bytes, int nrounds);
 
 static inline void chacha20_crypt(struct chacha_state *state,
-				  u8 *dst, const u8 *src, unsigned int bytes)
+				  u8 *dst, const u8 *src, size_t bytes)
 {
 	chacha_crypt(state, dst, src, bytes, 20);
 }
diff --git a/lib/crypto/chacha.c b/lib/crypto/chacha.c
index 86e5d382a4e0..ffa0c75ce501 100644
--- a/lib/crypto/chacha.c
+++ b/lib/crypto/chacha.c
@@ -39,8 +39,21 @@ chacha_crypt_generic(struct chacha_state *state, u8 *dst, const u8 *src,
 #endif
 
 void chacha_crypt(struct chacha_state *state, u8 *dst, const u8 *src,
-		  unsigned int bytes, int nrounds)
+		  size_t bytes, int nrounds)
 {
+	/*
+	 * chacha_crypt_arch() takes an unsigned int length.  Process longer
+	 * messages in chunks that are a multiple of CHACHA_BLOCK_SIZE, so that
+	 * the keystream continues correctly from one chunk to the next.
+	 */
+	while (bytes > UINT_MAX) {
+		unsigned int l = round_down(UINT_MAX, CHACHA_BLOCK_SIZE);
+
+		chacha_crypt_arch(state, dst, src, l, nrounds);
+		dst += l;
+		src += l;
+		bytes -= l;
+	}
 	chacha_crypt_arch(state, dst, src, bytes, nrounds);
 }
 EXPORT_SYMBOL_GPL(chacha_crypt);

-- 
2.43.0