This is the mail archive of the
binutils@sourceware.org
mailing list for the binutils project.
Re: [RISCV] Use after free in gas/config/tc-riscv.c: riscv_set_arch()
- From: "Klaus Kruse Pedersen (Klaus)" <klauskpedersen at rdamicro dot com>
- To: "andrew at sifive dot com" <andrew at sifive dot com>
- Cc: "binutils at sourceware dot org" <binutils at sourceware dot org>
- Date: Wed, 26 Jul 2017 05:59:46 +0000
- Subject: Re: [RISCV] Use after free in gas/config/tc-riscv.c: riscv_set_arch()
- Authentication-results: sourceware.org; auth=none
- References: <1501044502.7598.3.camel@rdamicro.com> <CA++6G0BDWR9VynD=OJMxyhc9KEU=4Rzn68ocMJzZGay_iQSOxA@mail.gmail.com>
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
>
>