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 <o...@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
>
