Messages in this thread | | | Date | Fri, 15 Nov 2019 16:11:24 +0100 | From | Jiri Olsa <> | Subject | Re: [PATCH v1] perf session: fix decompression of PERF_RECORD_COMPRESSED records |
| |
On Fri, Nov 15, 2019 at 12:05:14PM +0300, Alexey Budankov wrote: > > Avoid termination of trace loading in case the last record in > the decompressed buffer partly resides in the following > mmaped PERF_RECORD_COMPRESSED record. In this case NULL value > returned by fetch_mmaped_event() means to proceed to the next > mmaped record then decompress it and load compressed events. > > The issue can be reproduced like this: > > $ perf record -z -- some_long_running_workload > $ perf report --stdio -vv > decomp (B): 44519 to 163000 > decomp (B): 48119 to 174800 > decomp (B): 65527 to 131072 > fetch_mmaped_event: head=0x1ffe0 event->header_size=0x28, mmap_size=0x20000: fuzzed perf.data? > Error: > failed to process sample > ... > > Testing: > 71: Zstd perf.data compression/decompression : Ok > > Fixes: 57fc032ad643 ("perf session: Avoid infinite loop when seeing invalid header.size") > Link: https://marc.info/?l=linux-kernel&m=156580812427554&w=2 > Co-developed-by: Jiri Olsa <jolsa@kernel.org> > Signed-off-by: Alexey Budankov <alexey.budankov@linux.intel.com> > --- > tools/perf/util/session.c | 47 +++++++++++++++++++++++++++++++++-------------- > 1 file changed, 33 insertions(+), 14 deletions(-) > > diff --git a/tools/perf/util/session.c b/tools/perf/util/session.c > index f07b8ecb91bc..3f6f812ec4ed 100644 > --- a/tools/perf/util/session.c > +++ b/tools/perf/util/session.c > @@ -1957,9 +1957,31 @@ static int __perf_session__process_pipe_events(struct perf_session *session) > return err; > } > > +static union perf_event * > +prefetch_event(char *buf, u64 head, size_t mmap_size, > + bool needs_swap, union perf_event *ret);
why not move prefetch_event definition in here? I don't see any need for the static declaration..
> + > static union perf_event * > fetch_mmaped_event(struct perf_session *session, > u64 head, size_t mmap_size, char *buf) > +{ > + return prefetch_event(buf, head, mmap_size, > + session->header.needs_swap, > + ERR_PTR(-EINVAL)); > +} > + > +static union perf_event * > +fetch_decomp_event(struct perf_session *session, > + u64 head, size_t mmap_size, char *buf) > +{
if this is decomp specific, it could take 'struct decomp*' as argument
> + return prefetch_event(buf, head, mmap_size, > + session->header.needs_swap, > + NULL); > +} > + > +static union perf_event * > +prefetch_event(char *buf, u64 head, size_t mmap_size, > + bool needs_swap, union perf_event *ret) > {
'error' might be more suitable then ret in here
thanks, jirka
| |