linux-kernel - Re: [PATCH] perf trace: Handle legacy syscalls

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: <20150320114011.4e25167c@gandalf.local.home>
Date:	Fri, 20 Mar 2015 11:40:11 -0400
From:	Steven Rostedt <rostedt@...dmis.org>
To:	Arnaldo Carvalho de Melo <acme@...nel.org>
Cc:	David Ahern <dsahern@...il.com>, linux-kernel@...r.kernel.org,
	Namhyung Kim <namhyung@...nel.org>,
	Jiri Olsa <jolsa@...nel.org>
Subject: Re: [PATCH] perf trace: Handle legacy syscalls

On Fri, 20 Mar 2015 12:32:26 -0300
Arnaldo Carvalho de Melo <acme@...nel.org> wrote:

> Em Thu, Mar 19, 2015 at 12:36:21PM -0600, David Ahern escreveu:
> > Currently the code skips the first field with the expectation that it is
> > 'nr'. But older kernels do not have the 'nr' field:
> > 
> >     field:int nr;   offset:8;   size:4; signed:1;
> > 
> > Change perf-trace to drop the field if it exists after parsing the format
> > file.
> > 
> > This fixes the off-by-one problem with older kernels (e.g., RHEL6). e.g,
> > perf-trace shows this for write:
> > 
> >     1.515 ( 0.006 ms): dd/4245 write(buf: 2</dev/pts/0>, count: 140733837536224                      ) = 26
> > 
> > where 2 is really the fd, the huge number is really the buf address, etc.
> > With this patch you get the more appropriate:
> > 
> >     1.813 ( 0.003 ms): dd/6330 write(fd: 2</dev/pts/0>, buf: 0x7fff22fc81f0, count: 25               ) = 25
> > 
> > Signed-off-by: David Ahern <dsahern@...il.com>
> > ---
> >  tools/lib/traceevent/event-parse.c | 11 ++++++++---
> >  tools/lib/traceevent/event-parse.h |  1 +
> >  tools/perf/builtin-trace.c         | 16 +++++++++++++---
> >  3 files changed, 22 insertions(+), 6 deletions(-)
> > 
> > diff --git a/tools/lib/traceevent/event-parse.c b/tools/lib/traceevent/event-parse.c
> > index afe20ed9fac8..e8a29e730dfb 100644
> > --- a/tools/lib/traceevent/event-parse.c
> > +++ b/tools/lib/traceevent/event-parse.c
> > @@ -6228,15 +6228,20 @@ void pevent_ref(struct pevent *pevent)
> >  	pevent->ref_count++;
> >  }
> >  
> > +void free_format_field(struct format_field *field)
> > +{
> > +	free(field->type);
> > +	free(field->name);
> > +	free(field);
> > +}
> > +
> >  static void free_format_fields(struct format_field *field)
> >  {
> >  	struct format_field *next;
> >  
> >  	while (field) {
> >  		next = field->next;
> > -		free(field->type);
> > -		free(field->name);
> > -		free(field);
> > +		free_format_field(field);
> >  		field = next;
> >  	}
> >  }
> > diff --git a/tools/lib/traceevent/event-parse.h b/tools/lib/traceevent/event-parse.h
> > index 5b4efc062320..a548fac646f6 100644
> > --- a/tools/lib/traceevent/event-parse.h
> > +++ b/tools/lib/traceevent/event-parse.h
> > @@ -619,6 +619,7 @@ enum pevent_errno pevent_parse_format(struct pevent *pevent,
> >  				      const char *buf,
> >  				      unsigned long size, const char *sys);
> >  void pevent_free_format(struct event_format *event);
> > +void free_format_field(struct format_field *field);

Can we make it "pevent_free_format_field" to keep consistency.

> >  
> >  void *pevent_get_field_raw(struct trace_seq *s, struct event_format *event,
> >  			   const char *name, struct pevent_record *record,
> > diff --git a/tools/perf/builtin-trace.c b/tools/perf/builtin-trace.c
> > index dcd950ef2fd7..5caeefeda48a 100644
> 
> 
> Up to here we should have a separate patch, one that I would like to
> have an Acked-by: Rostedt, ok?

I'm fine with the change if the name of the function is updated.

-- Steve

> 
> But then I don't think we need to do that, i.e. we can just have a
> boolean we set at some point to tell that we need to skip the first
> entry.
> 
> I'll try to cook up a patch for that.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/