Commit: Use of alloca considered dangerous

[Nick Clifton]

Hi Paul,

>>> I agree that alloca() needs bounds checks, but I don't see why you say that avoiding alloca entirely is "a good idea".
>> Based on what I've seen through the years, if you can't put a hard bounds on an alloca, then you're far better off from a security standpoint avoiding it completely.
> 
> Absolutely.  But isn't that a case by case test?  If 7 uses of alloca don't have a guaranteed hard bound but the 8th one does, then I agree the 7 have to change -- but would it not be reasonable to keep the 8th?

I only found two places where alloca was being used with a hard constant.
In both cases I replaced with just ordinary local array locations, like
this:

   if (sh_relax)
     {
       static int count = 0;
+      char name[11];
 
       /* If the last loop insn is a two-byte-insn, it is in danger of being
 	 swapped with the insn after it.  To prevent this, create a new

replacing:

       /* A REPEAT takes 6 bytes.  The SH has a 32 bit address space.
 	 Hence a 9 digit number should be enough to count all REPEATs.  */
-      name = alloca (11);
       sprintf (name, "_R%x", count++ & 0x3fffffff);


and:

   static void
   msp430_set_arch (int option)
   {
  -  char *str = (char *) alloca (32);	/* 32 for good measure.  */
  +  char str[32];	/* 32 for good measure.  */
 

That was it.  All the other uses of alloca had dynamically computed sizes 
which, at least theorertically, could be exploited.

Cheers
  Nick