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

[Ard Biesheuvel <ardb@kernel.org>]

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)