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

[Eric Biggers <ebiggers@kernel.org>]

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