This is the mail archive of the
binutils@sourceware.org
mailing list for the binutils project.
Re: Commit: Use of alloca considered dangerous
- From: Nick Clifton <nickc at redhat dot com>
- To: Paul_Koning at dell dot com, law at redhat dot com
- Cc: binutils at sourceware dot org
- Date: Mon, 21 Mar 2016 17:34:32 +0000
- Subject: Re: Commit: Use of alloca considered dangerous
- Authentication-results: sourceware.org; auth=none
- References: <87mvprx079 dot fsf at redhat dot com> <B1F8A0EC-9D4D-459E-899A-5638E2CCE924 at dell dot com> <56F0284B dot 3010004 at redhat dot com> <5CFD6B41-561A-4058-85FE-3333931BC0F1 at dell dot com>
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