Re: [PATCH][4.15] crypto: adjust rijndaelEncrypt() prototype for gcc11

[Julien Grall <julien@xxxxxxx>]

Hi Jan,

On 01/03/2021 07:57, Jan Beulich wrote:
> The upcoming release complains, not entirely unreasonably:
>
> In file included from rijndael.c:33:
> .../xen/include/crypto/rijndael.h:55:53: note: previously declared as 'const 
> unsigned char[]'
>     55 | void    rijndaelEncrypt(const unsigned int [], int, const unsigned 
> char [],
>        |                                                     
> ^~~~~~~~~~~~~~~~~~~~~~
> rijndael.c:865:8: error: argument 4 of type 'u8[16]' {aka 'unsigned char[16]'} 
> with mismatched bound [-Werror=array-parameter=]
>    865 |     u8 ct[16])
>        |     ~~~^~~~~~
> In file included from rijndael.c:33:
> .../xen/include/crypto/rijndael.h:56:13: note: previously declared as 'unsigned 
> char[]'
>     56 |             unsigned char []);
>        |             ^~~~~~~~~~~~~~~~
>
> While it's not really clear to me why it would complain only for arg 4,
> the adjustment to make is obvious and riskfree also for arg 3: Simply
> declare the correct array dimension right away. This then allows
> compilers to apply checking at call sites, which seems desirable anyway.
I am a bit confused, if GCC is not complaining for arg3, then what is 
the following error message for:
> In file included from rijndael.c:33:
> .../xen/include/crypto/rijndael.h:55:53: note: previously declared as 
'const unsigned char[]'
>     55 | void    rijndaelEncrypt(const unsigned int [], int, const 
unsigned char [],
>        | 
^~~~~~~~~~~~~~~~~~~~~~
> For the moment I'm leaving untouched the disagreement between u8/u32
> used in the function definition and unsigned {char,int} used in the
> declaration, as making this consistent would call for touching further
> functions.
>
> Reported-by: Charles Arnold <carnold@xxxxxxxx>
> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
> ---
> There are quite a few more issues with gcc11, but from my brief initial
> inspection I'm suspecting (hoping) it'll rather be the compiler which
> will get further changed by the time their release gets finalized. Just
> one example:
>
> .../xen/include/xen/string.h:101:27: error: '__builtin_memcmp' specified bound 
> 4 exceeds source size 0 [-Werror=stringop-overread]
>    101 | #define memcmp(s1, s2, n) __builtin_memcmp(s1, s2, n)
>        |                           ^~~~~~~~~~~~~~~~~~~~~~~~~~~
> mpparse.c:722:13: note: in expansion of macro 'memcmp'
>    722 |         if (memcmp(mpf->mpf_signature, "_MP_", 4) == 0 &&
>        |             ^~~~~~
>
> Clearly neither the 1st nor the 2nd argument have a "source size" of 0.
It looks like there is a report on the redhat bug tracker for it [1]. Do 
you know if there is a bug report on the GCC tracker as well?
> --- a/xen/include/crypto/rijndael.h
> +++ b/xen/include/crypto/rijndael.h
> @@ -52,7 +52,7 @@
>   
>   int	rijndaelKeySetupEnc(unsigned int [], const unsigned char [], int);
>   int   rijndaelKeySetupDec(unsigned int [], const unsigned char [], int);
> -void   rijndaelEncrypt(const unsigned int [], int, const unsigned char [],
> -           unsigned char []);
> +void   rijndaelEncrypt(const unsigned int [], int, const unsigned char [16],
> +           unsigned char [16]);
>   
>   #endif /* __RIJNDAEL_H */
Cheers,

[1] https://bugzilla.redhat.com/show_bug.cgi?id=1892100

--
Julien Grall