* [RFC] perf session: Fix compression processing
@ 2019-11-03 22:24 Jiri Olsa
2019-11-06 16:50 ` Alexey Budankov
2019-11-11 14:38 ` Alexey Budankov
0 siblings, 2 replies; 8+ messages in thread
From: Jiri Olsa @ 2019-11-03 22:24 UTC (permalink / raw)
To: Alexey Budankov, Arnaldo Carvalho de Melo
Cc: Peter Zijlstra, Ingo Molnar, Alexander Shishkin, Namhyung Kim,
Andi Kleen, linux-kernel
hi,
I'm not sure I follow everything on compression,
so I might have missed something, but patch below
fixes the issue for me.
jirka
---
The compressed data processing occasionally fails with:
$ 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
...
It's caused by recent fuzzer fix that does not take into account
that compressed data do not need to by fully present in the buffer,
so it's ok to just return NULL and not to fail.
Fixes: 57fc032ad643 ("perf session: Avoid infinite loop when seeing invalid header.size")
Link: http://lkml.kernel.org/n/tip-q1biqscs4stcmc9bs1iokfro@git.kernel.org
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
tools/perf/util/session.c | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/tools/perf/util/session.c b/tools/perf/util/session.c
index f07b8ecb91bc..3589ed14a629 100644
--- a/tools/perf/util/session.c
+++ b/tools/perf/util/session.c
@@ -1959,7 +1959,7 @@ static int __perf_session__process_pipe_events(struct perf_session *session)
static union perf_event *
fetch_mmaped_event(struct perf_session *session,
- u64 head, size_t mmap_size, char *buf)
+ u64 head, size_t mmap_size, char *buf, bool decomp)
{
union perf_event *event;
@@ -1979,6 +1979,8 @@ fetch_mmaped_event(struct perf_session *session,
/* We're not fetching the event so swap back again */
if (session->header.needs_swap)
perf_event_header__bswap(&event->header);
+ if (decomp)
+ return NULL;
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);
@@ -1997,7 +1999,7 @@ 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_mmaped_event(session, decomp->head, decomp->size, decomp->data);
+ union perf_event *event = fetch_mmaped_event(session, decomp->head, decomp->size, decomp->data, true);
if (IS_ERR(event))
return PTR_ERR(event);
@@ -2100,7 +2102,7 @@ reader__process_events(struct reader *rd, struct perf_session *session,
}
more:
- event = fetch_mmaped_event(session, head, mmap_size, buf);
+ event = fetch_mmaped_event(session, head, mmap_size, buf, false);
if (IS_ERR(event))
return PTR_ERR(event);
--
2.21.0
^ permalink raw reply related [flat|nested] 8+ messages in thread* Re: [RFC] perf session: Fix compression processing
2019-11-03 22:24 [RFC] perf session: Fix compression processing Jiri Olsa
@ 2019-11-06 16:50 ` Alexey Budankov
2019-11-11 14:38 ` Alexey Budankov
1 sibling, 0 replies; 8+ messages in thread
From: Alexey Budankov @ 2019-11-06 16:50 UTC (permalink / raw)
To: Jiri Olsa, Arnaldo Carvalho de Melo
Cc: Peter Zijlstra, Ingo Molnar, Alexander Shishkin, Namhyung Kim,
Andi Kleen, linux-kernel
On 04.11.2019 1:24, Jiri Olsa wrote:
> hi,
> I'm not sure I follow everything on compression,
> so I might have missed something, but patch below
> fixes the issue for me.
>
> jirka
>
>
> ---
> The compressed data processing occasionally fails with:
> $ 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
> ...
>
> It's caused by recent fuzzer fix that does not take into account
> that compressed data do not need to by fully present in the buffer,
> so it's ok to just return NULL and not to fail.
>
> Fixes: 57fc032ad643 ("perf session: Avoid infinite loop when seeing invalid header.size")
> Link: http://lkml.kernel.org/n/tip-q1biqscs4stcmc9bs1iokfro@git.kernel.org
> Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> ---
> tools/perf/util/session.c | 8 +++++---
> 1 file changed, 5 insertions(+), 3 deletions(-)
I am on vacations currently, getting back on Monday (11/11).
Please expect delay in response.
~Alexey
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [RFC] perf session: Fix compression processing
2019-11-03 22:24 [RFC] perf session: Fix compression processing Jiri Olsa
2019-11-06 16:50 ` Alexey Budankov
@ 2019-11-11 14:38 ` Alexey Budankov
2019-11-11 14:56 ` Jiri Olsa
1 sibling, 1 reply; 8+ messages in thread
From: Alexey Budankov @ 2019-11-11 14:38 UTC (permalink / raw)
To: Jiri Olsa, Arnaldo Carvalho de Melo
Cc: Peter Zijlstra, Ingo Molnar, Alexander Shishkin, Namhyung Kim,
Andi Kleen, linux-kernel
On 04.11.2019 1:24, Jiri Olsa wrote:
> hi,
<SNIP>
> ---
> The compressed data processing occasionally fails with:
> $ 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
> ...
>
> It's caused by recent fuzzer fix that does not take into account
> that compressed data do not need to by fully present in the buffer,
> so it's ok to just return NULL and not to fail.
>
> Fixes: 57fc032ad643 ("perf session: Avoid infinite loop when seeing invalid header.size")
> Link: http://lkml.kernel.org/n/tip-q1biqscs4stcmc9bs1iokfro@git.kernel.org
> Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> ---
> tools/perf/util/session.c | 8 +++++---
> 1 file changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/tools/perf/util/session.c b/tools/perf/util/session.c
> index f07b8ecb91bc..3589ed14a629 100644
> --- a/tools/perf/util/session.c
> +++ b/tools/perf/util/session.c
> @@ -1959,7 +1959,7 @@ static int __perf_session__process_pipe_events(struct perf_session *session)
>
> static union perf_event *
> fetch_mmaped_event(struct perf_session *session,
> - u64 head, size_t mmap_size, char *buf)
> + u64 head, size_t mmap_size, char *buf, bool decomp)
bools in interface make code less transparent.
> {
> union perf_event *event;
>
> @@ -1979,6 +1979,8 @@ fetch_mmaped_event(struct perf_session *session,
> /* We're not fetching the event so swap back again */
> if (session->header.needs_swap)
> perf_event_header__bswap(&event->header);
> + if (decomp)
> + return NULL;
> 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);
> @@ -1997,7 +1999,7 @@ 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_mmaped_event(session, decomp->head, decomp->size, decomp->data);
> + union perf_event *event = fetch_mmaped_event(session, decomp->head, decomp->size, decomp->data, true);
It looks like this call can be skipped, at all, in this case.
>
> if (IS_ERR(event))
> return PTR_ERR(event);
> @@ -2100,7 +2102,7 @@ reader__process_events(struct reader *rd, struct perf_session *session,
> }
>
> more:
> - event = fetch_mmaped_event(session, head, mmap_size, buf);
> + event = fetch_mmaped_event(session, head, mmap_size, buf, false);
> if (IS_ERR(event))
> return PTR_ERR(event);
>
>
~Alexey
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [RFC] perf session: Fix compression processing
2019-11-11 14:38 ` Alexey Budankov
@ 2019-11-11 14:56 ` Jiri Olsa
2019-11-11 15:41 ` Alexey Budankov
0 siblings, 1 reply; 8+ messages in thread
From: Jiri Olsa @ 2019-11-11 14:56 UTC (permalink / raw)
To: Alexey Budankov
Cc: Arnaldo Carvalho de Melo, Peter Zijlstra, Ingo Molnar,
Alexander Shishkin, Namhyung Kim, Andi Kleen, linux-kernel
On Mon, Nov 11, 2019 at 05:38:49PM +0300, Alexey Budankov wrote:
>
> On 04.11.2019 1:24, Jiri Olsa wrote:
> > hi,
> <SNIP>
> > ---
> > The compressed data processing occasionally fails with:
> > $ 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
> > ...
> >
> > It's caused by recent fuzzer fix that does not take into account
> > that compressed data do not need to by fully present in the buffer,
> > so it's ok to just return NULL and not to fail.
> >
> > Fixes: 57fc032ad643 ("perf session: Avoid infinite loop when seeing invalid header.size")
> > Link: http://lkml.kernel.org/n/tip-q1biqscs4stcmc9bs1iokfro@git.kernel.org
> > Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> > ---
> > tools/perf/util/session.c | 8 +++++---
> > 1 file changed, 5 insertions(+), 3 deletions(-)
> >
> > diff --git a/tools/perf/util/session.c b/tools/perf/util/session.c
> > index f07b8ecb91bc..3589ed14a629 100644
> > --- a/tools/perf/util/session.c
> > +++ b/tools/perf/util/session.c
> > @@ -1959,7 +1959,7 @@ static int __perf_session__process_pipe_events(struct perf_session *session)
> >
> > static union perf_event *
> > fetch_mmaped_event(struct perf_session *session,
> > - u64 head, size_t mmap_size, char *buf)
> > + u64 head, size_t mmap_size, char *buf, bool decomp)
>
> bools in interface make code less transparent.
>
> > {
> > union perf_event *event;
> >
> > @@ -1979,6 +1979,8 @@ fetch_mmaped_event(struct perf_session *session,
> > /* We're not fetching the event so swap back again */
> > if (session->header.needs_swap)
> > perf_event_header__bswap(&event->header);
> > + if (decomp)
> > + return NULL;
> > 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);
> > @@ -1997,7 +1999,7 @@ 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_mmaped_event(session, decomp->head, decomp->size, decomp->data);
> > + union perf_event *event = fetch_mmaped_event(session, decomp->head, decomp->size, decomp->data, true);
>
> It looks like this call can be skipped, at all, in this case.
not sure what you mean, we are in decomp code no?
jirka
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [RFC] perf session: Fix compression processing
2019-11-11 14:56 ` Jiri Olsa
@ 2019-11-11 15:41 ` Alexey Budankov
2019-11-11 15:46 ` Jiri Olsa
0 siblings, 1 reply; 8+ messages in thread
From: Alexey Budankov @ 2019-11-11 15:41 UTC (permalink / raw)
To: Jiri Olsa
Cc: Arnaldo Carvalho de Melo, Peter Zijlstra, Ingo Molnar,
Alexander Shishkin, Namhyung Kim, Andi Kleen, linux-kernel
On 11.11.2019 17:56, Jiri Olsa wrote:
> On Mon, Nov 11, 2019 at 05:38:49PM +0300, Alexey Budankov wrote:
>>
>> On 04.11.2019 1:24, Jiri Olsa wrote:
>>> hi,
>> <SNIP>
>>> ---
>>> The compressed data processing occasionally fails with:
>>> $ 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
>>> ...
>>>
>>> It's caused by recent fuzzer fix that does not take into account
>>> that compressed data do not need to by fully present in the buffer,
>>> so it's ok to just return NULL and not to fail.
>>>
>>> Fixes: 57fc032ad643 ("perf session: Avoid infinite loop when seeing invalid header.size")
>>> Link: http://lkml.kernel.org/n/tip-q1biqscs4stcmc9bs1iokfro@git.kernel.org
>>> Signed-off-by: Jiri Olsa <jolsa@kernel.org>
>>> ---
>>> tools/perf/util/session.c | 8 +++++---
>>> 1 file changed, 5 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/tools/perf/util/session.c b/tools/perf/util/session.c
>>> index f07b8ecb91bc..3589ed14a629 100644
>>> --- a/tools/perf/util/session.c
>>> +++ b/tools/perf/util/session.c
>>> @@ -1959,7 +1959,7 @@ static int __perf_session__process_pipe_events(struct perf_session *session)
>>>
>>> static union perf_event *
>>> fetch_mmaped_event(struct perf_session *session,
>>> - u64 head, size_t mmap_size, char *buf)
>>> + u64 head, size_t mmap_size, char *buf, bool decomp)
>>
>> bools in interface make code less transparent.
>>
>>> {
>>> union perf_event *event;
>>>
>>> @@ -1979,6 +1979,8 @@ fetch_mmaped_event(struct perf_session *session,
>>> /* We're not fetching the event so swap back again */
>>> if (session->header.needs_swap)
>>> perf_event_header__bswap(&event->header);
>>> + if (decomp)
>>> + return NULL;
>>> 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);
>>> @@ -1997,7 +1999,7 @@ 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_mmaped_event(session, decomp->head, decomp->size, decomp->data);
>>> + union perf_event *event = fetch_mmaped_event(session, decomp->head, decomp->size, decomp->data, true);
>>
>> It looks like this call can be skipped, at all, in this case.
>
> not sure what you mean, we are in decomp code no?
Ok, it is inside "not fetching" branch.
NULL return value means to proceed getting further over the trace.
Checking record type == COMPRESSED at the higher level could
probably be cleaner fix and also work faster.
~Alexey
>
> jirka
>
>
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [RFC] perf session: Fix compression processing
2019-11-11 15:41 ` Alexey Budankov
@ 2019-11-11 15:46 ` Jiri Olsa
2019-11-11 15:53 ` Alexey Budankov
0 siblings, 1 reply; 8+ messages in thread
From: Jiri Olsa @ 2019-11-11 15:46 UTC (permalink / raw)
To: Alexey Budankov
Cc: Arnaldo Carvalho de Melo, Peter Zijlstra, Ingo Molnar,
Alexander Shishkin, Namhyung Kim, Andi Kleen, linux-kernel
On Mon, Nov 11, 2019 at 06:41:47PM +0300, Alexey Budankov wrote:
> On 11.11.2019 17:56, Jiri Olsa wrote:
> > On Mon, Nov 11, 2019 at 05:38:49PM +0300, Alexey Budankov wrote:
> >>
> >> On 04.11.2019 1:24, Jiri Olsa wrote:
> >>> hi,
> >> <SNIP>
> >>> ---
> >>> The compressed data processing occasionally fails with:
> >>> $ 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
> >>> ...
> >>>
> >>> It's caused by recent fuzzer fix that does not take into account
> >>> that compressed data do not need to by fully present in the buffer,
> >>> so it's ok to just return NULL and not to fail.
> >>>
> >>> Fixes: 57fc032ad643 ("perf session: Avoid infinite loop when seeing invalid header.size")
> >>> Link: http://lkml.kernel.org/n/tip-q1biqscs4stcmc9bs1iokfro@git.kernel.org
> >>> Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> >>> ---
> >>> tools/perf/util/session.c | 8 +++++---
> >>> 1 file changed, 5 insertions(+), 3 deletions(-)
> >>>
> >>> diff --git a/tools/perf/util/session.c b/tools/perf/util/session.c
> >>> index f07b8ecb91bc..3589ed14a629 100644
> >>> --- a/tools/perf/util/session.c
> >>> +++ b/tools/perf/util/session.c
> >>> @@ -1959,7 +1959,7 @@ static int __perf_session__process_pipe_events(struct perf_session *session)
> >>>
> >>> static union perf_event *
> >>> fetch_mmaped_event(struct perf_session *session,
> >>> - u64 head, size_t mmap_size, char *buf)
> >>> + u64 head, size_t mmap_size, char *buf, bool decomp)
> >>
> >> bools in interface make code less transparent.
> >>
> >>> {
> >>> union perf_event *event;
> >>>
> >>> @@ -1979,6 +1979,8 @@ fetch_mmaped_event(struct perf_session *session,
> >>> /* We're not fetching the event so swap back again */
> >>> if (session->header.needs_swap)
> >>> perf_event_header__bswap(&event->header);
> >>> + if (decomp)
> >>> + return NULL;
> >>> 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);
> >>> @@ -1997,7 +1999,7 @@ 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_mmaped_event(session, decomp->head, decomp->size, decomp->data);
> >>> + union perf_event *event = fetch_mmaped_event(session, decomp->head, decomp->size, decomp->data, true);
> >>
> >> It looks like this call can be skipped, at all, in this case.
> >
> > not sure what you mean, we are in decomp code no?
>
> Ok, it is inside "not fetching" branch.
> NULL return value means to proceed getting further over the trace.
> Checking record type == COMPRESSED at the higher level could
> probably be cleaner fix and also work faster.
any chance you could post the fix? the patch I did was a
quick fix to get the feature working for presentation ;-)
you're probably thinking of the proper approach
thanks,
jirka
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [RFC] perf session: Fix compression processing
2019-11-11 15:46 ` Jiri Olsa
@ 2019-11-11 15:53 ` Alexey Budankov
2019-11-11 16:07 ` Jiri Olsa
0 siblings, 1 reply; 8+ messages in thread
From: Alexey Budankov @ 2019-11-11 15:53 UTC (permalink / raw)
To: Jiri Olsa
Cc: Arnaldo Carvalho de Melo, Peter Zijlstra, Ingo Molnar,
Alexander Shishkin, Namhyung Kim, Andi Kleen, linux-kernel
On 11.11.2019 18:46, Jiri Olsa wrote:
> On Mon, Nov 11, 2019 at 06:41:47PM +0300, Alexey Budankov wrote:
>> On 11.11.2019 17:56, Jiri Olsa wrote:
>>> On Mon, Nov 11, 2019 at 05:38:49PM +0300, Alexey Budankov wrote:
>>>>
>>>> On 04.11.2019 1:24, Jiri Olsa wrote:
>>>>> hi,
>>>> <SNIP>
>>>>> ---
>>>>> The compressed data processing occasionally fails with:
>>>>> $ 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
>>>>> ...
>>>>>
>>>>> It's caused by recent fuzzer fix that does not take into account
>>>>> that compressed data do not need to by fully present in the buffer,
>>>>> so it's ok to just return NULL and not to fail.
>>>>>
>>>>> Fixes: 57fc032ad643 ("perf session: Avoid infinite loop when seeing invalid header.size")
>>>>> Link: http://lkml.kernel.org/n/tip-q1biqscs4stcmc9bs1iokfro@git.kernel.org
>>>>> Signed-off-by: Jiri Olsa <jolsa@kernel.org>
>>>>> ---
>>>>> tools/perf/util/session.c | 8 +++++---
>>>>> 1 file changed, 5 insertions(+), 3 deletions(-)
>>>>>
>>>>> diff --git a/tools/perf/util/session.c b/tools/perf/util/session.c
>>>>> index f07b8ecb91bc..3589ed14a629 100644
>>>>> --- a/tools/perf/util/session.c
>>>>> +++ b/tools/perf/util/session.c
>>>>> @@ -1959,7 +1959,7 @@ static int __perf_session__process_pipe_events(struct perf_session *session)
>>>>>
>>>>> static union perf_event *
>>>>> fetch_mmaped_event(struct perf_session *session,
>>>>> - u64 head, size_t mmap_size, char *buf)
>>>>> + u64 head, size_t mmap_size, char *buf, bool decomp)
>>>>
>>>> bools in interface make code less transparent.
>>>>
>>>>> {
>>>>> union perf_event *event;
>>>>>
>>>>> @@ -1979,6 +1979,8 @@ fetch_mmaped_event(struct perf_session *session,
>>>>> /* We're not fetching the event so swap back again */
>>>>> if (session->header.needs_swap)
>>>>> perf_event_header__bswap(&event->header);
>>>>> + if (decomp)
>>>>> + return NULL;
>>>>> 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);
>>>>> @@ -1997,7 +1999,7 @@ 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_mmaped_event(session, decomp->head, decomp->size, decomp->data);
>>>>> + union perf_event *event = fetch_mmaped_event(session, decomp->head, decomp->size, decomp->data, true);
>>>>
>>>> It looks like this call can be skipped, at all, in this case.
>>>
>>> not sure what you mean, we are in decomp code no?
>>
>> Ok, it is inside "not fetching" branch.
>> NULL return value means to proceed getting further over the trace.
>> Checking record type == COMPRESSED at the higher level could
>> probably be cleaner fix and also work faster.
>
> any chance you could post the fix? the patch I did was a
> quick fix to get the feature working for presentation ;-)
> you're probably thinking of the proper approach
Please share the exact reproducing steps
so I could come up with something.
~Alexey
>
> thanks,
> jirka
>
>
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [RFC] perf session: Fix compression processing
2019-11-11 15:53 ` Alexey Budankov
@ 2019-11-11 16:07 ` Jiri Olsa
0 siblings, 0 replies; 8+ messages in thread
From: Jiri Olsa @ 2019-11-11 16:07 UTC (permalink / raw)
To: Alexey Budankov
Cc: Arnaldo Carvalho de Melo, Peter Zijlstra, Ingo Molnar,
Alexander Shishkin, Namhyung Kim, Andi Kleen, linux-kernel
On Mon, Nov 11, 2019 at 06:53:35PM +0300, Alexey Budankov wrote:
> On 11.11.2019 18:46, Jiri Olsa wrote:
> > On Mon, Nov 11, 2019 at 06:41:47PM +0300, Alexey Budankov wrote:
> >> On 11.11.2019 17:56, Jiri Olsa wrote:
> >>> On Mon, Nov 11, 2019 at 05:38:49PM +0300, Alexey Budankov wrote:
> >>>>
> >>>> On 04.11.2019 1:24, Jiri Olsa wrote:
> >>>>> hi,
> >>>> <SNIP>
> >>>>> ---
> >>>>> The compressed data processing occasionally fails with:
> >>>>> $ 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
> >>>>> ...
> >>>>>
> >>>>> It's caused by recent fuzzer fix that does not take into account
> >>>>> that compressed data do not need to by fully present in the buffer,
> >>>>> so it's ok to just return NULL and not to fail.
> >>>>>
> >>>>> Fixes: 57fc032ad643 ("perf session: Avoid infinite loop when seeing invalid header.size")
> >>>>> Link: http://lkml.kernel.org/n/tip-q1biqscs4stcmc9bs1iokfro@git.kernel.org
> >>>>> Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> >>>>> ---
> >>>>> tools/perf/util/session.c | 8 +++++---
> >>>>> 1 file changed, 5 insertions(+), 3 deletions(-)
> >>>>>
> >>>>> diff --git a/tools/perf/util/session.c b/tools/perf/util/session.c
> >>>>> index f07b8ecb91bc..3589ed14a629 100644
> >>>>> --- a/tools/perf/util/session.c
> >>>>> +++ b/tools/perf/util/session.c
> >>>>> @@ -1959,7 +1959,7 @@ static int __perf_session__process_pipe_events(struct perf_session *session)
> >>>>>
> >>>>> static union perf_event *
> >>>>> fetch_mmaped_event(struct perf_session *session,
> >>>>> - u64 head, size_t mmap_size, char *buf)
> >>>>> + u64 head, size_t mmap_size, char *buf, bool decomp)
> >>>>
> >>>> bools in interface make code less transparent.
> >>>>
> >>>>> {
> >>>>> union perf_event *event;
> >>>>>
> >>>>> @@ -1979,6 +1979,8 @@ fetch_mmaped_event(struct perf_session *session,
> >>>>> /* We're not fetching the event so swap back again */
> >>>>> if (session->header.needs_swap)
> >>>>> perf_event_header__bswap(&event->header);
> >>>>> + if (decomp)
> >>>>> + return NULL;
> >>>>> 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);
> >>>>> @@ -1997,7 +1999,7 @@ 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_mmaped_event(session, decomp->head, decomp->size, decomp->data);
> >>>>> + union perf_event *event = fetch_mmaped_event(session, decomp->head, decomp->size, decomp->data, true);
> >>>>
> >>>> It looks like this call can be skipped, at all, in this case.
> >>>
> >>> not sure what you mean, we are in decomp code no?
> >>
> >> Ok, it is inside "not fetching" branch.
> >> NULL return value means to proceed getting further over the trace.
> >> Checking record type == COMPRESSED at the higher level could
> >> probably be cleaner fix and also work faster.
> >
> > any chance you could post the fix? the patch I did was a
> > quick fix to get the feature working for presentation ;-)
> > you're probably thinking of the proper approach
>
> Please share the exact reproducing steps
> so I could come up with something.
'perf record -z' for longer workloads
jirka
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2019-11-11 16:07 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-11-03 22:24 [RFC] perf session: Fix compression processing Jiri Olsa
2019-11-06 16:50 ` Alexey Budankov
2019-11-11 14:38 ` Alexey Budankov
2019-11-11 14:56 ` Jiri Olsa
2019-11-11 15:41 ` Alexey Budankov
2019-11-11 15:46 ` Jiri Olsa
2019-11-11 15:53 ` Alexey Budankov
2019-11-11 16:07 ` Jiri Olsa
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).