linux-kernel - Re: [PATCH 2/2] perf, tools: Avoid segfault on alias parse error

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20170817164213.GE10891@kernel.org>
Date:   Thu, 17 Aug 2017 13:42:13 -0300
From:   Arnaldo Carvalho de Melo <acme@...nel.org>
To:     Andi Kleen <andi@...stfloor.org>
Cc:     jolsa@...nel.org, linux-kernel@...r.kernel.org,
        Andi Kleen <ak@...ux.intel.com>
Subject: Re: [PATCH 2/2] perf, tools: Avoid segfault on alias parse error

Em Thu, Aug 17, 2017 at 12:28:16PM -0300, Arnaldo Carvalho de Melo escreveu:
> Em Wed, Aug 16, 2017 at 03:02:01PM -0700, Andi Kleen escreveu:
> > +++ b/tools/perf/util/parse-events.h
> > @@ -108,15 +108,17 @@ struct parse_events_error {
> >  	char *help;	/* optional help string */
> >  };
> >  
> > +/* error field must match parse_events_terms */

Argh, this seems to be the oddity that causes these problems, a void
pointer is passed around and sometimes code thinks it is a
struct_parse_events_evlist pointer, like in

#line 117 "util/parse-events.y" /* yacc.c:1646  */
    {
        struct parse_events_evlist *data = _data;

        parse_events_update_lists((yyvsp[0].head), &data->list);
}
#line 1503 "/tmp/build/perf/util/parse-events-bison.c" /* yacc.c:1646  */

while sometimes it thinks it is something else, like in:

  case 52: 
#line 496 "util/parse-events.y" /* yacc.c:1646  */ 
    { 
        struct parse_events_terms *data = _data; 
        data->terms = (yyvsp[0].head); 
} 
#line 1931 "/tmp/build/perf/util/parse-events-bison.c" /* yacc.c:1646  */

So it will _not_ set to null a member that it doesn't have, i.e. the
minimal fix is to just have the hunks below, making sure that the error
field is present in both structs. No need to set
parse_event_terms->error to anything, it will be set to null since other
fields are set to something.

Longer term we need to fix this mess wrt that "_data" void pointer...

- Arnaldo

> >  struct parse_events_evlist {
> > +	struct parse_events_error *error;
> >  	struct list_head	   list;
> >  	int			   idx;
> >  	int			   nr_groups;
> > -	struct parse_events_error *error;
> >  	struct perf_evlist	  *evlist;
> >  };
> >  
> >  struct parse_events_terms {
> > +	struct parse_events_error *error;
> >  	struct list_head *terms;
> >  };
> >  
> > -- 
> > 2.9.4