Uninteded Variable Length Array in ec-nist.c

[Jussi Kivilinna]

Hello,

On 27.9.2022 3.54, NIIBE Yutaka wrote:
> Hello,
> 
> While we allow use of some features of C99 for libgcrypt, we don't use
> variable length array in the code.
> 
> Thus, I'm considering adding -Wvla option in configure.ac.  In master,
> I found use of variable length array in ec-nist.c.  I attach the
> warning message of compiler when it is compiled with -Wvla option.
> 
> I think that it's not intended.  Unfortunately, even if we added const
> qualifier to the variable 'wsize', (because what is needed here is a
> constant expression), it is still considered as variable length array by
> compilers.
> 
> I think that use of macro for the size is needed here, although it would
> not look modern code.

How about instead define arrays with wanted size and define 'wsize' with
sizeof the array. This would avoid having macros. For example like this:

index 69b05a6d..0de41e48 100644
--- a/mpi/ec-nist.c
+++ b/mpi/ec-nist.c
@@ -94,9 +94,9 @@ _gcry_mpi_ec_nist192_mod (gcry_mpi_t w, mpi_ec_t ctx)
    };
    const mpi_limb64_t zero = LIMB_TO64(0);
    mpi_ptr_t wp;
-  mpi_size_t wsize = 192 / BITS_PER_MPI_LIMB64;
-  mpi_limb64_t s[wsize + 1];
-  mpi_limb64_t o[wsize + 1];
+  mpi_limb64_t s[192 / BITS_PER_MPI_LIMB64 + 1];
+  mpi_limb64_t o[sizeof(s)];
+  const mpi_size_t wsize = DIM(s) - 1;
    mpi_limb_t mask1;
    mpi_limb_t mask2;
    mpi_limb_t s_is_negative;


If we want to, we can get rid of VLA in __gcry_burn_stack too. For
example, following should work (avoids GCC from turning recursive
call to loop and wiping same 64-byte memory over and over again):

   void NOINLINE_FUNC
   __gcry_burn_stack_recursive (unsigned int bytes)
   {
     __gcry_burn_stack(bytes);
   }


   void NOINLINE_FUNC
   __gcry_burn_stack (unsigned int bytes)
   {
     char buf[64];

     _gcry_fast_wipememory (buf, sizeof buf);

     if (bytes > sizeof buf)
         __gcry_burn_stack_recursive (bytes - sizeof buf);
   }

'__gcry_burn_stack_recursive' could be moved to separate source
file just in case compiler does not support noinline function
attribute.

-Jussi