lib/crypto: Handle large lengths in poly1305 and chacha20poly1305

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 <[email protected]>

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.

---
 lib/crypto/riscv/poly1305.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

index 88f3df44e355..aaf24d0f6674 100644
@@ -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

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.

---
 lib/crypto/mips/poly1305.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

index 85de450f1a93..9e9147f40d17 100644
@@ -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

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").

---
 lib/crypto/arm64/poly1305.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

index 3d4bde857699..205a7b5fcf47 100644
@@ -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

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.

---
 include/crypto/poly1305.h |  2 +-
 lib/crypto/poly1305.c     | 14 +++++++-------
 2 files changed, 8 insertions(+), 8 deletions(-)

index 190beb427c6d..bf53b1df9aa3 100644
@@ -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

index f313ccc4b4dd..f04c4cc2f55c 100644
@@ -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

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.

---
 include/crypto/chacha.h |  4 ++--
 lib/crypto/chacha.c     | 15 ++++++++++++++-
 2 files changed, 16 insertions(+), 3 deletions(-)

index 1cc301a48469..4c18e99eb4f7 100644
@@ -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);
 }

index 86e5d382a4e0..ffa0c75ce501 100644
@@ -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

Thanks! Patches 1-3 look good. I'm less sure about patches 4-5, since they go through some trouble to work around (some implementations of) poly1305_blocks() and chacha_crypt_arch() not taking size_t. Wouldn't it make sense to convert those to size_t at the same time?

- Eric

I think we should just get this out of the way by converting each algorithm entirely to size_t. The series
https://lore.kernel.org/r/[email protected]
does this.

- Eric