2010-07-20 00:46:58

by David Daney

Subject: [PATCH 0/2] trace-cmd: Fix a couple of crashes.

I have been playing around with trace-cmd and kernelshark on mips64
and have encountered a couple of crashes.

These two patches fix the crashes. But what is slightly less clear, is
if they may cause valid trace events to be omitted. I don't think they do.

In any event here they are.

David Daney (2):
trace-cmd: Don't try to read unmapped memory (v2).
trace-cmd: Don't SIGSEGV in trace-graph.c:load_handle

parse-events.c | 5 +++++
trace-graph.c | 2 ++
trace-input.c | 3 ++-
3 files changed, 9 insertions(+), 1 deletions(-)

2010-07-20 00:46:59

by David Daney

Subject: [PATCH 1/2] trace-cmd: Don't try to read unmapped memory (v2).

Under some circumstances, I'm not sure exactly which, a trace.dat file
may contain a bunch of zeros at the end of one or more of the trace
logs. This can lead to tracecmd_peek_data() attempting to read past
the end of the mmaped log, causing SIGSEGV.

This is a two part fix.

1) For 'new format' data, we always try to read 8 bytes of data. Make
sure that they are all on the current page.

2) In pevent_print_event(), if record->size is negative, warn and then
skip attempting to print it.

Signed-off-by: David Daney <[email protected]>
---
parse-events.c | 5 +++++
trace-input.c | 3 ++-
2 files changed, 7 insertions(+), 1 deletions(-)

diff --git a/parse-events.c b/parse-events.c
index 16fff12..52b68f0 100644
--- a/parse-events.c
+++ b/parse-events.c
@@ -3881,6 +3881,11 @@ void pevent_print_event(struct pevent *pevent, struct trace_seq *s,
usecs = record->ts - secs * NSECS_PER_SEC;
usecs = (usecs + 500) / NSECS_PER_USEC;

+ if (record->size < 0) {
+ do_warning("ug! negative record size %d", record->size);
+ return;
+ }
+
type = trace_parse_common_type(pevent, data);

event = pevent_find_event(pevent, type);
diff --git a/trace-input.c b/trace-input.c
index 398d0f9..7241197 100644
--- a/trace-input.c
+++ b/trace-input.c
@@ -1511,7 +1511,8 @@ read_again:
if (index < 0)
die("negative index on cpu record %d", cpu);

- if (index >= handle->cpu_data[cpu].page_size + pevent->header_page_data_offset) {
+ if (index + (pevent->old_format ? 0 : 4) >=
+ handle->cpu_data[cpu].page_size + pevent->header_page_data_offset) {
if (get_next_page(handle, cpu))
return NULL;
return tracecmd_peek_data(handle, cpu);
--
1.7.1.1

2010-07-20 00:46:56

by David Daney

Subject: [PATCH 2/2] trace-cmd: Don't SIGSEGV in trace-graph.c:load_handle

This 'fixes' another crash I encountered. Don't try to dereference
NULL records.

Signed-off-by: David Daney <[email protected]>
---
trace-graph.c | 2 ++
1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/trace-graph.c b/trace-graph.c
index f940874..a53ab84 100644
--- a/trace-graph.c
+++ b/trace-graph.c
@@ -2413,6 +2413,8 @@ static int load_handle(struct graph_info *ginfo,

free_record(record);
record = tracecmd_read_cpu_last(handle, cpu);
+ if (!record)
+ continue;

if (record->ts > ginfo->end_time)
ginfo->end_time = record->ts;
--
1.7.1.1

2010-07-20 19:56:00

by Steven Rostedt

Subject: Re: [PATCH 0/2] trace-cmd: Fix a couple of crashes.

On Mon, 2010-07-19 at 17:46 -0700, David Daney wrote:
> I have been playing around with trace-cmd and kernelshark on mips64
> and have encountered a couple of crashes.
>
> These two patches fix the crashes. But what is slightly less clear, is
> if they may cause valid trace events to be omitted. I don't think they do.
>
> In any event here they are.
>
> David Daney (2):
> trace-cmd: Don't try to read unmapped memory (v2).
> trace-cmd: Don't SIGSEGV in trace-graph.c:load_handle

Applied!

Thanks David,

-- Steve