[RFC PATCH] libcrypto/chachapoly: Use strict typing for fixed size array arguments

[RFC PATCH] libcrypto/chachapoly: Use strict typing for fixed size array arguments

[Ard Biesheuvel]

From: Ard Biesheuvel <ardb@kernel.org>

The C routines in libcrypto take in/outputs of arbitrary size on the one
hand (plaintext, ciphertext, etc) and in/outputs of fixed size on the
other (keys, nonces, digests).

In many cases, we pass these fixed size in/outputs as array of u8, with
the dimension in square brackets. However, this dimension only serves as
a human readable annotation, and is completely ignored by the compiler.

For example, mixing up any of the const u8[]/const u8* arguments in the
prototype below will not trigger a compiler diagnostic:

void xchacha20poly1305_encrypt(u8 *dst, const u8 *src, const size_t src_len,
                               const u8 *ad, const size_t ad_len,
                               const u8 nonce[XCHACHA20POLY1305_NONCE_SIZE],
                               const u8 key[CHACHA20POLY1305_KEY_SIZE])

If we change this to the below, the codegen is identical, given that the
actual value passed as the argument is the address of the entire array,
which is equal to the address of its first element,

void xchacha20poly1305_encrypt(u8 *dst, const u8 *src, const size_t src_len,
                               const u8 *ad, const size_t ad_len,
                               const u8 (*nonce)[XCHACHA20POLY1305_NONCE_SIZE],
                               const u8 (*key)[CHACHA20POLY1305_KEY_SIZE])

However, this variant is checked more strictly by the compiler, and only
arrays of the correct size are accepted as plain arguments (using the &
operator), and so inadvertent mixing up of arguments or passing buffers
of an incorrect size will trigger an error at build time.

So let's switch to this for the ChaCha20Poly1305 library used by
WireGuard.

Cc: Eric Biggers <ebiggers@kernel.org>
Cc: Jason A. Donenfeld <Jason@zx2c4.com>
Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
---

Sending this RFC as an example, as a starting point for a discussion
whether or not we want to go down this route for all library crypto.

 drivers/net/wireguard/cookie.c    |  8 ++--
 drivers/net/wireguard/noise.c     | 16 ++++----
 drivers/net/wireguard/receive.c   |  2 +-
 drivers/net/wireguard/send.c      |  2 +-
 include/crypto/chacha20poly1305.h | 16 ++++----
 lib/crypto/chacha20poly1305.c     | 41 ++++++++++----------
 6 files changed, 43 insertions(+), 42 deletions(-)

diff --git a/drivers/net/wireguard/cookie.c b/drivers/net/wireguard/cookie.c
index 08731b3fa32b..b0ebbd2f8af4 100644
--- a/drivers/net/wireguard/cookie.c
+++ b/drivers/net/wireguard/cookie.c
@@ -191,8 +191,8 @@ void wg_cookie_message_create(struct message_handshake_cookie *dst,
 
 	make_cookie(cookie, skb, checker);
 	xchacha20poly1305_encrypt(dst->encrypted_cookie, cookie, COOKIE_LEN,
-				  macs->mac1, COOKIE_LEN, dst->nonce,
-				  checker->cookie_encryption_key);
+				  macs->mac1, COOKIE_LEN, &dst->nonce,
+				  &checker->cookie_encryption_key);
 }
 
 void wg_cookie_message_consume(struct message_handshake_cookie *src,
@@ -215,8 +215,8 @@ void wg_cookie_message_consume(struct message_handshake_cookie *src,
 	}
 	ret = xchacha20poly1305_decrypt(
 		cookie, src->encrypted_cookie, sizeof(src->encrypted_cookie),
-		peer->latest_cookie.last_mac1_sent, COOKIE_LEN, src->nonce,
-		peer->latest_cookie.cookie_decryption_key);
+		peer->latest_cookie.last_mac1_sent, COOKIE_LEN, &src->nonce,
+		&peer->latest_cookie.cookie_decryption_key);
 	up_read(&peer->latest_cookie.lock);
 
 	if (ret) {
diff --git a/drivers/net/wireguard/noise.c b/drivers/net/wireguard/noise.c
index 1fe8468f0bef..e4f560df5f03 100644
--- a/drivers/net/wireguard/noise.c
+++ b/drivers/net/wireguard/noise.c
@@ -461,7 +461,7 @@ static void handshake_init(u8 chaining_key[NOISE_HASH_LEN],
 }
 
 static void message_encrypt(u8 *dst_ciphertext, const u8 *src_plaintext,
-			    size_t src_len, u8 key[NOISE_SYMMETRIC_KEY_LEN],
+			    size_t src_len, u8 (*key)[NOISE_SYMMETRIC_KEY_LEN],
 			    u8 hash[NOISE_HASH_LEN])
 {
 	chacha20poly1305_encrypt(dst_ciphertext, src_plaintext, src_len, hash,
@@ -471,7 +471,7 @@ static void message_encrypt(u8 *dst_ciphertext, const u8 *src_plaintext,
 }
 
 static bool message_decrypt(u8 *dst_plaintext, const u8 *src_ciphertext,
-			    size_t src_len, u8 key[NOISE_SYMMETRIC_KEY_LEN],
+			    size_t src_len, u8 (*key)[NOISE_SYMMETRIC_KEY_LEN],
 			    u8 hash[NOISE_HASH_LEN])
 {
 	if (!chacha20poly1305_decrypt(dst_plaintext, src_ciphertext, src_len,
@@ -554,7 +554,7 @@ wg_noise_handshake_create_initiation(struct message_handshake_initiation *dst,
 	/* s */
 	message_encrypt(dst->encrypted_static,
 			handshake->static_identity->static_public,
-			NOISE_PUBLIC_KEY_LEN, key, handshake->hash);
+			NOISE_PUBLIC_KEY_LEN, &key, handshake->hash);
 
 	/* ss */
 	if (!mix_precomputed_dh(handshake->chaining_key, key,
@@ -564,7 +564,7 @@ wg_noise_handshake_create_initiation(struct message_handshake_initiation *dst,
 	/* {t} */
 	tai64n_now(timestamp);
 	message_encrypt(dst->encrypted_timestamp, timestamp,
-			NOISE_TIMESTAMP_LEN, key, handshake->hash);
+			NOISE_TIMESTAMP_LEN, &key, handshake->hash);
 
 	dst->sender_index = wg_index_hashtable_insert(
 		handshake->entry.peer->device->index_hashtable,
@@ -610,7 +610,7 @@ wg_noise_handshake_consume_initiation(struct message_handshake_initiation *src,
 
 	/* s */
 	if (!message_decrypt(s, src->encrypted_static,
-			     sizeof(src->encrypted_static), key, hash))
+			     sizeof(src->encrypted_static), &key, hash))
 		goto out;
 
 	/* Lookup which peer we're actually talking to */
@@ -626,7 +626,7 @@ wg_noise_handshake_consume_initiation(struct message_handshake_initiation *src,
 
 	/* {t} */
 	if (!message_decrypt(t, src->encrypted_timestamp,
-			     sizeof(src->encrypted_timestamp), key, hash))
+			     sizeof(src->encrypted_timestamp), &key, hash))
 		goto out;
 
 	down_read(&handshake->lock);
@@ -708,7 +708,7 @@ bool wg_noise_handshake_create_response(struct message_handshake_response *dst,
 		handshake->preshared_key);
 
 	/* {} */
-	message_encrypt(dst->encrypted_nothing, NULL, 0, key, handshake->hash);
+	message_encrypt(dst->encrypted_nothing, NULL, 0, &key, handshake->hash);
 
 	dst->sender_index = wg_index_hashtable_insert(
 		handshake->entry.peer->device->index_hashtable,
@@ -779,7 +779,7 @@ wg_noise_handshake_consume_response(struct message_handshake_response *src,
 
 	/* {} */
 	if (!message_decrypt(NULL, src->encrypted_nothing,
-			     sizeof(src->encrypted_nothing), key, hash))
+			     sizeof(src->encrypted_nothing), &key, hash))
 		goto fail;
 
 	/* Success! Copy everything to peer */
diff --git a/drivers/net/wireguard/receive.c b/drivers/net/wireguard/receive.c
index eb8851113654..21b4d825e9f7 100644
--- a/drivers/net/wireguard/receive.c
+++ b/drivers/net/wireguard/receive.c
@@ -277,7 +277,7 @@ static bool decrypt_packet(struct sk_buff *skb, struct noise_keypair *keypair)
 
 	if (!chacha20poly1305_decrypt_sg_inplace(sg, skb->len, NULL, 0,
 					         PACKET_CB(skb)->nonce,
-						 keypair->receiving.key))
+						 &keypair->receiving.key))
 		return false;
 
 	/* Another ugly situation of pushing and pulling the header so as to
diff --git a/drivers/net/wireguard/send.c b/drivers/net/wireguard/send.c
index 26e09c30d596..a0d8c541b72c 100644
--- a/drivers/net/wireguard/send.c
+++ b/drivers/net/wireguard/send.c
@@ -215,7 +215,7 @@ static bool encrypt_packet(struct sk_buff *skb, struct noise_keypair *keypair)
 		return false;
 	return chacha20poly1305_encrypt_sg_inplace(sg, plaintext_len, NULL, 0,
 						   PACKET_CB(skb)->nonce,
-						   keypair->sending.key);
+						   &keypair->sending.key);
 }
 
 void wg_packet_send_keepalive(struct wg_peer *peer)
diff --git a/include/crypto/chacha20poly1305.h b/include/crypto/chacha20poly1305.h
index d2ac3ff7dc1e..4b49b229eb77 100644
--- a/include/crypto/chacha20poly1305.h
+++ b/include/crypto/chacha20poly1305.h
@@ -18,32 +18,32 @@ enum chacha20poly1305_lengths {
 void chacha20poly1305_encrypt(u8 *dst, const u8 *src, const size_t src_len,
 			      const u8 *ad, const size_t ad_len,
 			      const u64 nonce,
-			      const u8 key[CHACHA20POLY1305_KEY_SIZE]);
+			      const u8 (*key)[CHACHA20POLY1305_KEY_SIZE]);
 
 bool __must_check
 chacha20poly1305_decrypt(u8 *dst, const u8 *src, const size_t src_len,
 			 const u8 *ad, const size_t ad_len, const u64 nonce,
-			 const u8 key[CHACHA20POLY1305_KEY_SIZE]);
+			 const u8 (*key)[CHACHA20POLY1305_KEY_SIZE]);
 
 void xchacha20poly1305_encrypt(u8 *dst, const u8 *src, const size_t src_len,
 			       const u8 *ad, const size_t ad_len,
-			       const u8 nonce[XCHACHA20POLY1305_NONCE_SIZE],
-			       const u8 key[CHACHA20POLY1305_KEY_SIZE]);
+			       const u8 (*nonce)[XCHACHA20POLY1305_NONCE_SIZE],
+			       const u8 (*key)[CHACHA20POLY1305_KEY_SIZE]);
 
 bool __must_check xchacha20poly1305_decrypt(
 	u8 *dst, const u8 *src, const size_t src_len, const u8 *ad,
-	const size_t ad_len, const u8 nonce[XCHACHA20POLY1305_NONCE_SIZE],
-	const u8 key[CHACHA20POLY1305_KEY_SIZE]);
+	const size_t ad_len, const u8 (*nonce)[XCHACHA20POLY1305_NONCE_SIZE],
+	const u8 (*key)[CHACHA20POLY1305_KEY_SIZE]);
 
 bool chacha20poly1305_encrypt_sg_inplace(struct scatterlist *src, size_t src_len,
 					 const u8 *ad, const size_t ad_len,
 					 const u64 nonce,
-					 const u8 key[CHACHA20POLY1305_KEY_SIZE]);
+					 const u8 (*key)[CHACHA20POLY1305_KEY_SIZE]);
 
 bool chacha20poly1305_decrypt_sg_inplace(struct scatterlist *src, size_t src_len,
 					 const u8 *ad, const size_t ad_len,
 					 const u64 nonce,
-					 const u8 key[CHACHA20POLY1305_KEY_SIZE]);
+					 const u8 (*key)[CHACHA20POLY1305_KEY_SIZE]);
 
 bool chacha20poly1305_selftest(void);
 
diff --git a/lib/crypto/chacha20poly1305.c b/lib/crypto/chacha20poly1305.c
index 0b49d6aedefd..413e06eb1da0 100644
--- a/lib/crypto/chacha20poly1305.c
+++ b/lib/crypto/chacha20poly1305.c
@@ -18,20 +18,21 @@
 #include <linux/module.h>
 #include <linux/unaligned.h>
 
-static void chacha_load_key(u32 *k, const u8 *in)
+static void chacha_load_key(u32 *k, const u8 (*in)[CHACHA20POLY1305_KEY_SIZE])
 {
-	k[0] = get_unaligned_le32(in);
-	k[1] = get_unaligned_le32(in + 4);
-	k[2] = get_unaligned_le32(in + 8);
-	k[3] = get_unaligned_le32(in + 12);
-	k[4] = get_unaligned_le32(in + 16);
-	k[5] = get_unaligned_le32(in + 20);
-	k[6] = get_unaligned_le32(in + 24);
-	k[7] = get_unaligned_le32(in + 28);
+	k[0] = get_unaligned_le32((u8 *)in);
+	k[1] = get_unaligned_le32((u8 *)in + 4);
+	k[2] = get_unaligned_le32((u8 *)in + 8);
+	k[3] = get_unaligned_le32((u8 *)in + 12);
+	k[4] = get_unaligned_le32((u8 *)in + 16);
+	k[5] = get_unaligned_le32((u8 *)in + 20);
+	k[6] = get_unaligned_le32((u8 *)in + 24);
+	k[7] = get_unaligned_le32((u8 *)in + 28);
 }
 
 static void xchacha_init(struct chacha_state *chacha_state,
-			 const u8 *key, const u8 *nonce)
+			 const u8 (*key)[CHACHA20POLY1305_KEY_SIZE],
+			 const u8 (*nonce)[XCHACHA20POLY1305_NONCE_SIZE])
 {
 	u32 k[CHACHA_KEY_WORDS];
 	u8 iv[CHACHA_IV_SIZE];
@@ -42,7 +43,7 @@ static void xchacha_init(struct chacha_state *chacha_state,
 	chacha_load_key(k, key);
 
 	/* Compute the subkey given the original key and first 128 nonce bits */
-	chacha_init(chacha_state, k, nonce);
+	chacha_init(chacha_state, k, (u8 *)nonce);
 	hchacha_block(chacha_state, k, 20);
 
 	chacha_init(chacha_state, k, iv);
@@ -89,7 +90,7 @@ __chacha20poly1305_encrypt(u8 *dst, const u8 *src, const size_t src_len,
 void chacha20poly1305_encrypt(u8 *dst, const u8 *src, const size_t src_len,
 			      const u8 *ad, const size_t ad_len,
 			      const u64 nonce,
-			      const u8 key[CHACHA20POLY1305_KEY_SIZE])
+			      const u8 (*key)[CHACHA20POLY1305_KEY_SIZE])
 {
 	struct chacha_state chacha_state;
 	u32 k[CHACHA_KEY_WORDS];
@@ -111,8 +112,8 @@ EXPORT_SYMBOL(chacha20poly1305_encrypt);
 
 void xchacha20poly1305_encrypt(u8 *dst, const u8 *src, const size_t src_len,
 			       const u8 *ad, const size_t ad_len,
-			       const u8 nonce[XCHACHA20POLY1305_NONCE_SIZE],
-			       const u8 key[CHACHA20POLY1305_KEY_SIZE])
+			       const u8 (*nonce)[XCHACHA20POLY1305_NONCE_SIZE],
+			       const u8 (*key)[CHACHA20POLY1305_KEY_SIZE])
 {
 	struct chacha_state chacha_state;
 
@@ -170,7 +171,7 @@ __chacha20poly1305_decrypt(u8 *dst, const u8 *src, const size_t src_len,
 bool chacha20poly1305_decrypt(u8 *dst, const u8 *src, const size_t src_len,
 			      const u8 *ad, const size_t ad_len,
 			      const u64 nonce,
-			      const u8 key[CHACHA20POLY1305_KEY_SIZE])
+			      const u8 (*key)[CHACHA20POLY1305_KEY_SIZE])
 {
 	struct chacha_state chacha_state;
 	u32 k[CHACHA_KEY_WORDS];
@@ -195,8 +196,8 @@ EXPORT_SYMBOL(chacha20poly1305_decrypt);
 
 bool xchacha20poly1305_decrypt(u8 *dst, const u8 *src, const size_t src_len,
 			       const u8 *ad, const size_t ad_len,
-			       const u8 nonce[XCHACHA20POLY1305_NONCE_SIZE],
-			       const u8 key[CHACHA20POLY1305_KEY_SIZE])
+			       const u8 (*nonce)[XCHACHA20POLY1305_NONCE_SIZE],
+			       const u8 (*key)[CHACHA20POLY1305_KEY_SIZE])
 {
 	struct chacha_state chacha_state;
 
@@ -211,7 +212,7 @@ bool chacha20poly1305_crypt_sg_inplace(struct scatterlist *src,
 				       const size_t src_len,
 				       const u8 *ad, const size_t ad_len,
 				       const u64 nonce,
-				       const u8 key[CHACHA20POLY1305_KEY_SIZE],
+				       const u8 (*key)[CHACHA20POLY1305_KEY_SIZE],
 				       int encrypt)
 {
 	const u8 *pad0 = page_address(ZERO_PAGE(0));
@@ -335,7 +336,7 @@ bool chacha20poly1305_crypt_sg_inplace(struct scatterlist *src,
 bool chacha20poly1305_encrypt_sg_inplace(struct scatterlist *src, size_t src_len,
 					 const u8 *ad, const size_t ad_len,
 					 const u64 nonce,
-					 const u8 key[CHACHA20POLY1305_KEY_SIZE])
+					 const u8 (*key)[CHACHA20POLY1305_KEY_SIZE])
 {
 	return chacha20poly1305_crypt_sg_inplace(src, src_len, ad, ad_len,
 						 nonce, key, 1);
@@ -345,7 +346,7 @@ EXPORT_SYMBOL(chacha20poly1305_encrypt_sg_inplace);
 bool chacha20poly1305_decrypt_sg_inplace(struct scatterlist *src, size_t src_len,
 					 const u8 *ad, const size_t ad_len,
 					 const u64 nonce,
-					 const u8 key[CHACHA20POLY1305_KEY_SIZE])
+					 const u8 (*key)[CHACHA20POLY1305_KEY_SIZE])
 {
 	if (unlikely(src_len < POLY1305_DIGEST_SIZE))
 		return false;
-- 
2.52.0.rc1.455.g30608eb744-goog

Re: [RFC PATCH] libcrypto/chachapoly: Use strict typing for fixed size array arguments

[Jason A. Donenfeld]

On Fri, Nov 14, 2025 at 07:07:07PM +0100, Ard Biesheuvel wrote:
> void xchacha20poly1305_encrypt(u8 *dst, const u8 *src, const size_t src_len,
>                                const u8 *ad, const size_t ad_len,
>                                const u8 (*nonce)[XCHACHA20POLY1305_NONCE_SIZE],
>                                const u8 (*key)[CHACHA20POLY1305_KEY_SIZE])
> 
> However, this variant is checked more strictly by the compiler, and only
> arrays of the correct size are accepted as plain arguments (using the &
> operator), and so inadvertent mixing up of arguments or passing buffers
> of an incorrect size will trigger an error at build time.

Interesting idea! And codegen is the same, you say?

There's another variant of this that doesn't change callsites and keeps
the single pointer, which more accurate reflects what the function does:

void xchacha20poly1305_encrypt(u8 *dst, const u8 *src, const size_t src_len,
                               const u8 *ad, const size_t ad_len,
                               const u8 nonce[static XCHACHA20POLY1305_NONCE_SIZE],
                               const u8 key[static CHACHA20POLY1305_KEY_SIZE])

An obscure use of the `static` keyword, but this is what it's used for -
telling the compiler what size you expect the object to be. Last time I
investigated this, only clang respected it, but now it looks like gcc
does too:

    zx2c4@thinkpad /tmp $ cat a.c
    
    void blah(unsigned char herp[static 7]);
    
    static void schma(void)
    {
        unsigned char good[] = { 1, 2, 3, 4, 5, 6, 7 };
        unsigned char bad[] = { 1, 2, 3, 4, 5, 6 };
        blah(good);
        blah(bad);
    }
    zx2c4@thinkpad /tmp $ gcc -c a.c
    a.c: In function ‘schma’:
    a.c:9:9: warning: ‘blah’ accessing 7 bytes in a region of size 6 [-Wstringop-overflow=]
        9 |         blah(bad);
          |         ^~~~~~~~~
    a.c:9:9: note: referencing argument 1 of type ‘unsigned char[7]’
    a.c:2:6: note: in a call to function ‘blah’
        2 | void blah(unsigned char herp[static 7]);
          |      ^~~~
    zx2c4@thinkpad /tmp $ clang -c a.c
    a.c:9:2: warning: array argument is too small; contains 6 elements, callee requires at least 7
          [-Warray-bounds]
        9 |         blah(bad);
          |         ^    ~~~
    a.c:2:25: note: callee declares array parameter as static here
        2 | void blah(unsigned char herp[static 7]);
          |                         ^   ~~~~~~~~~~
    1 warning generated.


This doesn't account for buffers that are oversize -- the less dangerous
case -- but maybe that's fine, to keep "normal" semantics of function
calls and still get some checking? And adding `static` a bunch of places
is easy. It could apply much wider than just chapoly.

This all makes me wish we had NT's SAL notations though...

Jason

Re: [RFC PATCH] libcrypto/chachapoly: Use strict typing for fixed size array arguments

[Ard Biesheuvel]

On Fri, 14 Nov 2025 at 21:23, Jason A. Donenfeld <Jason@zx2c4.com> wrote:
>
> On Fri, Nov 14, 2025 at 07:07:07PM +0100, Ard Biesheuvel wrote:
> > void xchacha20poly1305_encrypt(u8 *dst, const u8 *src, const size_t src_len,
> >                                const u8 *ad, const size_t ad_len,
> >                                const u8 (*nonce)[XCHACHA20POLY1305_NONCE_SIZE],
> >                                const u8 (*key)[CHACHA20POLY1305_KEY_SIZE])
> >
> > However, this variant is checked more strictly by the compiler, and only
> > arrays of the correct size are accepted as plain arguments (using the &
> > operator), and so inadvertent mixing up of arguments or passing buffers
> > of an incorrect size will trigger an error at build time.
>
> Interesting idea! And codegen is the same, you say?
>

Well, the address values passed into the functions are the same.
Whether or not some compilers may behave differently as a result is a
different matter: I suppose some heuristics may produce different
results knowing the fixed sizes of the inputs.

> There's another variant of this that doesn't change callsites and keeps
> the single pointer, which more accurate reflects what the function does:
>
> void xchacha20poly1305_encrypt(u8 *dst, const u8 *src, const size_t src_len,
>                                const u8 *ad, const size_t ad_len,
>                                const u8 nonce[static XCHACHA20POLY1305_NONCE_SIZE],
>                                const u8 key[static CHACHA20POLY1305_KEY_SIZE])
>

Whoah!

> An obscure use of the `static` keyword, but this is what it's used for -
> telling the compiler what size you expect the object to be. Last time I
> investigated this, only clang respected it, but now it looks like gcc
> does too:
>
>     zx2c4@thinkpad /tmp $ cat a.c
>
>     void blah(unsigned char herp[static 7]);
>
>     static void schma(void)
>     {
>         unsigned char good[] = { 1, 2, 3, 4, 5, 6, 7 };
>         unsigned char bad[] = { 1, 2, 3, 4, 5, 6 };
>         blah(good);
>         blah(bad);
>     }
>     zx2c4@thinkpad /tmp $ gcc -c a.c
>     a.c: In function ‘schma’:
>     a.c:9:9: warning: ‘blah’ accessing 7 bytes in a region of size 6 [-Wstringop-overflow=]
>         9 |         blah(bad);
>           |         ^~~~~~~~~
>     a.c:9:9: note: referencing argument 1 of type ‘unsigned char[7]’
>     a.c:2:6: note: in a call to function ‘blah’
>         2 | void blah(unsigned char herp[static 7]);
>           |      ^~~~
>     zx2c4@thinkpad /tmp $ clang -c a.c
>     a.c:9:2: warning: array argument is too small; contains 6 elements, callee requires at least 7
>           [-Warray-bounds]
>         9 |         blah(bad);
>           |         ^    ~~~
>     a.c:2:25: note: callee declares array parameter as static here
>         2 | void blah(unsigned char herp[static 7]);
>           |                         ^   ~~~~~~~~~~
>     1 warning generated.
>
>
> This doesn't account for buffers that are oversize -- the less dangerous
> case -- but maybe that's fine, to keep "normal" semantics of function
> calls and still get some checking? And adding `static` a bunch of places
> is easy.

Yeah if that is as portable as you say it is, it is a much better
solution, given that the minimum size is the most important:
inadvertently swapping two arguments will still result in a
diagnostic, unless the buffers are the same size, in which case there
is still a bug but not a memory safety issue. And passing a buffer
that is too large is not a memory safety issue either.

> It could apply much wider than just chapoly.
>

Yes, that was always the intent. I used this as an example because it
is low hanging fruit, given that it only has a single user.

> This all makes me wish we had NT's SAL notations though...
>

(quotation needed)

Re: [RFC PATCH] libcrypto/chachapoly: Use strict typing for fixed size array arguments

[Eric Biggers]

[+Linus and Kees]

On Fri, Nov 14, 2025 at 09:33:43PM +0100, Ard Biesheuvel wrote:
> On Fri, 14 Nov 2025 at 21:23, Jason A. Donenfeld <Jason@zx2c4.com> wrote:
> >
> > On Fri, Nov 14, 2025 at 07:07:07PM +0100, Ard Biesheuvel wrote:
> > > void xchacha20poly1305_encrypt(u8 *dst, const u8 *src, const size_t src_len,
> > >                                const u8 *ad, const size_t ad_len,
> > >                                const u8 (*nonce)[XCHACHA20POLY1305_NONCE_SIZE],
> > >                                const u8 (*key)[CHACHA20POLY1305_KEY_SIZE])
> > >
> > > However, this variant is checked more strictly by the compiler, and only
> > > arrays of the correct size are accepted as plain arguments (using the &
> > > operator), and so inadvertent mixing up of arguments or passing buffers
> > > of an incorrect size will trigger an error at build time.
> >
> > Interesting idea! And codegen is the same, you say?
> >
> 
> Well, the address values passed into the functions are the same.
> Whether or not some compilers may behave differently as a result is a
> different matter: I suppose some heuristics may produce different
> results knowing the fixed sizes of the inputs.
> 
> > There's another variant of this that doesn't change callsites and keeps
> > the single pointer, which more accurate reflects what the function does:
> >
> > void xchacha20poly1305_encrypt(u8 *dst, const u8 *src, const size_t src_len,
> >                                const u8 *ad, const size_t ad_len,
> >                                const u8 nonce[static XCHACHA20POLY1305_NONCE_SIZE],
> >                                const u8 key[static CHACHA20POLY1305_KEY_SIZE])
> >
> 
> Whoah!
> 
> > An obscure use of the `static` keyword, but this is what it's used for -
> > telling the compiler what size you expect the object to be. Last time I
> > investigated this, only clang respected it, but now it looks like gcc
> > does too:
> >
> >     zx2c4@thinkpad /tmp $ cat a.c
> >
> >     void blah(unsigned char herp[static 7]);
> >
> >     static void schma(void)
> >     {
> >         unsigned char good[] = { 1, 2, 3, 4, 5, 6, 7 };
> >         unsigned char bad[] = { 1, 2, 3, 4, 5, 6 };
> >         blah(good);
> >         blah(bad);
> >     }
> >     zx2c4@thinkpad /tmp $ gcc -c a.c
> >     a.c: In function ‘schma’:
> >     a.c:9:9: warning: ‘blah’ accessing 7 bytes in a region of size 6 [-Wstringop-overflow=]
> >         9 |         blah(bad);
> >           |         ^~~~~~~~~
> >     a.c:9:9: note: referencing argument 1 of type ‘unsigned char[7]’
> >     a.c:2:6: note: in a call to function ‘blah’
> >         2 | void blah(unsigned char herp[static 7]);
> >           |      ^~~~
> >     zx2c4@thinkpad /tmp $ clang -c a.c
> >     a.c:9:2: warning: array argument is too small; contains 6 elements, callee requires at least 7
> >           [-Warray-bounds]
> >         9 |         blah(bad);
> >           |         ^    ~~~
> >     a.c:2:25: note: callee declares array parameter as static here
> >         2 | void blah(unsigned char herp[static 7]);
> >           |                         ^   ~~~~~~~~~~
> >     1 warning generated.
> >
> >
> > This doesn't account for buffers that are oversize -- the less dangerous
> > case -- but maybe that's fine, to keep "normal" semantics of function
> > calls and still get some checking? And adding `static` a bunch of places
> > is easy.
> 
> Yeah if that is as portable as you say it is, it is a much better
> solution, given that the minimum size is the most important:
> inadvertently swapping two arguments will still result in a
> diagnostic, unless the buffers are the same size, in which case there
> is still a bug but not a memory safety issue. And passing a buffer
> that is too large is not a memory safety issue either.
> 
> > It could apply much wider than just chapoly.
> >
> 
> Yes, that was always the intent. I used this as an example because it
> is low hanging fruit, given that it only has a single user.
> 
> > This all makes me wish we had NT's SAL notations though...
> >
> 
> (quotation needed)

Those are some interesting ideas to make C a bit less bad!

I knew about the 'static' trick with array parameters, and I used to use
it in other projects.  It's a bit obscure, but it's in the C standard,
and both gcc and clang support the syntax.  It indeed causes clang to
start warning about too-small arrays, via -Warray-bounds which is
enabled by default.  So if we e.g. change:

    void sha256(const u8 *data, size_t len, u8 out[SHA256_DIGEST_SIZE]);

to

    void sha256(const u8 *data, size_t len, u8 out[static SHA256_DIGEST_SIZE]);

... then clang warns if a caller passes an array smaller than
SHA256_DIGEST_SIZE as 'out' (if its size is statically known).

gcc can actually warn about the too-small array regardless of 'static'.
However, gcc's warning is under -Wstringop-overflow, which we never see
because the kernel build system disables -Wstringop-overflow with gcc.

So, for now the benefit of adding 'static' would be to get warnings
about too-small arrays with one of the two supported compilers (clang).
It's too bad 'static' isn't the default behavior for array parameters in
C, but oh well...

I think it's worthwhile adding it to get better warnings, though we
should check with Linus whether he'd be okay with kernel code starting
to use this relatively obscure feature of C.

I think the 'static' trick would be better than Ard's suggestion of:

    void sha256(const u8 *data, size_t len, u8 (*out)[SHA256_DIGEST_SIZE]);

... since the "pointer to array of N elements" type would make things
difficult for callers that have any other type.  For example callers
wouldn't be able to directly use 'u8 *a', 'u8 a[M]' with size M > N, or
even a function argument 'u8 a[N]' since C implicitly converts that to
'u8 *'.  They'd need to cast it first.

- Eric

Re: [RFC PATCH] libcrypto/chachapoly: Use strict typing for fixed size array arguments

[Linus Torvalds]

On Fri, 14 Nov 2025 at 18:16, Eric Biggers <ebiggers@kernel.org> wrote:
>
> I think it's worthwhile adding it to get better warnings, though we
> should check with Linus whether he'd be okay with kernel code starting
> to use this relatively obscure feature of C.

I wouldn't worry about the feature being obscure. We very obviously
use entirely non-standard features, and regularly depend on "sane
implementation" as opposed to any documentation language-lawyering.

And we already have existing users of this syntax, so it's not even
new to the kernel - it's just obscure and unusual.

The main issue with the whole 'static' thing is just that the syntax
is such a horrible hack, where people obviously picked an existing
keyword that made absolutely no sense, but was also guaranteed to have
no backwards compatibility issues.

So the syntax is disgusting and nonsensical, but the feature certainly
isn't wrong.

So *if* we end up using this syntax more widely, I suspect we'd want
to have a macro that makes the semantics more obvious, even if it's
something silly and trivial like

   #define min_array_size(n) static n

just so that people who aren't familiar with that crazy syntax
understand what it means.

But as mentioned, we already have a handful of users, and I don't
think it has ever caused any actual confusion or that people have even
noticed. So that wrapper would be more of a "if it becomes a more
common pattern, maybe we could document it better", although the
counter-argument is that "if it really becomes common, people will be
aware of it anyway".

Anyway, I have no objections to the crypto code using that odd syntax.
It's _odd_, but absolutely not wrong.

                Linus

Re: [RFC PATCH] libcrypto/chachapoly: Use strict typing for fixed size array arguments

[Linus Torvalds]

On Sat, 15 Nov 2025 at 09:11, Linus Torvalds
<torvalds@linux-foundation.org> wrote:
>
> And we already have existing users of this syntax, so it's not even
> new to the kernel - it's just obscure and unusual.

Actually, looking around, people removed one of the users because
sparse didn't understand it.

But that was over a decade ago: see commit 1ee9fcc1a021
("powerpc/perf/hv-24x7: Remove [static 4096], sparse chokes on it").

And sparse was updated to understand that extended array declaration
syntax back in 2020, so that's no longer an issue.

             Linus

Re: [RFC PATCH] libcrypto/chachapoly: Use strict typing for fixed size array arguments

[Jason A. Donenfeld]

Hi Linus,

On Sat, Nov 15, 2025 at 09:11:31AM -0800, Linus Torvalds wrote:
> So *if* we end up using this syntax more widely, I suspect we'd want
> to have a macro that makes the semantics more obvious, even if it's
> something silly and trivial like
> 
>    #define min_array_size(n) static n
> 
> just so that people who aren't familiar with that crazy syntax
> understand what it means.

Oh that's a good suggestion. I'll see if I can rig that up and send
something.

Jason

Re: [RFC PATCH] libcrypto/chachapoly: Use strict typing for fixed size array arguments

[Alejandro Colomar]

[-- Attachment #1: Type: text/plain, Size: 2480 bytes --]

Hi,

Jason said:
> On Sat, Nov 15, 2025 at 09:11:31AM -0800, Linus Torvalds wrote:
> > So *if* we end up using this syntax more widely, I suspect we'd want
> > to have a macro that makes the semantics more obvious, even if it's
> > something silly and trivial like
> > 
> >    #define min_array_size(n) static n
> > 
> > just so that people who aren't familiar with that crazy syntax
> > understand what it means.
> 
> Oh that's a good suggestion. I'll see if I can rig that up and send
> something.

Be careful about [static n].  It has implications that you're probably
not aware of.  Also, it doesn't have some implications you might expect
from it.

-  [static n] on an argument implies __attribute__((nonnull())) on that
   argument; it means that the argument can't be null.  You may want to
   make sure you're using -fno-delete-null-pointer-checks if you use
   [static n].

-  [static n] implies that n>0.  You should make sure that n>0, or UB
   would be triggered.

-  [n] means two promises traditionally:
   -  The caller will provide at least n elements.
   -  The callee will use no more than n elements.
   However, [static n] only carries the first promise.  According to
   ISO C, the callee may access elements beyond that.
   GCC, as a quality implementation, enforces the second promise too,
   but this is not portable; you should make sure that all supported
   compilers enforces that as an extension.

-  Plus, it's just brain-damaged noise.

I recommend that you talk with GCC to fix the issues with
-Wstringop-overflow that don't allow you to use [n] safely.  That would
be useful anyway.

On the other hand, to resolve the issue at hand, how about an
alternative approach?

void xchacha20poly1305_encrypt(u8 *dst, const u8 *src, const size_t src_len,
                               const u8 *ad, const size_t ad_len,
                               const u8 nonce[XCHACHA20POLY1305_NONCE_SIZE],
                               const u8 key[CHACHA20POLY1305_KEY_SIZE]);

#define xchacha20poly1305_encrypt_arr(dst, src, slen, ad, ad_len, nonce, k)\
({                                                                    \
	static_assert(ARRAY_SIZE(nonce) == XCHACHA20POLY1305_NONCE_SIZE);\
	static_assert(ARRAY_SIZE(key) == CHACHA20POLY1305_KEY_SIZE);  \
	xchacha20poly1305_encrypt(dst, src, slen, ad, ad_len, nonce, k);\
})


Have a lovely night!
Alex

-- 
<https://www.alejandro-colomar.es>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [RFC PATCH] libcrypto/chachapoly: Use strict typing for fixed size array arguments

[Eric Biggers]

On Tue, Dec 02, 2025 at 02:12:47AM +0100, Alejandro Colomar wrote:
> Be careful about [static n].  It has implications that you're probably
> not aware of.  Also, it doesn't have some implications you might expect
> from it.
> 
> -  [static n] on an argument implies __attribute__((nonnull())) on that
>    argument; it means that the argument can't be null.  You may want to
>    make sure you're using -fno-delete-null-pointer-checks if you use
>    [static n].

The kernel uses -fno-delete-null-pointer-checks.

As for the caller side, isn't it the expected behavior?  NULL isn't
at_least n, unless n == 0 (see below).

> -  [static n] implies that n>0.  You should make sure that n>0, or UB
>    would be triggered.

There isn't any reason to use it on an array parameter with size 0,
though.  Unless someone uses it on a VLA where the size is a previous
function parameter, but that's not what this is wanted for.

> -  [n] means two promises traditionally:
>    -  The caller will provide at least n elements.
>    -  The callee will use no more than n elements.
>    However, [static n] only carries the first promise.  According to
>    ISO C, the callee may access elements beyond that.
>    GCC, as a quality implementation, enforces the second promise too,
>    but this is not portable; you should make sure that all supported
>    compilers enforces that as an extension.

While it would be helpful to get a warning in the second case too, the
first case is already helpful (and more important anyway).

> -  Plus, it's just brain-damaged noise.
> 
> I recommend that you talk with GCC to fix the issues with
> -Wstringop-overflow that don't allow you to use [n] safely.  That would
> be useful anyway.

It seems the ship already sailed decades ago, though: [n] has always
been "advisory" in C.  [static n] is needed to make it be enforced, and
surely it was done that way for backwards compatibility.

Perhaps people would like to volunteer to get gcc and clang to provide
an option to provide nonstandard behavior where [n] is enforced, and
then push to get the C standard revised to specify that behavior.  It
sounds great to me, but that would of course be a very long project.

In the mean time, we don't need to delay using the tool we have now.

> On the other hand, to resolve the issue at hand, how about an
> alternative approach?
> 
> void xchacha20poly1305_encrypt(u8 *dst, const u8 *src, const size_t src_len,
>                                const u8 *ad, const size_t ad_len,
>                                const u8 nonce[XCHACHA20POLY1305_NONCE_SIZE],
>                                const u8 key[CHACHA20POLY1305_KEY_SIZE]);
> 
> #define xchacha20poly1305_encrypt_arr(dst, src, slen, ad, ad_len, nonce, k)\
> ({                                                                    \
> 	static_assert(ARRAY_SIZE(nonce) == XCHACHA20POLY1305_NONCE_SIZE);\
> 	static_assert(ARRAY_SIZE(key) == CHACHA20POLY1305_KEY_SIZE);  \
> 	xchacha20poly1305_encrypt(dst, src, slen, ad, ad_len, nonce, k);\
> })

No.  That would be more code, would double the API size, and make it the
caller's responsibility to decide which one to call.  And often there
won't be a correct option, as the caller may have arrays that are larger
than the required size, or a mix of arrays and pointers, etc.

- Eric

Re: [RFC PATCH] libcrypto/chachapoly: Use strict typing for fixed size array arguments

[Alejandro Colomar]

[-- Attachment #1: Type: text/plain, Size: 8854 bytes --]

[CC += Aaron BAllman]

Hi Eric,

On Tue, Dec 02, 2025 at 01:57:50AM +0000, Eric Biggers wrote:
> On Tue, Dec 02, 2025 at 02:12:47AM +0100, Alejandro Colomar wrote:
> > Be careful about [static n].  It has implications that you're probably
> > not aware of.  Also, it doesn't have some implications you might expect
> > from it.
> > 
> > -  [static n] on an argument implies __attribute__((nonnull())) on that
> >    argument; it means that the argument can't be null.  You may want to
> >    make sure you're using -fno-delete-null-pointer-checks if you use
> >    [static n].
> 
> The kernel uses -fno-delete-null-pointer-checks.

Ok.

> As for the caller side, isn't it the expected behavior?  NULL isn't
> at_least n, unless n == 0 (see below).

NULL is not an array of zero elements.  NULL is an invalid pointer,
often used as a sentinel value.

Consider for example the freezero(3) function from OpenBSDs libc.  If
we could use arrays of void, we could define it as

	void
	freezero(size_t n;
	    void p[n], size_t n)
	{
		if (p != NULL)
			explicit_bzero(p, n);
		free(p);
	}

Where p is either a null pointer, or a pointer to an object with at
least n bytes.

Often, APIs will want nonnull pointers, but this isn't special of [n].
APIs taking null pointers are rare per-se.

> > -  [static n] implies that n>0.  You should make sure that n>0, or UB
> >    would be triggered.
> 
> There isn't any reason to use it on an array parameter with size 0,
> though.  Unless someone uses it on a VLA where the size is a previous
> function parameter, but that's not what this is wanted for.

Indeed, be careful about the latter.  In this specific case, it may be
not the case, but you should be aware of this issue in case you consider
using [static n] more often.  This could be a footgun.

> > -  [n] means two promises traditionally:
> >    -  The caller will provide at least n elements.
> >    -  The callee will use no more than n elements.
> >    However, [static n] only carries the first promise.  According to
> >    ISO C, the callee may access elements beyond that.
> >    GCC, as a quality implementation, enforces the second promise too,
> >    but this is not portable; you should make sure that all supported
> >    compilers enforces that as an extension.
> 
> While it would be helpful to get a warning in the second case too, the
> first case is already helpful (and more important anyway).

There are other ways to get the same diagnostic without the dangers of
[static n].

> > -  Plus, it's just brain-damaged noise.
> > 
> > I recommend that you talk with GCC to fix the issues with
> > -Wstringop-overflow that don't allow you to use [n] safely.  That would
> > be useful anyway.
> 
> It seems the ship already sailed decades ago, though: [n] has always
> been "advisory" in C.  [static n] is needed to make it be enforced, and
> surely it was done that way for backwards compatibility.

This is not true.

-  [n] is advisory in standard C, but in GCC, it can be mandatory,
   as long as you use proper compiler flags.

-  [static n] is unnecessary, and --except for the dangers mentioned
   above--, is entirely ignored by many compilers, including GCC.

	alx@devuan:~/tmp$ cat array-bounds.c 
	void g(int a[43]);

	void f(int a[42]);
	void f(int a[42])
	{
		a[100] = 7;
		g(a);
	}

	void gs(int a[static 43]);

	void fs(int a[static 42]);
	void fs(int a[static 42])
	{
		a[101] = 8;
		gs(a);
	}
	alx@devuan:~/tmp$ gcc -Wall -Wextra -O2 -S array-bounds.c
	array-bounds.c: In function ‘f’:
	array-bounds.c:7:9: warning: ‘g’ accessing 172 bytes in a region of size 168 [-Wstringop-overflow=]
	    7 |         g(a);
	      |         ^~~~
	array-bounds.c:7:9: note: referencing argument 1 of type ‘int[43]’
	array-bounds.c:1:6: note: in a call to function ‘g’
	    1 | void g(int a[43]);
	      |      ^
	array-bounds.c: In function ‘fs’:
	array-bounds.c:16:9: warning: ‘gs’ accessing 172 bytes in a region of size 168 [-Wstringop-overflow=]
	   16 |         gs(a);
	      |         ^~~~~
	array-bounds.c:16:9: note: referencing argument 1 of type ‘int[43]’
	array-bounds.c:10:6: note: in a call to function ‘gs’
	   10 | void gs(int a[static 43]);
	      |      ^~
	array-bounds.c: In function ‘f’:
	array-bounds.c:6:10: warning: array subscript 100 is outside array bounds of ‘int[42]’ [-Warray-bounds=]
	    6 |         a[100] = 7;
	      |         ~^~~~~
	array-bounds.c:4:12: note: at offset 400 into object ‘a’ of size [0, 168]
	    4 | void f(int a[42])
	      |        ~~~~^~~~~
	array-bounds.c: In function ‘fs’:
	array-bounds.c:15:10: warning: array subscript 101 is outside array bounds of ‘int[42]’ [-Warray-bounds=]
	   15 |         a[101] = 8;
	      |         ~^~~~~
	array-bounds.c:13:13: note: at offset 404 into object ‘a’ of size [0, 168]
	   13 | void fs(int a[static 42])
	      |         ~~~~^~~~~~~~~~~~


> Perhaps people would like to volunteer to get gcc and clang to provide
> an option to provide nonstandard behavior where [n] is enforced,

GCC already enforces [n].  See above.  I'm using:

	alx@devuan:~/tmp$ gcc --version
	gcc (Debian 15.2.0-9) 15.2.0
	Copyright (C) 2025 Free Software Foundation, Inc.
	This is free software; see the source for copying conditions.  There is NO
	warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.

Clang doesn't enforce it.  Clang doesn't even enforce [static n].
Clang is in fact quite bad at diagnosing array bounds violations.
Aaron, could you please fix this in Clang?

	alx@devuan:~/tmp$ cat array-bounds.c 
	void g(int a[43]);

	void f(int a[42]);
	void f(int a[42])
	{
		a[100] = 7;
		g(a);
	}

	void gs(int a[static 43]);

	void fs(int a[static 42]);
	void fs(int a[static 42])
	{
		a[101] = 8;
		gs(a);
	}
	alx@devuan:~/tmp$ clang -S -Weverything array-bounds.c 
	array-bounds.c:6:2: warning: unsafe buffer access [-Wunsafe-buffer-usage]
	    6 |         a[100] = 7;
	      |         ^
	array-bounds.c:15:2: warning: unsafe buffer access [-Wunsafe-buffer-usage]
	   15 |         a[101] = 8;
	      |         ^
	2 warnings generated.
	alx@devuan:~/tmp$ clang-21 -S -Weverything array-bounds.c 
	array-bounds.c:6:2: warning: unsafe buffer access
	      [-Wunsafe-buffer-usage]
	    6 |         a[100] = 7;
	      |         ^
	array-bounds.c:15:2: warning: unsafe buffer access
	      [-Wunsafe-buffer-usage]
	   15 |         a[101] = 8;
	      |         ^
	2 warnings generated.

> and
> then push to get the C standard revised to specify that behavior.

I'm working on convincing the C Committee regarding this at the moment.
In the meantime, we have GCC available.

>  It
> sounds great to me, but that would of course be a very long project.

Not so much.  GCC is here already.  And the C Committee could be
convinced.

> In the mean time, we don't need to delay using the tool we have now.

Are you sure [static n] is a tool?  I don't see any compiler diagnosing
it differently from [n].  Neither GCC nor Clang diagnose it, from what
I've shown above.

> > On the other hand, to resolve the issue at hand, how about an
> > alternative approach?
> > 
> > void xchacha20poly1305_encrypt(u8 *dst, const u8 *src, const size_t src_len,
> >                                const u8 *ad, const size_t ad_len,
> >                                const u8 nonce[XCHACHA20POLY1305_NONCE_SIZE],
> >                                const u8 key[CHACHA20POLY1305_KEY_SIZE]);
> > 
> > #define xchacha20poly1305_encrypt_arr(dst, src, slen, ad, ad_len, nonce, k)\
> > ({                                                                    \
> > 	static_assert(ARRAY_SIZE(nonce) == XCHACHA20POLY1305_NONCE_SIZE);\
> > 	static_assert(ARRAY_SIZE(key) == CHACHA20POLY1305_KEY_SIZE);  \
> > 	xchacha20poly1305_encrypt(dst, src, slen, ad, ad_len, nonce, k);\
> > })
> 
> No.  That would be more code, would double the API size, and make it the
> caller's responsibility to decide which one to call.

You could name the function as __xchacha20poly1305_dont_call_me_directly()
to make sure it's never called.

>  And often there
> won't be a correct option, as the caller may have arrays that are larger
> than the required size, or a mix of arrays and pointers, etc.

If you want to enforce a minimum array size, you could change the static
assertions to do >= instead.

You could also lift the check when the input is a pointer:

	static_assert(!is_array(nonce)
	              || sizeof(nonce) >= XCHACHA20POLY1305_NONCE_SIZE);

It all depends on what you need exactly.


Have a lovely day!
Alex

> 
> - Eric

-- 
<https://www.alejandro-colomar.es>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [RFC PATCH] libcrypto/chachapoly: Use strict typing for fixed size array arguments

[Alejandro Colomar]

[-- Attachment #1: Type: text/plain, Size: 13697 bytes --]

On Tue, Dec 02, 2025 at 11:14:20AM +0100, Alejandro Colomar wrote:
> [CC += Aaron BAllman]
> 
> Hi Eric,
> 
> On Tue, Dec 02, 2025 at 01:57:50AM +0000, Eric Biggers wrote:
> > On Tue, Dec 02, 2025 at 02:12:47AM +0100, Alejandro Colomar wrote:
> > > Be careful about [static n].  It has implications that you're probably
> > > not aware of.  Also, it doesn't have some implications you might expect
> > > from it.
> > > 
> > > -  [static n] on an argument implies __attribute__((nonnull())) on that
> > >    argument; it means that the argument can't be null.  You may want to
> > >    make sure you're using -fno-delete-null-pointer-checks if you use
> > >    [static n].
> > 
> > The kernel uses -fno-delete-null-pointer-checks.
> 
> Ok.
> 
> > As for the caller side, isn't it the expected behavior?  NULL isn't
> > at_least n, unless n == 0 (see below).
> 
> NULL is not an array of zero elements.  NULL is an invalid pointer,
> often used as a sentinel value.
> 
> Consider for example the freezero(3) function from OpenBSDs libc.  If
> we could use arrays of void, we could define it as
> 
> 	void
> 	freezero(size_t n;
> 	    void p[n], size_t n)
> 	{
> 		if (p != NULL)
> 			explicit_bzero(p, n);
> 		free(p);
> 	}
> 
> Where p is either a null pointer, or a pointer to an object with at
> least n bytes.
> 
> Often, APIs will want nonnull pointers, but this isn't special of [n].
> APIs taking null pointers are rare per-se.
> 
> > > -  [static n] implies that n>0.  You should make sure that n>0, or UB
> > >    would be triggered.
> > 
> > There isn't any reason to use it on an array parameter with size 0,
> > though.  Unless someone uses it on a VLA where the size is a previous
> > function parameter, but that's not what this is wanted for.
> 
> Indeed, be careful about the latter.  In this specific case, it may be
> not the case, but you should be aware of this issue in case you consider
> using [static n] more often.  This could be a footgun.
> 
> > > -  [n] means two promises traditionally:
> > >    -  The caller will provide at least n elements.
> > >    -  The callee will use no more than n elements.
> > >    However, [static n] only carries the first promise.  According to
> > >    ISO C, the callee may access elements beyond that.
> > >    GCC, as a quality implementation, enforces the second promise too,
> > >    but this is not portable; you should make sure that all supported
> > >    compilers enforces that as an extension.
> > 
> > While it would be helpful to get a warning in the second case too, the
> > first case is already helpful (and more important anyway).
> 
> There are other ways to get the same diagnostic without the dangers of
> [static n].
> 
> > > -  Plus, it's just brain-damaged noise.
> > > 
> > > I recommend that you talk with GCC to fix the issues with
> > > -Wstringop-overflow that don't allow you to use [n] safely.  That would
> > > be useful anyway.
> > 
> > It seems the ship already sailed decades ago, though: [n] has always
> > been "advisory" in C.  [static n] is needed to make it be enforced, and
> > surely it was done that way for backwards compatibility.
> 
> This is not true.
> 
> -  [n] is advisory in standard C, but in GCC, it can be mandatory,
>    as long as you use proper compiler flags.
> 
> -  [static n] is unnecessary, and --except for the dangers mentioned
>    above--, is entirely ignored by many compilers, including GCC.
> 
> 	alx@devuan:~/tmp$ cat array-bounds.c 
> 	void g(int a[43]);
> 
> 	void f(int a[42]);
> 	void f(int a[42])
> 	{
> 		a[100] = 7;
> 		g(a);
> 	}
> 
> 	void gs(int a[static 43]);
> 
> 	void fs(int a[static 42]);
> 	void fs(int a[static 42])
> 	{
> 		a[101] = 8;
> 		gs(a);
> 	}
> 	alx@devuan:~/tmp$ gcc -Wall -Wextra -O2 -S array-bounds.c
> 	array-bounds.c: In function ‘f’:
> 	array-bounds.c:7:9: warning: ‘g’ accessing 172 bytes in a region of size 168 [-Wstringop-overflow=]
> 	    7 |         g(a);
> 	      |         ^~~~
> 	array-bounds.c:7:9: note: referencing argument 1 of type ‘int[43]’
> 	array-bounds.c:1:6: note: in a call to function ‘g’
> 	    1 | void g(int a[43]);
> 	      |      ^
> 	array-bounds.c: In function ‘fs’:
> 	array-bounds.c:16:9: warning: ‘gs’ accessing 172 bytes in a region of size 168 [-Wstringop-overflow=]
> 	   16 |         gs(a);
> 	      |         ^~~~~
> 	array-bounds.c:16:9: note: referencing argument 1 of type ‘int[43]’
> 	array-bounds.c:10:6: note: in a call to function ‘gs’
> 	   10 | void gs(int a[static 43]);
> 	      |      ^~
> 	array-bounds.c: In function ‘f’:
> 	array-bounds.c:6:10: warning: array subscript 100 is outside array bounds of ‘int[42]’ [-Warray-bounds=]
> 	    6 |         a[100] = 7;
> 	      |         ~^~~~~
> 	array-bounds.c:4:12: note: at offset 400 into object ‘a’ of size [0, 168]
> 	    4 | void f(int a[42])
> 	      |        ~~~~^~~~~
> 	array-bounds.c: In function ‘fs’:
> 	array-bounds.c:15:10: warning: array subscript 101 is outside array bounds of ‘int[42]’ [-Warray-bounds=]
> 	   15 |         a[101] = 8;
> 	      |         ~^~~~~
> 	array-bounds.c:13:13: note: at offset 404 into object ‘a’ of size [0, 168]
> 	   13 | void fs(int a[static 42])
> 	      |         ~~~~^~~~~~~~~~~~
> 
> 
> > Perhaps people would like to volunteer to get gcc and clang to provide
> > an option to provide nonstandard behavior where [n] is enforced,
> 
> GCC already enforces [n].  See above.  I'm using:
> 
> 	alx@devuan:~/tmp$ gcc --version
> 	gcc (Debian 15.2.0-9) 15.2.0
> 	Copyright (C) 2025 Free Software Foundation, Inc.
> 	This is free software; see the source for copying conditions.  There is NO
> 	warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
> 
> Clang doesn't enforce it.  Clang doesn't even enforce [static n].
> Clang is in fact quite bad at diagnosing array bounds violations.
> Aaron, could you please fix this in Clang?

Self-correction: Actually, there's different enforcement in one case:

	alx@devuan:~/tmp$ cat array-bounds.c
	void g(int a[43]);

	void f(int a[42]);
	void f(int a[42])
	{
		a[100] = 7;
		g(a);
	}

	void gs(int a[static 43]);

	void fs(int a[static 42]);
	void fs(int a[static 42])
	{
		a[101] = 8;
		gs(a);
	}

	int
	main(void)
	{
		int a[2];

		f(a);
		fs(a);
	}
	alx@devuan:~/tmp$ clang -Weverything -S array-bounds.c
	array-bounds.c:25:2: warning: array argument is too small; contains 2 elements, callee requires at least 42 [-Warray-bounds]
	   25 |         fs(a);
	      |         ^  ~
	array-bounds.c:13:13: note: callee declares array parameter as static here
	   13 | void fs(int a[static 42])
	      |             ^~~~~~~~~~~~
	array-bounds.c:6:2: warning: unsafe buffer access [-Wunsafe-buffer-usage]
	    6 |         a[100] = 7;
	      |         ^
	array-bounds.c:15:2: warning: unsafe buffer access [-Wunsafe-buffer-usage]
	   15 |         a[101] = 8;
	      |         ^
	3 warnings generated.

which GCC also warns, but under -Wstringop-overflow, which is the issue
the kernel has.

	alx@devuan:~/tmp$ gcc -Wall -Wextra -S -O2 array-bounds.c
	array-bounds.c: In function ‘f’:
	array-bounds.c:7:9: warning: ‘g’ accessing 172 bytes in a region of size 168 [-Wstringop-overflow=]
	    7 |         g(a);
	      |         ^~~~
	array-bounds.c:7:9: note: referencing argument 1 of type ‘int[43]’
	array-bounds.c:1:6: note: in a call to function ‘g’
	    1 | void g(int a[43]);
	      |      ^
	array-bounds.c: In function ‘fs’:
	array-bounds.c:16:9: warning: ‘gs’ accessing 172 bytes in a region of size 168 [-Wstringop-overflow=]
	   16 |         gs(a);
	      |         ^~~~~
	array-bounds.c:16:9: note: referencing argument 1 of type ‘int[43]’
	array-bounds.c:10:6: note: in a call to function ‘gs’
	   10 | void gs(int a[static 43]);
	      |      ^~
	array-bounds.c: In function ‘main’:
	array-bounds.c:24:9: warning: ‘f’ accessing 168 bytes in a region of size 8 [-Wstringop-overflow=]
	   24 |         f(a);
	      |         ^~~~
	array-bounds.c:24:9: note: referencing argument 1 of type ‘int[42]’
	array-bounds.c:4:6: note: in a call to function ‘f’
	    4 | void f(int a[42])
	      |      ^
	array-bounds.c:25:9: warning: ‘fs’ accessing 168 bytes in a region of size 8 [-Wstringop-overflow=]
	   25 |         fs(a);
	      |         ^~~~~
	array-bounds.c:25:9: note: referencing argument 1 of type ‘int[42]’
	array-bounds.c:13:6: note: in a call to function ‘fs’
	   13 | void fs(int a[static 42])
	      |      ^~
	array-bounds.c: In function ‘f’:
	array-bounds.c:6:10: warning: array subscript 100 is outside array bounds of ‘int[42]’ [-Warray-bounds=]
	    6 |         a[100] = 7;
	      |         ~^~~~~
	array-bounds.c:4:12: note: at offset 400 into object ‘a’ of size [0, 168]
	    4 | void f(int a[42])
	      |        ~~~~^~~~~
	array-bounds.c: In function ‘fs’:
	array-bounds.c:15:10: warning: array subscript 101 is outside array bounds of ‘int[42]’ [-Warray-bounds=]
	   15 |         a[101] = 8;
	      |         ~^~~~~
	array-bounds.c:13:13: note: at offset 404 into object ‘a’ of size [0, 168]
	   13 | void fs(int a[static 42])
	      |         ~~~~^~~~~~~~~~~~
	In function ‘f’,
	    inlined from ‘main’ at array-bounds.c:24:2:
	array-bounds.c:6:16: warning: array subscript 100 is outside array bounds of ‘int[2]’ [-Warray-bounds=]
	    6 |         a[100] = 7;
	      |         ~~~~~~~^~~
	array-bounds.c: In function ‘main’:
	array-bounds.c:22:13: note: at offset 400 into object ‘a’ of size 8
	   22 |         int a[2];
	      |             ^
	In function ‘fs’,
	    inlined from ‘main’ at array-bounds.c:25:2:
	array-bounds.c:15:16: warning: array subscript 101 is outside array bounds of ‘int[2]’ [-Warray-bounds=]
	   15 |         a[101] = 8;
	      |         ~~~~~~~^~~
	array-bounds.c: In function ‘main’:
	array-bounds.c:22:13: note: at offset 404 into object ‘a’ of size 8
	   22 |         int a[2];
	      |             ^

Cheers,
Alex

> 
> 	alx@devuan:~/tmp$ cat array-bounds.c 
> 	void g(int a[43]);
> 
> 	void f(int a[42]);
> 	void f(int a[42])
> 	{
> 		a[100] = 7;
> 		g(a);
> 	}
> 
> 	void gs(int a[static 43]);
> 
> 	void fs(int a[static 42]);
> 	void fs(int a[static 42])
> 	{
> 		a[101] = 8;
> 		gs(a);
> 	}
> 	alx@devuan:~/tmp$ clang -S -Weverything array-bounds.c 
> 	array-bounds.c:6:2: warning: unsafe buffer access [-Wunsafe-buffer-usage]
> 	    6 |         a[100] = 7;
> 	      |         ^
> 	array-bounds.c:15:2: warning: unsafe buffer access [-Wunsafe-buffer-usage]
> 	   15 |         a[101] = 8;
> 	      |         ^
> 	2 warnings generated.
> 	alx@devuan:~/tmp$ clang-21 -S -Weverything array-bounds.c 
> 	array-bounds.c:6:2: warning: unsafe buffer access
> 	      [-Wunsafe-buffer-usage]
> 	    6 |         a[100] = 7;
> 	      |         ^
> 	array-bounds.c:15:2: warning: unsafe buffer access
> 	      [-Wunsafe-buffer-usage]
> 	   15 |         a[101] = 8;
> 	      |         ^
> 	2 warnings generated.
> 
> > and
> > then push to get the C standard revised to specify that behavior.
> 
> I'm working on convincing the C Committee regarding this at the moment.
> In the meantime, we have GCC available.
> 
> >  It
> > sounds great to me, but that would of course be a very long project.
> 
> Not so much.  GCC is here already.  And the C Committee could be
> convinced.
> 
> > In the mean time, we don't need to delay using the tool we have now.
> 
> Are you sure [static n] is a tool?  I don't see any compiler diagnosing
> it differently from [n].  Neither GCC nor Clang diagnose it, from what
> I've shown above.
> 
> > > On the other hand, to resolve the issue at hand, how about an
> > > alternative approach?
> > > 
> > > void xchacha20poly1305_encrypt(u8 *dst, const u8 *src, const size_t src_len,
> > >                                const u8 *ad, const size_t ad_len,
> > >                                const u8 nonce[XCHACHA20POLY1305_NONCE_SIZE],
> > >                                const u8 key[CHACHA20POLY1305_KEY_SIZE]);
> > > 
> > > #define xchacha20poly1305_encrypt_arr(dst, src, slen, ad, ad_len, nonce, k)\
> > > ({                                                                    \
> > > 	static_assert(ARRAY_SIZE(nonce) == XCHACHA20POLY1305_NONCE_SIZE);\
> > > 	static_assert(ARRAY_SIZE(key) == CHACHA20POLY1305_KEY_SIZE);  \
> > > 	xchacha20poly1305_encrypt(dst, src, slen, ad, ad_len, nonce, k);\
> > > })
> > 
> > No.  That would be more code, would double the API size, and make it the
> > caller's responsibility to decide which one to call.
> 
> You could name the function as __xchacha20poly1305_dont_call_me_directly()
> to make sure it's never called.
> 
> >  And often there
> > won't be a correct option, as the caller may have arrays that are larger
> > than the required size, or a mix of arrays and pointers, etc.
> 
> If you want to enforce a minimum array size, you could change the static
> assertions to do >= instead.
> 
> You could also lift the check when the input is a pointer:
> 
> 	static_assert(!is_array(nonce)
> 	              || sizeof(nonce) >= XCHACHA20POLY1305_NONCE_SIZE);
> 
> It all depends on what you need exactly.
> 
> 
> Have a lovely day!
> Alex
> 
> > 
> > - Eric
> 
> -- 
> <https://www.alejandro-colomar.es>



-- 
<https://www.alejandro-colomar.es>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [RFC PATCH] libcrypto/chachapoly: Use strict typing for fixed size array arguments

[Eric Biggers]

On Tue, Dec 02, 2025 at 02:42:13PM +0100, Alejandro Colomar wrote:
> > Clang doesn't enforce it.  Clang doesn't even enforce [static n].
> > Clang is in fact quite bad at diagnosing array bounds violations.
> > Aaron, could you please fix this in Clang?
> 
> Self-correction: Actually, there's different enforcement in one case:

Right, and it's the case that actually matters, which is kind of the
point.  Maybe you missed the earlier discussions.

- Eric

Re: [RFC PATCH] libcrypto/chachapoly: Use strict typing for fixed size array arguments

[Alejandro Colomar]

[-- Attachment #1: Type: text/plain, Size: 1854 bytes --]

Hi Eric,

On Tue, Dec 02, 2025 at 04:49:49PM +0000, Eric Biggers wrote:
> On Tue, Dec 02, 2025 at 02:42:13PM +0100, Alejandro Colomar wrote:
> > > Clang doesn't enforce it.  Clang doesn't even enforce [static n].
> > > Clang is in fact quite bad at diagnosing array bounds violations.
> > > Aaron, could you please fix this in Clang?
> > 
> > Self-correction: Actually, there's different enforcement in one case:
> 
> Right, and it's the case that actually matters, which is kind of the
> point.  Maybe you missed the earlier discussions.

Yup, I only noticed about this because of LWN.

So, I've asked GCC to remove dubious diagnostics from
-Wstringop-overflow so that it doesn't need to be disabled by anyone.
<https://gcc.gnu.org/bugzilla/show_bug.cgi?id=122963>

I'm not entirely sure why the kernel can't enable it, but from what
I saw it seems it has something to do with some libc function.  That
could go to a separate -W flag, and then you could re-enable it.

However, please report a minimal reproducer that shows the false
positives you have.  That would be useful.

It would also be useful if (plural) you pushed Clang to diagnose [n]
just as [static n].  It makes sense to use [static n] temporarily
because that's what Clang diagnoses today, but it would be better if
Clang diagnosed the code without changing it.  I can try to ask them,
but if kernel maintainers ask for it, it will have more effect.

In my work convincing the C Committee of strengthening [n] semantics,
the hardest part is convincing members of the committee from Clang.
If Clang had this, we'd likely pass it in the committee.  They've told
me they're opposed to doing this work without clients requesting it, and
you'd be one such client.


Have a lovely night!
Alex

> 
> - Eric

-- 
<https://www.alejandro-colomar.es>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [RFC PATCH] libcrypto/chachapoly: Use strict typing for fixed size array arguments

[Daniel Thompson]

On Tue, Dec 02, 2025 at 02:12:47AM +0100, Alejandro Colomar wrote:
> Hi,
>
> Jason said:
> > On Sat, Nov 15, 2025 at 09:11:31AM -0800, Linus Torvalds wrote:
> > > So *if* we end up using this syntax more widely, I suspect we'd want
> > > to have a macro that makes the semantics more obvious, even if it's
> > > something silly and trivial like
> > >
> > >    #define min_array_size(n) static n
> > >
> > > just so that people who aren't familiar with that crazy syntax
> > > understand what it means.
> >
> > Oh that's a good suggestion. I'll see if I can rig that up and send
> > something.
>
> Be careful about [static n].  It has implications that you're probably
> not aware of.  Also, it doesn't have some implications you might expect
> from it.
>
> -  [static n] on an argument implies __attribute__((nonnull())) on that
>    argument; it means that the argument can't be null.  You may want to
>    make sure you're using -fno-delete-null-pointer-checks if you use
>    [static n].
>
> -  [static n] implies that n>0.  You should make sure that n>0, or UB
>    would be triggered.
>
> -  [n] means two promises traditionally:
>    -  The caller will provide at least n elements.
>    -  The callee will use no more than n elements.
>    However, [static n] only carries the first promise.  According to
>    ISO C, the callee may access elements beyond that.

This description implies that [n] carries promises that [static n] does
not. However you are comparing the "traditional" behaviour (that is
well beyond the scope of the standard) on one side with ISO C behaviour
on the other.

It makes sense to compare ISO C behavior for [n] (where neither of the
above promises applies) with ISO C behaviour for [static n]...


>    GCC, as a quality implementation, enforces the second promise too,
>    but this is not portable; you should make sure that all supported
>    compilers enforces that as an extension.

... and equally it makes sense to compare the gcc/clang warnings for
[n] versus [static n] as recommended here.

However it should not be motivated by [static n] carrying few promises
than [n], especially given gcc/clang's enforcement of the promises is
a best effort static check that won't cover all cases anyway.


Daniel.

Re: [RFC PATCH] libcrypto/chachapoly: Use strict typing for fixed size array arguments

[Alejandro Colomar]

[-- Attachment #1: Type: text/plain, Size: 2951 bytes --]

Hi Daniel,

On Wed, Dec 03, 2025 at 05:24:06PM +0000, Daniel Thompson wrote:
[...]
> > Be careful about [static n].  It has implications that you're probably
> > not aware of.  Also, it doesn't have some implications you might expect
> > from it.
> >
> > -  [static n] on an argument implies __attribute__((nonnull())) on that
> >    argument; it means that the argument can't be null.  You may want to
> >    make sure you're using -fno-delete-null-pointer-checks if you use
> >    [static n].
> >
> > -  [static n] implies that n>0.  You should make sure that n>0, or UB
> >    would be triggered.
> >
> > -  [n] means two promises traditionally:
> >    -  The caller will provide at least n elements.
> >    -  The callee will use no more than n elements.
> >    However, [static n] only carries the first promise.  According to
> >    ISO C, the callee may access elements beyond that.
> 
> This description implies that [n] carries promises that [static n] does
> not. However you are comparing the "traditional" behaviour (that is
> well beyond the scope of the standard) on one side with ISO C behaviour
> on the other.
> 
> It makes sense to compare ISO C behavior for [n] (where neither of the
> above promises applies) with ISO C behaviour for [static n]...

Clang seems to implement the ISO C behavior, so in retrospective, the
comparison I did seems appropriate.  By moving from the GCC/traditional
behavior to the Clang/ISO one, a project would be lowering quality.

Plus, there's still the issues about n>0 and nonnullness.

The only reason why it makes some sense to use [static n] in the kernel
is because it's moving from no-rules to some rules.  But the real
problem here is that the kernel needs to turn GCC's -Wstringop-overflow
off, and that's what the kernel do some effort to re-enable.

If you show a minimal reproducer of what the problem is with
-Wstringop-overflow, I may be able to help with that.

In general, [static n] is bogus and never to be used, except temporarily
while other issues get fixed, like in this case.

> >    GCC, as a quality implementation, enforces the second promise too,
> >    but this is not portable; you should make sure that all supported
> >    compilers enforces that as an extension.
> 
> ... and equally it makes sense to compare the gcc/clang warnings for
> [n] versus [static n] as recommended here.

Clang is really bad at both [n] and [static n].  If you need to rely on
clang for array bounds, you're screwed.

> However it should not be motivated by [static n] carrying few promises
> than [n], especially given gcc/clang's enforcement of the promises is
> a best effort static check that won't cover all cases anyway.

GCC is quite good at those diagnostics; it might not cover all cases,
but that's better than what ISO or Clang will promise.


Have a lovely night!
Alex

-- 
<https://www.alejandro-colomar.es>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [RFC PATCH] libcrypto/chachapoly: Use strict typing for fixed size array arguments

[Daniel Thompson]

On Wed, Dec 03, 2025 at 07:01:39PM +0100, Alejandro Colomar wrote:
> Hi Daniel,
>
> On Wed, Dec 03, 2025 at 05:24:06PM +0000, Daniel Thompson wrote:
> [...]
> > > Be careful about [static n].  It has implications that you're probably
> > > not aware of.  Also, it doesn't have some implications you might expect
> > > from it.
> > >
> > > -  [static n] on an argument implies __attribute__((nonnull())) on that
> > >    argument; it means that the argument can't be null.  You may want to
> > >    make sure you're using -fno-delete-null-pointer-checks if you use
> > >    [static n].
> > >
> > > -  [static n] implies that n>0.  You should make sure that n>0, or UB
> > >    would be triggered.
> > >
> > > -  [n] means two promises traditionally:
> > >    -  The caller will provide at least n elements.
> > >    -  The callee will use no more than n elements.
> > >    However, [static n] only carries the first promise.  According to
> > >    ISO C, the callee may access elements beyond that.
> >
> > This description implies that [n] carries promises that [static n] does
> > not. However you are comparing the "traditional" behaviour (that is
> > well beyond the scope of the standard) on one side with ISO C behaviour
> > on the other.
> >
> > It makes sense to compare ISO C behavior for [n] (where neither of the
> > above promises applies) with ISO C behaviour for [static n]...
>
> Clang seems to implement the ISO C behavior, so in retrospective, the
> comparison I did seems appropriate.  By moving from the GCC/traditional
> behavior to the Clang/ISO one, a project would be lowering quality.

I'm still rather dubious about confusing the optimization opportunities
afforded by the ISO C standard with the diagnostic messages that both
gcc and clang can produce (and are beyond the scope of the standard).

However, let's agree to disagree on that, since it doesn't change the
outcome: it *is* useful to compare the behaviour of the two compilers.


> Plus, there's still the issues about n>0 and nonnullness.
>
> The only reason why it makes some sense to use [static n] in the kernel
> is because it's moving from no-rules to some rules.  But the real
> problem here is that the kernel needs to turn GCC's -Wstringop-overflow
> off, and that's what the kernel do some effort to re-enable.
>
> If you show a minimal reproducer of what the problem is with
> -Wstringop-overflow, I may be able to help with that.

I'm not 100% caught up on the history but I think this was the issue
that prompted -Wstringop-overflow to be disabled by default (and
includes a minimal reproducer):
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=113214


> In general, [static n] is bogus and never to be used, except temporarily
> while other issues get fixed, like in this case.
>
> > >    GCC, as a quality implementation, enforces the second promise too,
> > >    but this is not portable; you should make sure that all supported
> > >    compilers enforces that as an extension.
> >
> > ... and equally it makes sense to compare the gcc/clang warnings for
> > [n] versus [static n] as recommended here.
>
> Clang is really bad at both [n] and [static n].  If you need to rely on
> clang for array bounds, you're screwed.

What do you mean by clang is really bad at [static n]? Most of this
thread is based on the observation that clang gives *useful*
diagnostics for [static n] that are not issued for [n].


> > However it should not be motivated by [static n] carrying few promises
> > than [n], especially given gcc/clang's enforcement of the promises is
> > a best effort static check that won't cover all cases anyway.
>
> GCC is quite good at those diagnostics; it might not cover all cases,
> but that's better than what ISO or Clang will promise.

gcc certainly can generate warnings we don't get with clang, so it's
just a question of whether the false positives are enough to stop it
from being quite good!

I was curious is anything has changed in the last two years so I
compiled v6.18 allmodconfig with -Wstringop-overflow (without the
thread sanitizer which causes the known problem mentioned above
right the way up to gcc-15.2). I ran check across five architectures
(arm64, arm, riscv, s390 & x86) since we know there have been
architecture dependant differences. Not all the builds have gone
through but unless there are regressions in newer compilers then
I've only seen two -Wstringop-overflow warnings.

First is a clear false positive (of the "if something impossible
happens then something bad might happen" variety) which, happily, is
only triggered on gcc-12/aarch64 and appears to be fixed from gcc-13
onward.

Second isn't a kernel bug but it's arguably not a false positive
either. I think it could be reasonably fixed with source code changes.
See https://elixir.bootlin.com/linux/v6.18/source/net/sctp/auth.c#L645

Overall I'll look a little deeper to try and see if there are any
other ways to break -Werror builds with gcc-13 or later.


Daniel.

Re: [RFC PATCH] libcrypto/chachapoly: Use strict typing for fixed size array arguments

[Alejandro Colomar]

[-- Attachment #1: Type: text/plain, Size: 6615 bytes --]

Hi Daniel,

On Fri, Dec 05, 2025 at 01:59:26PM +0000, Daniel Thompson wrote:
> On Wed, Dec 03, 2025 at 07:01:39PM +0100, Alejandro Colomar wrote:
> > Hi Daniel,
> >
> > On Wed, Dec 03, 2025 at 05:24:06PM +0000, Daniel Thompson wrote:
> > [...]
> > > > Be careful about [static n].  It has implications that you're probably
> > > > not aware of.  Also, it doesn't have some implications you might expect
> > > > from it.
> > > >
> > > > -  [static n] on an argument implies __attribute__((nonnull())) on that
> > > >    argument; it means that the argument can't be null.  You may want to
> > > >    make sure you're using -fno-delete-null-pointer-checks if you use
> > > >    [static n].
> > > >
> > > > -  [static n] implies that n>0.  You should make sure that n>0, or UB
> > > >    would be triggered.
> > > >
> > > > -  [n] means two promises traditionally:
> > > >    -  The caller will provide at least n elements.
> > > >    -  The callee will use no more than n elements.
> > > >    However, [static n] only carries the first promise.  According to
> > > >    ISO C, the callee may access elements beyond that.
> > >
> > > This description implies that [n] carries promises that [static n] does
> > > not. However you are comparing the "traditional" behaviour (that is
> > > well beyond the scope of the standard) on one side with ISO C behaviour
> > > on the other.
> > >
> > > It makes sense to compare ISO C behavior for [n] (where neither of the
> > > above promises applies) with ISO C behaviour for [static n]...
> >
> > Clang seems to implement the ISO C behavior, so in retrospective, the
> > comparison I did seems appropriate.  By moving from the GCC/traditional
> > behavior to the Clang/ISO one, a project would be lowering quality.
> 
> I'm still rather dubious about confusing the optimization opportunities
> afforded by the ISO C standard with the diagnostic messages that both
> gcc and clang can produce (and are beyond the scope of the standard).

I think the ISO C standard should not support any syntax that allows
optimizations without diagnostics.  Those are huge footguns.

We already have experience in GCC with __attribute__((__nonnull__())).
A better would look like 'const', and should diagnose any violations at
compile time.

Then, ISO C added 'restrict', which is another case of optimizations
without diagnostics.  See also:
<https://gcc.gnu.org/bugzilla/show_bug.cgi?id=112833>.

And [static n] is essentially the same kind of bad stuff.  In fact, it
implies __attribute__((__nonnull__())), so that should tell enough.

At least, the GCC attribute is not part of the type system.  It's bad,
but it doesn't claim to be good.  But having [static n] as if it were
part of the type system, but then using it for optimizations, that's
just evil.

> However, let's agree to disagree on that, since it doesn't change the
> outcome: it *is* useful to compare the behaviour of the two compilers.

Okay.

> > Plus, there's still the issues about n>0 and nonnullness.
> >
> > The only reason why it makes some sense to use [static n] in the kernel
> > is because it's moving from no-rules to some rules.  But the real
> > problem here is that the kernel needs to turn GCC's -Wstringop-overflow
> > off, and that's what the kernel do some effort to re-enable.
> >
> > If you show a minimal reproducer of what the problem is with
> > -Wstringop-overflow, I may be able to help with that.
> 
> I'm not 100% caught up on the history but I think this was the issue
> that prompted -Wstringop-overflow to be disabled by default (and
> includes a minimal reproducer):
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=113214

Ahhh, sanitizers.  Yeah, sanitizers are usually bad for diagnostics.

> > In general, [static n] is bogus and never to be used, except temporarily
> > while other issues get fixed, like in this case.
> >
> > > >    GCC, as a quality implementation, enforces the second promise too,
> > > >    but this is not portable; you should make sure that all supported
> > > >    compilers enforces that as an extension.
> > >
> > > ... and equally it makes sense to compare the gcc/clang warnings for
> > > [n] versus [static n] as recommended here.
> >
> > Clang is really bad at both [n] and [static n].  If you need to rely on
> > clang for array bounds, you're screwed.
> 
> What do you mean by clang is really bad at [static n]?

GCC gives more diagnostics for [static n] than Clang.

> Most of this
> thread is based on the observation that clang gives *useful*
> diagnostics for [static n] that are not issued for [n].

Clang is even worse at [n] than at [static n], but it's bad at both.

> > > However it should not be motivated by [static n] carrying few promises
> > > than [n], especially given gcc/clang's enforcement of the promises is
> > > a best effort static check that won't cover all cases anyway.
> >
> > GCC is quite good at those diagnostics; it might not cover all cases,
> > but that's better than what ISO or Clang will promise.
> 
> gcc certainly can generate warnings we don't get with clang, so it's
> just a question of whether the false positives are enough to stop it
> from being quite good!
> 
> I was curious is anything has changed in the last two years so I
> compiled v6.18 allmodconfig with -Wstringop-overflow (without the
> thread sanitizer which causes the known problem mentioned above
> right the way up to gcc-15.2). I ran check across five architectures
> (arm64, arm, riscv, s390 & x86) since we know there have been
> architecture dependant differences. Not all the builds have gone
> through but unless there are regressions in newer compilers then
> I've only seen two -Wstringop-overflow warnings.
> 
> First is a clear false positive (of the "if something impossible
> happens then something bad might happen" variety) which, happily, is
> only triggered on gcc-12/aarch64 and appears to be fixed from gcc-13
> onward.
> 
> Second isn't a kernel bug but it's arguably not a false positive
> either. I think it could be reasonably fixed with source code changes.
> See https://elixir.bootlin.com/linux/v6.18/source/net/sctp/auth.c#L645
> 
> Overall I'll look a little deeper to try and see if there are any
> other ways to break -Werror builds with gcc-13 or later.

Thanks for researching this!  That would be very useful!  Please CC me
if you write any patches about this; I'm interested.


Have a lovely night!
Alex

> 
> 
> Daniel.

-- 
<https://www.alejandro-colomar.es>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]