[RISCV] Use after free in gas/config/tc-riscv.c: riscv_set_arch()

[Klaus Kruse Pedersen (Klaus)]

On Tue, 2017-07-25 at 22:08 -0700, Andrew Waterman wrote:
> Hi Klaus,
> 
> Thanks for pointing this out.  We'll submit a patch shortly, tail
> between legs.

Andrew,

Actually, just below there is something that look like another bug. The
separate check for 'q' seem to suggest that 'q' can be specified at any
position in the arch string:

 150 const char *all_subsets = "imafdc";
[...]
 207       else if ((all_subsets = strchr (all_subsets, *p)) != NULL)
 208         {
 209           const char subset[] = {*p, 0};
 210           riscv_add_subset (subset);
 211           all_subsets++;
 212           p++;
 213         }
 214       else if (*p == 'q')
 215         {
 216           const char subset[] = {*p, 0};
 217           riscv_add_subset (subset);
 218           p++;
 219         }

Now, lets say you pass: '...qc'. When matching 'q' strchr() will not
find 'q' in all_subsets and return NULL; and the 'if (*p == 'q')'
branch is executed as expected.
But all_subsets is now NULL, so the 'c' can't be matched in the next
iteration.

So implicitly 'q' have to be the last subset. i.e. 'imfadcq'.

But then there is no reason for the special case:

 150 const char *all_subsets = "imafdcq";
[...]
 207       else if
((all_subsets = strchr (all_subsets, *p)) != NULL)
 208         {
 209   
        const char subset[] = {*p, 0};
 210           riscv_add_subset
(subset);
 211           all_subsets++;
 212           p++;
 213         }


Right?


Klaus





> 
> Andrew
> 
> On Tue, Jul 25, 2017 at 9:48 PM, Klaus Kruse Pedersen (Klaus)
> <klauskpedersen@rdamicro.com> wrote:
> > 
> > 
> > The code below will pass a pointer to a free'd string to as_fatal()
> > when more than one extension is been specified:
> > 
> >  151 const char *extension = NULL;
> > [...]
> >  187   while (*p)
> >  188     {
> >  189       if (*p == 'x')
> >  190         {
> >  191           char *subset = xstrdup (p), *q = subset;
> >  192
> >  193           while (*++q != '\0' && *q != '_')
> >  194             ;
> >  195           *q = '\0';
> >  196
> >  197           if (extension)
> >  198             as_fatal ("-march=%s: only one non-standard
> > extension
> > is supported"
> >  199                       " (found `%s' and `%s')", s, extension,
> > subset);
> > 
> > Problem is here:
> > 
> >  200           extension = subset;
> >  201           riscv_add_subset (subset);
> >  202           p += strlen (subset);
> >  203           free (subset);
> >  204         }
> > [...]
> > 
> > https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;a=blob;f=g
> > as/c
> > onfig/tc-
> > riscv.c;h=55c41c5db3c9240e8da3cdf8906cfb745d041c6f;hb=HEAD#l187
> 
>