Messages in this thread | | | Date | Tue, 24 Nov 2020 15:36:45 +0100 | From | Jiri Olsa <> | Subject | Re: [PATCH v2 1/3] Revert "perf session: Fix decompression of PERF_RECORD_COMPRESSED records" |
| |
On Tue, Nov 24, 2020 at 11:29:15AM +0100, Petr Malat wrote: > Both mmapped and compressed events can be split by the buffer boundary, > it doesn't make sense to handle them differently.
hi, I'm going to need more than this, if there's a problem with current code please share more details, what's broken and how it shows
thanks, jirka
> > Fixes: bb1835a3b86c ("perf session: Fix decompression of PERF_RECORD_COMPRESSED records") > Fixes: 57fc032ad643 ("perf session: Avoid infinite loop when seeing invalid header.size") > Signed-off-by: Petr Malat <oss@malat.biz> > --- > tools/perf/util/session.c | 44 +++++++++++++++------------------------ > 1 file changed, 17 insertions(+), 27 deletions(-) > > diff --git a/tools/perf/util/session.c b/tools/perf/util/session.c > index 098080287c68..0d7a59c1aeb6 100644 > --- a/tools/perf/util/session.c > +++ b/tools/perf/util/session.c > @@ -2038,8 +2038,8 @@ static int __perf_session__process_pipe_events(struct perf_session *session) > } > > static union perf_event * > -prefetch_event(char *buf, u64 head, size_t mmap_size, > - bool needs_swap, union perf_event *error) > +fetch_mmaped_event(struct perf_session *session, > + u64 head, size_t mmap_size, char *buf) > { > union perf_event *event; > > @@ -2051,32 +2051,20 @@ prefetch_event(char *buf, u64 head, size_t mmap_size, > return NULL; > > event = (union perf_event *)(buf + head); > - if (needs_swap) > - perf_event_header__bswap(&event->header); > - > - if (head + event->header.size <= mmap_size) > - return event; > > - /* We're not fetching the event so swap back again */ > - if (needs_swap) > + if (session->header.needs_swap) > perf_event_header__bswap(&event->header); > > - pr_debug("%s: head=%#" PRIx64 " event->header_size=%#x, mmap_size=%#zx:" > - " fuzzed or compressed perf.data?\n",__func__, head, event->header.size, mmap_size); > - > - return error; > -} > - > -static union perf_event * > -fetch_mmaped_event(u64 head, size_t mmap_size, char *buf, bool needs_swap) > -{ > - return prefetch_event(buf, head, mmap_size, needs_swap, ERR_PTR(-EINVAL)); > -} > + if (head + event->header.size > mmap_size) { > + /* We're not fetching the event so swap back again */ > + if (session->header.needs_swap) > + perf_event_header__bswap(&event->header); > + pr_debug("%s: head=%#" PRIx64 " event->header_size=%#x, mmap_size=%#zx: fuzzed perf.data?\n", > + __func__, head, event->header.size, mmap_size); > + return ERR_PTR(-EINVAL); > + } > > -static union perf_event * > -fetch_decomp_event(u64 head, size_t mmap_size, char *buf, bool needs_swap) > -{ > - return prefetch_event(buf, head, mmap_size, needs_swap, NULL); > + return event; > } > > static int __perf_session__process_decomp_events(struct perf_session *session) > @@ -2089,8 +2077,10 @@ static int __perf_session__process_decomp_events(struct perf_session *session) > return 0; > > while (decomp->head < decomp->size && !session_done()) { > - union perf_event *event = fetch_decomp_event(decomp->head, decomp->size, decomp->data, > - session->header.needs_swap); > + union perf_event *event = fetch_mmaped_event(session, decomp->head, decomp->size, decomp->data); > + > + if (IS_ERR(event)) > + return PTR_ERR(event); > > if (!event) > break; > @@ -2190,7 +2180,7 @@ reader__process_events(struct reader *rd, struct perf_session *session, > } > > more: > - event = fetch_mmaped_event(head, mmap_size, buf, session->header.needs_swap); > + event = fetch_mmaped_event(session, head, mmap_size, buf); > if (IS_ERR(event)) > return PTR_ERR(event); > > -- > 2.20.1 >
| |