* [PATCH v1 1/2] perf cs-etm: Always allocate memory for cs_etm_queue::prev_packet
@ 2019-04-28 8:32 ` Leo Yan
0 siblings, 0 replies; 12+ messages in thread
From: Leo Yan @ 2019-04-28 8:32 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo, Mathieu Poirier, Robert Walker,
Suzuki K Poulose, Mike Leach, Alexander Shishkin, Jiri Olsa,
Namhyung Kim, linux-arm-kernel, linux-kernel
Cc: Leo Yan
Robert Walker reported a segmentation fault is observed when process
CoreSight trace data; this issue can be easily reproduced by the
command 'perf report --itrace=i1000i' for decoding tracing data.
If neither the 'b' flag (synthesize branches events) nor 'l' flag
(synthesize last branch entries) are specified to option '--itrace',
cs_etm_queue::prev_packet will not been initialised. After merging
the code to support exception packets and sample flags, there
introduced a number of uses of cs_etm_queue::prev_packet without
checking whether it is valid, for these cases any accessing to
uninitialised prev_packet will cause crash.
As cs_etm_queue::prev_packet is used more widely now and it's already
hard to follow which functions have been called in a context where the
validity of cs_etm_queue::prev_packet has been checked, this patch
always allocates memory for cs_etm_queue::prev_packet.
Reported-by: Robert Walker <robert.walker@arm.com>
Suggested-by: Robert Walker <robert.walker@arm.com>
Fixes: 7100b12cf474 ("perf cs-etm: Generate branch sample for exception packet")
Fixes: 24fff5eb2b93 ("perf cs-etm: Avoid stale branch samples when flush packet")
Signed-off-by: Leo Yan <leo.yan@linaro.org>
---
tools/perf/util/cs-etm.c | 8 +++-----
1 file changed, 3 insertions(+), 5 deletions(-)
diff --git a/tools/perf/util/cs-etm.c b/tools/perf/util/cs-etm.c
index 110804936fc3..054b480aab04 100644
--- a/tools/perf/util/cs-etm.c
+++ b/tools/perf/util/cs-etm.c
@@ -422,11 +422,9 @@ static struct cs_etm_queue *cs_etm__alloc_queue(struct cs_etm_auxtrace *etm)
if (!etmq->packet)
goto out_free;
- if (etm->synth_opts.last_branch || etm->sample_branches) {
- etmq->prev_packet = zalloc(szp);
- if (!etmq->prev_packet)
- goto out_free;
- }
+ etmq->prev_packet = zalloc(szp);
+ if (!etmq->prev_packet)
+ goto out_free;
if (etm->synth_opts.last_branch) {
size_t sz = sizeof(struct branch_stack);
--
2.17.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH v1 2/2] perf cs-etm: Don't check cs_etm_queue::prev_packet validity
2019-04-28 8:32 ` Leo Yan
@ 2019-04-28 8:32 ` Leo Yan
-1 siblings, 0 replies; 12+ messages in thread
From: Leo Yan @ 2019-04-28 8:32 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo, Mathieu Poirier, Robert Walker,
Suzuki K Poulose, Mike Leach, Alexander Shishkin, Jiri Olsa,
Namhyung Kim, linux-arm-kernel, linux-kernel
Cc: Leo Yan
Since cs_etm_queue::prev_packet is allocated for all cases, it will
never be NULL pointer; now validity checking prev_packet is pointless,
remove all of them.
Signed-off-by: Leo Yan <leo.yan@linaro.org>
---
tools/perf/util/cs-etm.c | 6 +-----
1 file changed, 1 insertion(+), 5 deletions(-)
diff --git a/tools/perf/util/cs-etm.c b/tools/perf/util/cs-etm.c
index 054b480aab04..de488b43f440 100644
--- a/tools/perf/util/cs-etm.c
+++ b/tools/perf/util/cs-etm.c
@@ -979,7 +979,6 @@ static int cs_etm__sample(struct cs_etm_queue *etmq)
* PREV_PACKET is a branch.
*/
if (etm->synth_opts.last_branch &&
- etmq->prev_packet &&
etmq->prev_packet->sample_type == CS_ETM_RANGE &&
etmq->prev_packet->last_instr_taken_branch)
cs_etm__update_last_branch_rb(etmq);
@@ -1012,7 +1011,7 @@ static int cs_etm__sample(struct cs_etm_queue *etmq)
etmq->period_instructions = instrs_over;
}
- if (etm->sample_branches && etmq->prev_packet) {
+ if (etm->sample_branches) {
bool generate_sample = false;
/* Generate sample for tracing on packet */
@@ -1069,9 +1068,6 @@ static int cs_etm__flush(struct cs_etm_queue *etmq)
struct cs_etm_auxtrace *etm = etmq->etm;
struct cs_etm_packet *tmp;
- if (!etmq->prev_packet)
- return 0;
-
/* Handle start tracing packet */
if (etmq->prev_packet->sample_type == CS_ETM_EMPTY)
goto swap_packet;
--
2.17.1
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH v1 2/2] perf cs-etm: Don't check cs_etm_queue::prev_packet validity
@ 2019-04-28 8:32 ` Leo Yan
0 siblings, 0 replies; 12+ messages in thread
From: Leo Yan @ 2019-04-28 8:32 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo, Mathieu Poirier, Robert Walker,
Suzuki K Poulose, Mike Leach, Alexander Shishkin, Jiri Olsa,
Namhyung Kim, linux-arm-kernel, linux-kernel
Cc: Leo Yan
Since cs_etm_queue::prev_packet is allocated for all cases, it will
never be NULL pointer; now validity checking prev_packet is pointless,
remove all of them.
Signed-off-by: Leo Yan <leo.yan@linaro.org>
---
tools/perf/util/cs-etm.c | 6 +-----
1 file changed, 1 insertion(+), 5 deletions(-)
diff --git a/tools/perf/util/cs-etm.c b/tools/perf/util/cs-etm.c
index 054b480aab04..de488b43f440 100644
--- a/tools/perf/util/cs-etm.c
+++ b/tools/perf/util/cs-etm.c
@@ -979,7 +979,6 @@ static int cs_etm__sample(struct cs_etm_queue *etmq)
* PREV_PACKET is a branch.
*/
if (etm->synth_opts.last_branch &&
- etmq->prev_packet &&
etmq->prev_packet->sample_type == CS_ETM_RANGE &&
etmq->prev_packet->last_instr_taken_branch)
cs_etm__update_last_branch_rb(etmq);
@@ -1012,7 +1011,7 @@ static int cs_etm__sample(struct cs_etm_queue *etmq)
etmq->period_instructions = instrs_over;
}
- if (etm->sample_branches && etmq->prev_packet) {
+ if (etm->sample_branches) {
bool generate_sample = false;
/* Generate sample for tracing on packet */
@@ -1069,9 +1068,6 @@ static int cs_etm__flush(struct cs_etm_queue *etmq)
struct cs_etm_auxtrace *etm = etmq->etm;
struct cs_etm_packet *tmp;
- if (!etmq->prev_packet)
- return 0;
-
/* Handle start tracing packet */
if (etmq->prev_packet->sample_type == CS_ETM_EMPTY)
goto swap_packet;
--
2.17.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH v1 2/2] perf cs-etm: Don't check cs_etm_queue::prev_packet validity
2019-04-28 8:32 ` Leo Yan
@ 2019-04-29 14:53 ` Robert Walker
-1 siblings, 0 replies; 12+ messages in thread
From: Robert Walker @ 2019-04-29 14:53 UTC (permalink / raw)
To: Leo Yan, Arnaldo Carvalho de Melo, Mathieu Poirier,
Suzuki K Poulose, Mike Leach, Alexander Shishkin, Jiri Olsa,
Namhyung Kim, linux-arm-kernel, linux-kernel
On 28/04/2019 09:32, Leo Yan wrote:
> Since cs_etm_queue::prev_packet is allocated for all cases, it will
> never be NULL pointer; now validity checking prev_packet is pointless,
> remove all of them.
>
> Signed-off-by: Leo Yan <leo.yan@linaro.org>
> ---
> tools/perf/util/cs-etm.c | 6 +-----
> 1 file changed, 1 insertion(+), 5 deletions(-)
Tested-by: Robert Walker <robert.walker@arm.com>
Regards
Rob
> diff --git a/tools/perf/util/cs-etm.c b/tools/perf/util/cs-etm.c
> index 054b480aab04..de488b43f440 100644
> --- a/tools/perf/util/cs-etm.c
> +++ b/tools/perf/util/cs-etm.c
> @@ -979,7 +979,6 @@ static int cs_etm__sample(struct cs_etm_queue *etmq)
> * PREV_PACKET is a branch.
> */
> if (etm->synth_opts.last_branch &&
> - etmq->prev_packet &&
> etmq->prev_packet->sample_type == CS_ETM_RANGE &&
> etmq->prev_packet->last_instr_taken_branch)
> cs_etm__update_last_branch_rb(etmq);
> @@ -1012,7 +1011,7 @@ static int cs_etm__sample(struct cs_etm_queue *etmq)
> etmq->period_instructions = instrs_over;
> }
>
> - if (etm->sample_branches && etmq->prev_packet) {
> + if (etm->sample_branches) {
> bool generate_sample = false;
>
> /* Generate sample for tracing on packet */
> @@ -1069,9 +1068,6 @@ static int cs_etm__flush(struct cs_etm_queue *etmq)
> struct cs_etm_auxtrace *etm = etmq->etm;
> struct cs_etm_packet *tmp;
>
> - if (!etmq->prev_packet)
> - return 0;
> -
> /* Handle start tracing packet */
> if (etmq->prev_packet->sample_type == CS_ETM_EMPTY)
> goto swap_packet;
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v1 2/2] perf cs-etm: Don't check cs_etm_queue::prev_packet validity
@ 2019-04-29 14:53 ` Robert Walker
0 siblings, 0 replies; 12+ messages in thread
From: Robert Walker @ 2019-04-29 14:53 UTC (permalink / raw)
To: Leo Yan, Arnaldo Carvalho de Melo, Mathieu Poirier,
Suzuki K Poulose, Mike Leach, Alexander Shishkin, Jiri Olsa,
Namhyung Kim, linux-arm-kernel, linux-kernel
On 28/04/2019 09:32, Leo Yan wrote:
> Since cs_etm_queue::prev_packet is allocated for all cases, it will
> never be NULL pointer; now validity checking prev_packet is pointless,
> remove all of them.
>
> Signed-off-by: Leo Yan <leo.yan@linaro.org>
> ---
> tools/perf/util/cs-etm.c | 6 +-----
> 1 file changed, 1 insertion(+), 5 deletions(-)
Tested-by: Robert Walker <robert.walker@arm.com>
Regards
Rob
> diff --git a/tools/perf/util/cs-etm.c b/tools/perf/util/cs-etm.c
> index 054b480aab04..de488b43f440 100644
> --- a/tools/perf/util/cs-etm.c
> +++ b/tools/perf/util/cs-etm.c
> @@ -979,7 +979,6 @@ static int cs_etm__sample(struct cs_etm_queue *etmq)
> * PREV_PACKET is a branch.
> */
> if (etm->synth_opts.last_branch &&
> - etmq->prev_packet &&
> etmq->prev_packet->sample_type == CS_ETM_RANGE &&
> etmq->prev_packet->last_instr_taken_branch)
> cs_etm__update_last_branch_rb(etmq);
> @@ -1012,7 +1011,7 @@ static int cs_etm__sample(struct cs_etm_queue *etmq)
> etmq->period_instructions = instrs_over;
> }
>
> - if (etm->sample_branches && etmq->prev_packet) {
> + if (etm->sample_branches) {
> bool generate_sample = false;
>
> /* Generate sample for tracing on packet */
> @@ -1069,9 +1068,6 @@ static int cs_etm__flush(struct cs_etm_queue *etmq)
> struct cs_etm_auxtrace *etm = etmq->etm;
> struct cs_etm_packet *tmp;
>
> - if (!etmq->prev_packet)
> - return 0;
> -
> /* Handle start tracing packet */
> if (etmq->prev_packet->sample_type == CS_ETM_EMPTY)
> goto swap_packet;
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 12+ messages in thread
* [tip:perf/urgent] perf cs-etm: Don't check cs_etm_queue::prev_packet validity
2019-04-28 8:32 ` Leo Yan
(?)
(?)
@ 2019-05-03 5:56 ` tip-bot for Leo Yan
-1 siblings, 0 replies; 12+ messages in thread
From: tip-bot for Leo Yan @ 2019-05-03 5:56 UTC (permalink / raw)
To: linux-tip-commits
Cc: jolsa, leo.yan, acme, hpa, linux-kernel, mathieu.poirier,
alexander.shishkin, suzuki.poulose, tglx, mingo, namhyung,
robert.walker, mike.leach
Commit-ID: cf0c37b6dbf74fb71bea07b516612d29e00dcbc4
Gitweb: https://git.kernel.org/tip/cf0c37b6dbf74fb71bea07b516612d29e00dcbc4
Author: Leo Yan <leo.yan@linaro.org>
AuthorDate: Sun, 28 Apr 2019 16:32:28 +0800
Committer: Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Thu, 2 May 2019 16:00:20 -0400
perf cs-etm: Don't check cs_etm_queue::prev_packet validity
Since cs_etm_queue::prev_packet is allocated for all cases, it will
never be NULL pointer; now validity checking prev_packet is pointless,
remove all of them.
Signed-off-by: Leo Yan <leo.yan@linaro.org>
Tested-by: Robert Walker <robert.walker@arm.com>
Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Mathieu Poirier <mathieu.poirier@linaro.org>
Cc: Mike Leach <mike.leach@linaro.org>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Suzuki K Poulouse <suzuki.poulose@arm.com>
Cc: linux-arm-kernel@lists.infradead.org
Link: http://lkml.kernel.org/r/20190428083228.20246-2-leo.yan@linaro.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
tools/perf/util/cs-etm.c | 6 +-----
1 file changed, 1 insertion(+), 5 deletions(-)
diff --git a/tools/perf/util/cs-etm.c b/tools/perf/util/cs-etm.c
index 110804936fc3..7777cfc1ad8c 100644
--- a/tools/perf/util/cs-etm.c
+++ b/tools/perf/util/cs-etm.c
@@ -981,7 +981,6 @@ static int cs_etm__sample(struct cs_etm_queue *etmq)
* PREV_PACKET is a branch.
*/
if (etm->synth_opts.last_branch &&
- etmq->prev_packet &&
etmq->prev_packet->sample_type == CS_ETM_RANGE &&
etmq->prev_packet->last_instr_taken_branch)
cs_etm__update_last_branch_rb(etmq);
@@ -1014,7 +1013,7 @@ static int cs_etm__sample(struct cs_etm_queue *etmq)
etmq->period_instructions = instrs_over;
}
- if (etm->sample_branches && etmq->prev_packet) {
+ if (etm->sample_branches) {
bool generate_sample = false;
/* Generate sample for tracing on packet */
@@ -1071,9 +1070,6 @@ static int cs_etm__flush(struct cs_etm_queue *etmq)
struct cs_etm_auxtrace *etm = etmq->etm;
struct cs_etm_packet *tmp;
- if (!etmq->prev_packet)
- return 0;
-
/* Handle start tracing packet */
if (etmq->prev_packet->sample_type == CS_ETM_EMPTY)
goto swap_packet;
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH v1 1/2] perf cs-etm: Always allocate memory for cs_etm_queue::prev_packet
2019-04-28 8:32 ` Leo Yan
@ 2019-04-29 14:53 ` Robert Walker
-1 siblings, 0 replies; 12+ messages in thread
From: Robert Walker @ 2019-04-29 14:53 UTC (permalink / raw)
To: Leo Yan, Arnaldo Carvalho de Melo, Mathieu Poirier,
Suzuki K Poulose, Mike Leach, Alexander Shishkin, Jiri Olsa,
Namhyung Kim, linux-arm-kernel, linux-kernel
Hi,
On 28/04/2019 09:32, Leo Yan wrote:
> Robert Walker reported a segmentation fault is observed when process
> CoreSight trace data; this issue can be easily reproduced by the
> command 'perf report --itrace=i1000i' for decoding tracing data.
>
> If neither the 'b' flag (synthesize branches events) nor 'l' flag
> (synthesize last branch entries) are specified to option '--itrace',
> cs_etm_queue::prev_packet will not been initialised. After merging
> the code to support exception packets and sample flags, there
> introduced a number of uses of cs_etm_queue::prev_packet without
> checking whether it is valid, for these cases any accessing to
> uninitialised prev_packet will cause crash.
>
> As cs_etm_queue::prev_packet is used more widely now and it's already
> hard to follow which functions have been called in a context where the
> validity of cs_etm_queue::prev_packet has been checked, this patch
> always allocates memory for cs_etm_queue::prev_packet.
>
> Reported-by: Robert Walker <robert.walker@arm.com>
> Suggested-by: Robert Walker <robert.walker@arm.com>
> Fixes: 7100b12cf474 ("perf cs-etm: Generate branch sample for exception packet")
> Fixes: 24fff5eb2b93 ("perf cs-etm: Avoid stale branch samples when flush packet")
> Signed-off-by: Leo Yan <leo.yan@linaro.org>
> ---
> tools/perf/util/cs-etm.c | 8 +++-----
> 1 file changed, 3 insertions(+), 5 deletions(-)
I've tested these with the trace from the HiKey960 and the segfault no
longer occurs
Tested-by: Robert Walker <robert.walker@arm.com>
Regards
Rob
>
> diff --git a/tools/perf/util/cs-etm.c b/tools/perf/util/cs-etm.c
> index 110804936fc3..054b480aab04 100644
> --- a/tools/perf/util/cs-etm.c
> +++ b/tools/perf/util/cs-etm.c
> @@ -422,11 +422,9 @@ static struct cs_etm_queue *cs_etm__alloc_queue(struct cs_etm_auxtrace *etm)
> if (!etmq->packet)
> goto out_free;
>
> - if (etm->synth_opts.last_branch || etm->sample_branches) {
> - etmq->prev_packet = zalloc(szp);
> - if (!etmq->prev_packet)
> - goto out_free;
> - }
> + etmq->prev_packet = zalloc(szp);
> + if (!etmq->prev_packet)
> + goto out_free;
>
> if (etm->synth_opts.last_branch) {
> size_t sz = sizeof(struct branch_stack);
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v1 1/2] perf cs-etm: Always allocate memory for cs_etm_queue::prev_packet
@ 2019-04-29 14:53 ` Robert Walker
0 siblings, 0 replies; 12+ messages in thread
From: Robert Walker @ 2019-04-29 14:53 UTC (permalink / raw)
To: Leo Yan, Arnaldo Carvalho de Melo, Mathieu Poirier,
Suzuki K Poulose, Mike Leach, Alexander Shishkin, Jiri Olsa,
Namhyung Kim, linux-arm-kernel, linux-kernel
Hi,
On 28/04/2019 09:32, Leo Yan wrote:
> Robert Walker reported a segmentation fault is observed when process
> CoreSight trace data; this issue can be easily reproduced by the
> command 'perf report --itrace=i1000i' for decoding tracing data.
>
> If neither the 'b' flag (synthesize branches events) nor 'l' flag
> (synthesize last branch entries) are specified to option '--itrace',
> cs_etm_queue::prev_packet will not been initialised. After merging
> the code to support exception packets and sample flags, there
> introduced a number of uses of cs_etm_queue::prev_packet without
> checking whether it is valid, for these cases any accessing to
> uninitialised prev_packet will cause crash.
>
> As cs_etm_queue::prev_packet is used more widely now and it's already
> hard to follow which functions have been called in a context where the
> validity of cs_etm_queue::prev_packet has been checked, this patch
> always allocates memory for cs_etm_queue::prev_packet.
>
> Reported-by: Robert Walker <robert.walker@arm.com>
> Suggested-by: Robert Walker <robert.walker@arm.com>
> Fixes: 7100b12cf474 ("perf cs-etm: Generate branch sample for exception packet")
> Fixes: 24fff5eb2b93 ("perf cs-etm: Avoid stale branch samples when flush packet")
> Signed-off-by: Leo Yan <leo.yan@linaro.org>
> ---
> tools/perf/util/cs-etm.c | 8 +++-----
> 1 file changed, 3 insertions(+), 5 deletions(-)
I've tested these with the trace from the HiKey960 and the segfault no
longer occurs
Tested-by: Robert Walker <robert.walker@arm.com>
Regards
Rob
>
> diff --git a/tools/perf/util/cs-etm.c b/tools/perf/util/cs-etm.c
> index 110804936fc3..054b480aab04 100644
> --- a/tools/perf/util/cs-etm.c
> +++ b/tools/perf/util/cs-etm.c
> @@ -422,11 +422,9 @@ static struct cs_etm_queue *cs_etm__alloc_queue(struct cs_etm_auxtrace *etm)
> if (!etmq->packet)
> goto out_free;
>
> - if (etm->synth_opts.last_branch || etm->sample_branches) {
> - etmq->prev_packet = zalloc(szp);
> - if (!etmq->prev_packet)
> - goto out_free;
> - }
> + etmq->prev_packet = zalloc(szp);
> + if (!etmq->prev_packet)
> + goto out_free;
>
> if (etm->synth_opts.last_branch) {
> size_t sz = sizeof(struct branch_stack);
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v1 1/2] perf cs-etm: Always allocate memory for cs_etm_queue::prev_packet
2019-04-28 8:32 ` Leo Yan
@ 2019-04-30 1:07 ` Arnaldo Carvalho de Melo
-1 siblings, 0 replies; 12+ messages in thread
From: Arnaldo Carvalho de Melo @ 2019-04-30 1:07 UTC (permalink / raw)
To: Leo Yan
Cc: Mathieu Poirier, Robert Walker, Suzuki K Poulose, Mike Leach,
Alexander Shishkin, Jiri Olsa, Namhyung Kim, linux-arm-kernel,
linux-kernel
Em Sun, Apr 28, 2019 at 04:32:27PM +0800, Leo Yan escreveu:
> Robert Walker reported a segmentation fault is observed when process
> CoreSight trace data; this issue can be easily reproduced by the
> command 'perf report --itrace=i1000i' for decoding tracing data.
>
> If neither the 'b' flag (synthesize branches events) nor 'l' flag
> (synthesize last branch entries) are specified to option '--itrace',
> cs_etm_queue::prev_packet will not been initialised. After merging
> the code to support exception packets and sample flags, there
> introduced a number of uses of cs_etm_queue::prev_packet without
> checking whether it is valid, for these cases any accessing to
> uninitialised prev_packet will cause crash.
>
> As cs_etm_queue::prev_packet is used more widely now and it's already
> hard to follow which functions have been called in a context where the
> validity of cs_etm_queue::prev_packet has been checked, this patch
> always allocates memory for cs_etm_queue::prev_packet.
>
> Reported-by: Robert Walker <robert.walker@arm.com>
> Suggested-by: Robert Walker <robert.walker@arm.com>
> Fixes: 7100b12cf474 ("perf cs-etm: Generate branch sample for exception packet")
Thanks, applied both to perf/urgent, testing them now in the containers.
- Arnaldo
> Fixes: 24fff5eb2b93 ("perf cs-etm: Avoid stale branch samples when flush packet")
> Signed-off-by: Leo Yan <leo.yan@linaro.org>
> ---
> tools/perf/util/cs-etm.c | 8 +++-----
> 1 file changed, 3 insertions(+), 5 deletions(-)
>
> diff --git a/tools/perf/util/cs-etm.c b/tools/perf/util/cs-etm.c
> index 110804936fc3..054b480aab04 100644
> --- a/tools/perf/util/cs-etm.c
> +++ b/tools/perf/util/cs-etm.c
> @@ -422,11 +422,9 @@ static struct cs_etm_queue *cs_etm__alloc_queue(struct cs_etm_auxtrace *etm)
> if (!etmq->packet)
> goto out_free;
>
> - if (etm->synth_opts.last_branch || etm->sample_branches) {
> - etmq->prev_packet = zalloc(szp);
> - if (!etmq->prev_packet)
> - goto out_free;
> - }
> + etmq->prev_packet = zalloc(szp);
> + if (!etmq->prev_packet)
> + goto out_free;
>
> if (etm->synth_opts.last_branch) {
> size_t sz = sizeof(struct branch_stack);
> --
> 2.17.1
--
- Arnaldo
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v1 1/2] perf cs-etm: Always allocate memory for cs_etm_queue::prev_packet
@ 2019-04-30 1:07 ` Arnaldo Carvalho de Melo
0 siblings, 0 replies; 12+ messages in thread
From: Arnaldo Carvalho de Melo @ 2019-04-30 1:07 UTC (permalink / raw)
To: Leo Yan
Cc: Mathieu Poirier, Suzuki K Poulose, Alexander Shishkin,
linux-kernel, Namhyung Kim, Robert Walker, Jiri Olsa,
linux-arm-kernel, Mike Leach
Em Sun, Apr 28, 2019 at 04:32:27PM +0800, Leo Yan escreveu:
> Robert Walker reported a segmentation fault is observed when process
> CoreSight trace data; this issue can be easily reproduced by the
> command 'perf report --itrace=i1000i' for decoding tracing data.
>
> If neither the 'b' flag (synthesize branches events) nor 'l' flag
> (synthesize last branch entries) are specified to option '--itrace',
> cs_etm_queue::prev_packet will not been initialised. After merging
> the code to support exception packets and sample flags, there
> introduced a number of uses of cs_etm_queue::prev_packet without
> checking whether it is valid, for these cases any accessing to
> uninitialised prev_packet will cause crash.
>
> As cs_etm_queue::prev_packet is used more widely now and it's already
> hard to follow which functions have been called in a context where the
> validity of cs_etm_queue::prev_packet has been checked, this patch
> always allocates memory for cs_etm_queue::prev_packet.
>
> Reported-by: Robert Walker <robert.walker@arm.com>
> Suggested-by: Robert Walker <robert.walker@arm.com>
> Fixes: 7100b12cf474 ("perf cs-etm: Generate branch sample for exception packet")
Thanks, applied both to perf/urgent, testing them now in the containers.
- Arnaldo
> Fixes: 24fff5eb2b93 ("perf cs-etm: Avoid stale branch samples when flush packet")
> Signed-off-by: Leo Yan <leo.yan@linaro.org>
> ---
> tools/perf/util/cs-etm.c | 8 +++-----
> 1 file changed, 3 insertions(+), 5 deletions(-)
>
> diff --git a/tools/perf/util/cs-etm.c b/tools/perf/util/cs-etm.c
> index 110804936fc3..054b480aab04 100644
> --- a/tools/perf/util/cs-etm.c
> +++ b/tools/perf/util/cs-etm.c
> @@ -422,11 +422,9 @@ static struct cs_etm_queue *cs_etm__alloc_queue(struct cs_etm_auxtrace *etm)
> if (!etmq->packet)
> goto out_free;
>
> - if (etm->synth_opts.last_branch || etm->sample_branches) {
> - etmq->prev_packet = zalloc(szp);
> - if (!etmq->prev_packet)
> - goto out_free;
> - }
> + etmq->prev_packet = zalloc(szp);
> + if (!etmq->prev_packet)
> + goto out_free;
>
> if (etm->synth_opts.last_branch) {
> size_t sz = sizeof(struct branch_stack);
> --
> 2.17.1
--
- Arnaldo
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 12+ messages in thread
* [tip:perf/urgent] perf cs-etm: Always allocate memory for cs_etm_queue::prev_packet
2019-04-28 8:32 ` Leo Yan
` (3 preceding siblings ...)
(?)
@ 2019-05-03 5:57 ` tip-bot for Leo Yan
-1 siblings, 0 replies; 12+ messages in thread
From: tip-bot for Leo Yan @ 2019-05-03 5:57 UTC (permalink / raw)
To: linux-tip-commits
Cc: acme, tglx, jolsa, leo.yan, robert.walker, linux-kernel,
suzuki.poulose, alexander.shishkin, hpa, mike.leach, namhyung,
mingo, mathieu.poirier
Commit-ID: 35bb59c10a6d0578806dd500477dae9cb4be344e
Gitweb: https://git.kernel.org/tip/35bb59c10a6d0578806dd500477dae9cb4be344e
Author: Leo Yan <leo.yan@linaro.org>
AuthorDate: Sun, 28 Apr 2019 16:32:27 +0800
Committer: Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Thu, 2 May 2019 16:00:20 -0400
perf cs-etm: Always allocate memory for cs_etm_queue::prev_packet
Robert Walker reported a segmentation fault is observed when process
CoreSight trace data; this issue can be easily reproduced by the command
'perf report --itrace=i1000i' for decoding tracing data.
If neither the 'b' flag (synthesize branches events) nor 'l' flag
(synthesize last branch entries) are specified to option '--itrace',
cs_etm_queue::prev_packet will not been initialised. After merging the
code to support exception packets and sample flags, there introduced a
number of uses of cs_etm_queue::prev_packet without checking whether it
is valid, for these cases any accessing to uninitialised prev_packet
will cause crash.
As cs_etm_queue::prev_packet is used more widely now and it's already
hard to follow which functions have been called in a context where the
validity of cs_etm_queue::prev_packet has been checked, this patch
always allocates memory for cs_etm_queue::prev_packet.
Reported-by: Robert Walker <robert.walker@arm.com>
Suggested-by: Robert Walker <robert.walker@arm.com>
Signed-off-by: Leo Yan <leo.yan@linaro.org>
Tested-by: Robert Walker <robert.walker@arm.com>
Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Mathieu Poirier <mathieu.poirier@linaro.org>
Cc: Mike Leach <mike.leach@linaro.org>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Suzuki K Poulouse <suzuki.poulose@arm.com>
Cc: linux-arm-kernel@lists.infradead.org
Fixes: 7100b12cf474 ("perf cs-etm: Generate branch sample for exception packet")
Fixes: 24fff5eb2b93 ("perf cs-etm: Avoid stale branch samples when flush packet")
Link: http://lkml.kernel.org/r/20190428083228.20246-1-leo.yan@linaro.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
tools/perf/util/cs-etm.c | 8 +++-----
1 file changed, 3 insertions(+), 5 deletions(-)
diff --git a/tools/perf/util/cs-etm.c b/tools/perf/util/cs-etm.c
index 7777cfc1ad8c..de488b43f440 100644
--- a/tools/perf/util/cs-etm.c
+++ b/tools/perf/util/cs-etm.c
@@ -422,11 +422,9 @@ static struct cs_etm_queue *cs_etm__alloc_queue(struct cs_etm_auxtrace *etm)
if (!etmq->packet)
goto out_free;
- if (etm->synth_opts.last_branch || etm->sample_branches) {
- etmq->prev_packet = zalloc(szp);
- if (!etmq->prev_packet)
- goto out_free;
- }
+ etmq->prev_packet = zalloc(szp);
+ if (!etmq->prev_packet)
+ goto out_free;
if (etm->synth_opts.last_branch) {
size_t sz = sizeof(struct branch_stack);
^ permalink raw reply related [flat|nested] 12+ messages in thread