Re: [PATCH libcrypto 2/2] crypto: chacha20poly1305: statically check fixed array lengths

Re: [PATCH libcrypto 2/2] crypto: chacha20poly1305: statically check fixed array lengths

[kernel test robot]

Hi Jason,

kernel test robot noticed the following build warnings:

[auto build test WARNING on herbert-cryptodev-2.6/master]
[also build test WARNING on herbert-crypto-2.6/master ebiggers/libcrypto-next ebiggers/libcrypto-fixes linus/master linux/master v6.18-rc6 next-20251119]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Jason-A-Donenfeld/crypto-chacha20poly1305-statically-check-fixed-array-lengths/20251119-011125
base:   https://git.kernel.org/pub/scm/linux/kernel/git/herbert/cryptodev-2.6.git master
patch link:    https://lore.kernel.org/r/20251118170240.689299-2-Jason%40zx2c4.com
patch subject: [PATCH libcrypto 2/2] crypto: chacha20poly1305: statically check fixed array lengths
config: loongarch-allmodconfig (https://download.01.org/0day-ci/archive/20251119/202511192000.TLYrcg0Z-lkp@intel.com/config)
compiler: clang version 19.1.7 (https://github.com/llvm/llvm-project cd708029e0b2869e80abe31ddb175f7c35361f90)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20251119/202511192000.TLYrcg0Z-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202511192000.TLYrcg0Z-lkp@intel.com/

All warnings (new ones prefixed by >>):

>> drivers/net/wireguard/cookie.c:193:2: warning: array argument is too small; contains 31 elements, callee requires at least 32 [-Warray-bounds]
     193 |         xchacha20poly1305_encrypt(dst->encrypted_cookie, cookie, COOKIE_LEN,
         |         ^
     194 |                                   macs->mac1, COOKIE_LEN, dst->nonce,
     195 |                                   checker->cookie_encryption_key);
         |                                   ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   include/crypto/chacha20poly1305.h:32:20: note: callee declares array parameter as static here
      32 |                                const u8 key[min_array_size(CHACHA20POLY1305_KEY_SIZE)]);
         |                                         ^  ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   In file included from drivers/net/wireguard/cookie.c:6:
   In file included from drivers/net/wireguard/cookie.h:9:
   In file included from drivers/net/wireguard/messages.h:10:
   In file included from include/crypto/chacha20poly1305.h:11:
   In file included from include/linux/scatterlist.h:5:
   In file included from include/linux/string.h:382:
   include/linux/fortify-string.h:480:4: warning: call to '__write_overflow_field' declared with 'warning' attribute: detected write beyond size of field (1st parameter); maybe use struct_group()? [-Wattribute-warning]
     480 |                         __write_overflow_field(p_size_field, size);
         |                         ^
   2 warnings generated.


vim +193 drivers/net/wireguard/cookie.c

e7096c131e5161 Jason A. Donenfeld 2019-12-09  179  
e7096c131e5161 Jason A. Donenfeld 2019-12-09  180  void wg_cookie_message_create(struct message_handshake_cookie *dst,
e7096c131e5161 Jason A. Donenfeld 2019-12-09  181  			      struct sk_buff *skb, __le32 index,
e7096c131e5161 Jason A. Donenfeld 2019-12-09  182  			      struct cookie_checker *checker)
e7096c131e5161 Jason A. Donenfeld 2019-12-09  183  {
e7096c131e5161 Jason A. Donenfeld 2019-12-09  184  	struct message_macs *macs = (struct message_macs *)
e7096c131e5161 Jason A. Donenfeld 2019-12-09  185  		((u8 *)skb->data + skb->len - sizeof(*macs));
e7096c131e5161 Jason A. Donenfeld 2019-12-09  186  	u8 cookie[COOKIE_LEN];
e7096c131e5161 Jason A. Donenfeld 2019-12-09  187  
e7096c131e5161 Jason A. Donenfeld 2019-12-09  188  	dst->header.type = cpu_to_le32(MESSAGE_HANDSHAKE_COOKIE);
e7096c131e5161 Jason A. Donenfeld 2019-12-09  189  	dst->receiver_index = index;
e7096c131e5161 Jason A. Donenfeld 2019-12-09  190  	get_random_bytes_wait(dst->nonce, COOKIE_NONCE_LEN);
e7096c131e5161 Jason A. Donenfeld 2019-12-09  191  
e7096c131e5161 Jason A. Donenfeld 2019-12-09  192  	make_cookie(cookie, skb, checker);
e7096c131e5161 Jason A. Donenfeld 2019-12-09 @193  	xchacha20poly1305_encrypt(dst->encrypted_cookie, cookie, COOKIE_LEN,
e7096c131e5161 Jason A. Donenfeld 2019-12-09  194  				  macs->mac1, COOKIE_LEN, dst->nonce,
e7096c131e5161 Jason A. Donenfeld 2019-12-09  195  				  checker->cookie_encryption_key);
e7096c131e5161 Jason A. Donenfeld 2019-12-09  196  }
e7096c131e5161 Jason A. Donenfeld 2019-12-09  197  

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH libcrypto 2/2] crypto: chacha20poly1305: statically check fixed array lengths

[Linus Torvalds]

 On Wed, 19 Nov 2025 at 04:46, kernel test robot <lkp@intel.com> wrote:
>
> >> drivers/net/wireguard/cookie.c:193:2: warning: array argument is too small; contains 31 elements, callee requires at least 32 [-Warray-bounds]

Hmm. Is this a compiler bug?

That checker->cookie_encryption_key is declared as

        u8 cookie_encryption_key[NOISE_SYMMETRIC_KEY_LEN];

and NOISE_SYMMETRIC_KEY_LEN is an enum that is defined to be the same
as CHACHA20POLY1305_KEY_SIZE, which is 32.

And the compiler is aware of that:

>    include/crypto/chacha20poly1305.h:32:20: note: callee declares array parameter as static here
>       32 |                                const u8 key[min_array_size(CHACHA20POLY1305_KEY_SIZE)]);
>          |                                         ^  ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

but still talks about "contains 31 elements".

Is this some confusion with the compiler thinking that a "const u8[]"
is a string, and then at some point subtracted one as the max length,
and then is confused due to that?

Because if compilers screw this up, we can't do that 'static' thing,
regardless of name. I'm  not willing to play silly buggers with broken
compiler warnings.

               Linus

Re: [PATCH libcrypto 2/2] crypto: chacha20poly1305: statically check fixed array lengths

[Jason A. Donenfeld]

Hey Linus,

On Wed, Nov 19, 2025 at 5:29 PM Linus Torvalds
<torvalds@linux-foundation.org> wrote:
>
>  On Wed, 19 Nov 2025 at 04:46, kernel test robot <lkp@intel.com> wrote:
> >
> > >> drivers/net/wireguard/cookie.c:193:2: warning: array argument is too small; contains 31 elements, callee requires at least 32 [-Warray-bounds]
>
> Hmm. Is this a compiler bug?

It's not. It's a 0day test bot bug! My original patch had in it some
commentary about what a bug would look like when it's caught by the
compiler. In order to provoke that compiler output, I mentioned in the
commit message that this diff will produce such and such result:

diff --git a/drivers/net/wireguard/cookie.h b/drivers/net/wireguard/cookie.h
index c4bd61ca03f2..2839c46029f8 100644
--- a/drivers/net/wireguard/cookie.h
+++ b/drivers/net/wireguard/cookie.h
@@ -13,7 +13,7 @@ struct wg_peer;

 struct cookie_checker {
        u8 secret[NOISE_HASH_LEN];
-       u8 cookie_encryption_key[NOISE_SYMMETRIC_KEY_LEN];
+       u8 cookie_encryption_key[NOISE_SYMMETRIC_KEY_LEN - 1];
        u8 message_mac1_key[NOISE_SYMMETRIC_KEY_LEN];
        u64 secret_birthdate;
        struct rw_semaphore secret_lock;

It looks like the 0day test bot just went through the email and
applied all the `diff --git ...` hunks, without taking into account
the context area above where the actual patches start.

So, if anything, this test bot output is showing that the compiler
feature works as intended.

Jason

Re: [PATCH libcrypto 2/2] crypto: chacha20poly1305: statically check fixed array lengths

[Linus Torvalds]

On Wed, 19 Nov 2025 at 08:47, Jason A. Donenfeld <Jason@zx2c4.com> wrote:
>
> > Hmm. Is this a compiler bug?
>
> It's not. It's a 0day test bot bug! My original patch had in it some
> commentary about what a bug would look like when it's caught by the
> compiler. In order to provoke that compiler output, I mentioned in the
> commit message that this diff will produce such and such result:

Lol, ok.

I sometimes actively just whitespace-damage my "what about this"
patches to make sure people don't just blindly apply them without
thinking about them and actually look at them.

Maybe that's a good policy in general, so that the bots don't do the same ;)

            Linus

Re: [PATCH libcrypto 2/2] crypto: chacha20poly1305: statically check fixed array lengths

[Nathan Chancellor]

On Wed, Nov 19, 2025 at 05:46:44PM +0100, Jason A. Donenfeld wrote:
> Hey Linus,
> 
> On Wed, Nov 19, 2025 at 5:29 PM Linus Torvalds
> <torvalds@linux-foundation.org> wrote:
> >
> >  On Wed, 19 Nov 2025 at 04:46, kernel test robot <lkp@intel.com> wrote:
> > >
> > > >> drivers/net/wireguard/cookie.c:193:2: warning: array argument is too small; contains 31 elements, callee requires at least 32 [-Warray-bounds]
> >
> > Hmm. Is this a compiler bug?
> 
> It's not. It's a 0day test bot bug! My original patch had in it some
> commentary about what a bug would look like when it's caught by the
> compiler. In order to provoke that compiler output, I mentioned in the
> commit message that this diff will produce such and such result:
> 
> diff --git a/drivers/net/wireguard/cookie.h b/drivers/net/wireguard/cookie.h
> index c4bd61ca03f2..2839c46029f8 100644
> --- a/drivers/net/wireguard/cookie.h
> +++ b/drivers/net/wireguard/cookie.h
> @@ -13,7 +13,7 @@ struct wg_peer;
> 
>  struct cookie_checker {
>         u8 secret[NOISE_HASH_LEN];
> -       u8 cookie_encryption_key[NOISE_SYMMETRIC_KEY_LEN];
> +       u8 cookie_encryption_key[NOISE_SYMMETRIC_KEY_LEN - 1];
>         u8 message_mac1_key[NOISE_SYMMETRIC_KEY_LEN];
>         u64 secret_birthdate;
>         struct rw_semaphore secret_lock;
> 
> It looks like the 0day test bot just went through the email and
> applied all the `diff --git ...` hunks, without taking into account
> the context area above where the actual patches start.

I don't think it is a bot issue. Just running 'b4 shazam' (i.e. just
applying the patch with 'git am') on the series results in:

commit 6ddc87109d4bb589d02cc3a8b037c99fdc4cbbf9
Author: Jason A. Donenfeld <Jason@zx2c4.com>
Date:   Tue Nov 18 18:02:40 2025 +0100

    crypto: chacha20poly1305: statically check fixed array lengths
    
    Several parameters of the chacha20poly1305 functions require arrays of
    an exact length. Use the new min_array_size() macro to instruct gcc and
    clang to statically check that the caller is passing an object of at
    least that length.
    
    Here it is in action, with this faulty patch:

diff --git a/drivers/net/wireguard/cookie.h b/drivers/net/wireguard/cookie.h
index c4bd61ca03f2..2839c46029f8 100644
--- a/drivers/net/wireguard/cookie.h
+++ b/drivers/net/wireguard/cookie.h
@@ -13,7 +13,7 @@ struct wg_peer;
 
 struct cookie_checker {
 	u8 secret[NOISE_HASH_LEN];
-	u8 cookie_encryption_key[NOISE_SYMMETRIC_KEY_LEN];
+	u8 cookie_encryption_key[NOISE_SYMMETRIC_KEY_LEN - 1];
 	u8 message_mac1_key[NOISE_SYMMETRIC_KEY_LEN];
 	u64 secret_birthdate;
 	struct rw_semaphore secret_lock;

followed by the rest of the patch for me.

Cheers,
Nathan