linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/2] Coresight ETF NULL pointer dereference and ETM save/restore fixes
@ 2020-09-27 16:20 Sai Prakash Ranjan
  2020-09-27 16:20 ` [RFC PATCH 1/2] coresight: tmc-etf: Fix NULL pointer dereference in tmc_enable_etf_sink_perf() Sai Prakash Ranjan
  2020-09-27 16:20 ` [PATCH 2/2] coresight: etm4x: Fix save and restore of TRCVMIDCCTLR1 register Sai Prakash Ranjan
  0 siblings, 2 replies; 5+ messages in thread
From: Sai Prakash Ranjan @ 2020-09-27 16:20 UTC (permalink / raw)
  To: Mathieu Poirier, Suzuki K Poulose, Mike Leach, Leo Yan
  Cc: Sai Prakash Ranjan, Alexander Shishkin, linux-arm-msm, coresight,
	linux-kernel, Stephen Boyd, Peter Zijlstra, denik,
	linux-arm-kernel

This 2 patch series provides fixes to ETF null pointer dereference crash
and TRCVMIDCCTLR1 register save and restore fix.

Patch 1 is an RFC since I am not sure of the fix provided since it looks
more like a band-aid than the actual fix.

Sai Prakash Ranjan (2):
  coresight: tmc-etf: Fix NULL pointer dereference in
    tmc_enable_etf_sink_perf()
  coresight: etm4x: Fix save and restore of TRCVMIDCCTLR1 register

 drivers/hwtracing/coresight/coresight-etm4x-core.c | 4 ++--
 drivers/hwtracing/coresight/coresight-tmc-etf.c    | 3 +++
 2 files changed, 5 insertions(+), 2 deletions(-)


base-commit: e209e73bee253afe969410150248f0c300c13d84
-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation


_______________________________________________
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] 5+ messages in thread

* [RFC PATCH 1/2] coresight: tmc-etf: Fix NULL pointer dereference in tmc_enable_etf_sink_perf()
  2020-09-27 16:20 [PATCH 0/2] Coresight ETF NULL pointer dereference and ETM save/restore fixes Sai Prakash Ranjan
@ 2020-09-27 16:20 ` Sai Prakash Ranjan
  2020-09-27 16:20 ` [PATCH 2/2] coresight: etm4x: Fix save and restore of TRCVMIDCCTLR1 register Sai Prakash Ranjan
  1 sibling, 0 replies; 5+ messages in thread
From: Sai Prakash Ranjan @ 2020-09-27 16:20 UTC (permalink / raw)
  To: Mathieu Poirier, Suzuki K Poulose, Mike Leach, Leo Yan
  Cc: Sai Prakash Ranjan, Alexander Shishkin, linux-arm-msm, coresight,
	linux-kernel, Stephen Boyd, Peter Zijlstra, denik,
	linux-arm-kernel

There was a report of NULL pointer dereference in ETF enable
path for perf CS mode with PID. It is almost 100% reproducible
when the process to monitor is something very active such as
chrome and only with ETF as the sink. Currently in a bid to
find the pid, the owner is dereferenced via task_pid_nr() call
in tmc_enable_etf_sink_perf(). With owner being NULL, we get a
NULL pointer dereference, so check the owner before dereferencing
it to prevent the system crash.

 perf record -e cs_etm/@tmc_etf0/ -N -p <pid>

Unable to handle kernel NULL pointer dereference at virtual address 0000000000000548
Mem abort info:
  ESR = 0x96000006
  EC = 0x25: DABT (current EL), IL = 32 bits
  SET = 0, FnV = 0
  EA = 0, S1PTW = 0
Data abort info:
  ISV = 0, ISS = 0x00000006
  CM = 0, WnR = 0

Call trace:
 tmc_enable_etf_sink+0xe4/0x280
 coresight_enable_path+0x168/0x1fc
 etm_event_start+0x8c/0xf8
 etm_event_add+0x38/0x54
 event_sched_in+0x194/0x2ac
 group_sched_in+0x54/0x12c
 flexible_sched_in+0xd8/0x120
 visit_groups_merge+0x100/0x16c
 ctx_flexible_sched_in+0x50/0x74
 ctx_sched_in+0xa4/0xa8
 perf_event_sched_in+0x60/0x6c
 perf_event_context_sched_in+0x98/0xe0
 __perf_event_task_sched_in+0x5c/0xd8
 finish_task_switch+0x184/0x1cc
 schedule_tail+0x20/0xec
 ret_from_fork+0x4/0x18

Signed-off-by: Sai Prakash Ranjan <saiprakash.ranjan@codeaurora.org>
---

I am not sure of this incomplete solution hence the RFC. This issue was also
reported when this code was first added [1] but somehow it didn't get much
notice at the time. So the NULL pointer is propagated from as far as
flexible_sched_in() (might even be earlier than this) in events core and
deferenced in ETF code where it crashes. So I am not sure if its a problem
with the core code or the etf driver. Plus it is not reproducible with all
the processes, just something which is quite active ones such as chrome.
This is with 5.4 kernel with all the coresight patches backported, I did
go through events/core code from latest kernel to see if we are missing
any fixes related to this but I couldn't find any so I believe this problem
should also exist on latest kernel as well.

[1] https://lists.linaro.org/pipermail/coresight/2019-March/002278.html 
---
 drivers/hwtracing/coresight/coresight-tmc-etf.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/hwtracing/coresight/coresight-tmc-etf.c b/drivers/hwtracing/coresight/coresight-tmc-etf.c
index 44402d413ebb..32f141d943ca 100644
--- a/drivers/hwtracing/coresight/coresight-tmc-etf.c
+++ b/drivers/hwtracing/coresight/coresight-tmc-etf.c
@@ -242,6 +242,9 @@ static int tmc_enable_etf_sink_perf(struct coresight_device *csdev, void *data)
 			break;
 		}
 
+		if (!handle->event->owner)
+			break;
+
 		/* Get a handle on the pid of the process to monitor */
 		pid = task_pid_nr(handle->event->owner);
 
-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation


_______________________________________________
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] 5+ messages in thread

* [PATCH 2/2] coresight: etm4x: Fix save and restore of TRCVMIDCCTLR1 register
  2020-09-27 16:20 [PATCH 0/2] Coresight ETF NULL pointer dereference and ETM save/restore fixes Sai Prakash Ranjan
  2020-09-27 16:20 ` [RFC PATCH 1/2] coresight: tmc-etf: Fix NULL pointer dereference in tmc_enable_etf_sink_perf() Sai Prakash Ranjan
@ 2020-09-27 16:20 ` Sai Prakash Ranjan
  2020-09-28 11:05   ` Suzuki K Poulose
  1 sibling, 1 reply; 5+ messages in thread
From: Sai Prakash Ranjan @ 2020-09-27 16:20 UTC (permalink / raw)
  To: Mathieu Poirier, Suzuki K Poulose, Mike Leach, Leo Yan
  Cc: Sai Prakash Ranjan, Alexander Shishkin, linux-arm-msm, coresight,
	linux-kernel, Stephen Boyd, Peter Zijlstra, denik,
	linux-arm-kernel

In commit f188b5e76aae ("coresight: etm4x: Save/restore state
across CPU low power states"), mistakenly TRCVMIDCCTLR1 register
value was saved in trcvmidcctlr0 state variable which is used to
store TRCVMIDCCTLR0 register value in etm4x_cpu_save() and then
same value is written back to both TRCVMIDCCTLR0 and TRCVMIDCCTLR1
in etm4x_cpu_restore(). There is already a trcvmidcctlr1 state
variable available for TRCVMIDCCTLR1, so use it.

Fixes: 8b44fdfef6a2 ("coresight: etm4x: Allow etm4x to be built as a module")
Fixes: f188b5e76aae ("coresight: etm4x: Save/restore state across CPU low power states")
Signed-off-by: Sai Prakash Ranjan <saiprakash.ranjan@codeaurora.org>
---
 drivers/hwtracing/coresight/coresight-etm4x-core.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c b/drivers/hwtracing/coresight/coresight-etm4x-core.c
index de76d57850bc..abd706b216ac 100644
--- a/drivers/hwtracing/coresight/coresight-etm4x-core.c
+++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c
@@ -1243,7 +1243,7 @@ static int etm4_cpu_save(struct etmv4_drvdata *drvdata)
 	state->trccidcctlr1 = readl(drvdata->base + TRCCIDCCTLR1);
 
 	state->trcvmidcctlr0 = readl(drvdata->base + TRCVMIDCCTLR0);
-	state->trcvmidcctlr0 = readl(drvdata->base + TRCVMIDCCTLR1);
+	state->trcvmidcctlr1 = readl(drvdata->base + TRCVMIDCCTLR1);
 
 	state->trcclaimset = readl(drvdata->base + TRCCLAIMCLR);
 
@@ -1353,7 +1353,7 @@ static void etm4_cpu_restore(struct etmv4_drvdata *drvdata)
 	writel_relaxed(state->trccidcctlr1, drvdata->base + TRCCIDCCTLR1);
 
 	writel_relaxed(state->trcvmidcctlr0, drvdata->base + TRCVMIDCCTLR0);
-	writel_relaxed(state->trcvmidcctlr0, drvdata->base + TRCVMIDCCTLR1);
+	writel_relaxed(state->trcvmidcctlr1, drvdata->base + TRCVMIDCCTLR1);
 
 	writel_relaxed(state->trcclaimset, drvdata->base + TRCCLAIMSET);
 
-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation


_______________________________________________
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] 5+ messages in thread

* Re: [PATCH 2/2] coresight: etm4x: Fix save and restore of TRCVMIDCCTLR1 register
  2020-09-27 16:20 ` [PATCH 2/2] coresight: etm4x: Fix save and restore of TRCVMIDCCTLR1 register Sai Prakash Ranjan
@ 2020-09-28 11:05   ` Suzuki K Poulose
  2020-09-28 11:22     ` Sai Prakash Ranjan
  0 siblings, 1 reply; 5+ messages in thread
From: Suzuki K Poulose @ 2020-09-28 11:05 UTC (permalink / raw)
  To: saiprakash.ranjan, mathieu.poirier, mike.leach, leo.yan
  Cc: alexander.shishkin, linux-arm-msm, coresight, linux-kernel,
	swboyd, peterz, denik, linux-arm-kernel

Hi Sai,

On 09/27/2020 05:20 PM, Sai Prakash Ranjan wrote:
> In commit f188b5e76aae ("coresight: etm4x: Save/restore state
> across CPU low power states"), mistakenly TRCVMIDCCTLR1 register
> value was saved in trcvmidcctlr0 state variable which is used to
> store TRCVMIDCCTLR0 register value in etm4x_cpu_save() and then
> same value is written back to both TRCVMIDCCTLR0 and TRCVMIDCCTLR1
> in etm4x_cpu_restore(). There is already a trcvmidcctlr1 state
> variable available for TRCVMIDCCTLR1, so use it.
> 
> Fixes: 8b44fdfef6a2 ("coresight: etm4x: Allow etm4x to be built as a module")

Why is this commit in question ?

> Fixes: f188b5e76aae ("coresight: etm4x: Save/restore state across CPU low power states")

I believe this is the right fixes tag.

> Signed-off-by: Sai Prakash Ranjan <saiprakash.ranjan@codeaurora.org>
> ---
>   drivers/hwtracing/coresight/coresight-etm4x-core.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c b/drivers/hwtracing/coresight/coresight-etm4x-core.c
> index de76d57850bc..abd706b216ac 100644
> --- a/drivers/hwtracing/coresight/coresight-etm4x-core.c
> +++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c
> @@ -1243,7 +1243,7 @@ static int etm4_cpu_save(struct etmv4_drvdata *drvdata)
>   	state->trccidcctlr1 = readl(drvdata->base + TRCCIDCCTLR1);
>   
>   	state->trcvmidcctlr0 = readl(drvdata->base + TRCVMIDCCTLR0);
> -	state->trcvmidcctlr0 = readl(drvdata->base + TRCVMIDCCTLR1);
> +	state->trcvmidcctlr1 = readl(drvdata->base + TRCVMIDCCTLR1);
>   
>   	state->trcclaimset = readl(drvdata->base + TRCCLAIMCLR);
>   
> @@ -1353,7 +1353,7 @@ static void etm4_cpu_restore(struct etmv4_drvdata *drvdata)
>   	writel_relaxed(state->trccidcctlr1, drvdata->base + TRCCIDCCTLR1);
>   
>   	writel_relaxed(state->trcvmidcctlr0, drvdata->base + TRCVMIDCCTLR0);
> -	writel_relaxed(state->trcvmidcctlr0, drvdata->base + TRCVMIDCCTLR1);
> +	writel_relaxed(state->trcvmidcctlr1, drvdata->base + TRCVMIDCCTLR1);
>   

Reviewed-by: Suzuki K Poulose <suzuki.poulose@arm.com>

_______________________________________________
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] 5+ messages in thread

* Re: [PATCH 2/2] coresight: etm4x: Fix save and restore of TRCVMIDCCTLR1 register
  2020-09-28 11:05   ` Suzuki K Poulose
@ 2020-09-28 11:22     ` Sai Prakash Ranjan
  0 siblings, 0 replies; 5+ messages in thread
From: Sai Prakash Ranjan @ 2020-09-28 11:22 UTC (permalink / raw)
  To: Suzuki K Poulose
  Cc: mathieu.poirier, alexander.shishkin, linux-arm-msm, coresight,
	linux-kernel, swboyd, peterz, denik, leo.yan, linux-arm-kernel,
	mike.leach

Hi Suzuki,

On 2020-09-28 16:35, Suzuki K Poulose wrote:
> Hi Sai,
> 
> On 09/27/2020 05:20 PM, Sai Prakash Ranjan wrote:
>> In commit f188b5e76aae ("coresight: etm4x: Save/restore state
>> across CPU low power states"), mistakenly TRCVMIDCCTLR1 register
>> value was saved in trcvmidcctlr0 state variable which is used to
>> store TRCVMIDCCTLR0 register value in etm4x_cpu_save() and then
>> same value is written back to both TRCVMIDCCTLR0 and TRCVMIDCCTLR1
>> in etm4x_cpu_restore(). There is already a trcvmidcctlr1 state
>> variable available for TRCVMIDCCTLR1, so use it.
>> 
>> Fixes: 8b44fdfef6a2 ("coresight: etm4x: Allow etm4x to be built as a 
>> module")
> 
> Why is this commit in question ?

My bad sorry, I thought there are two commits which touch this hunk of 
code,
but I see now that the module code just renamed the file, so this fixes 
tag
is not required.

> 
>> Fixes: f188b5e76aae ("coresight: etm4x: Save/restore state across CPU 
>> low power states")
> 
> I believe this is the right fixes tag.
> 

Yes, I will resend with only this fixes tag.

>> Signed-off-by: Sai Prakash Ranjan <saiprakash.ranjan@codeaurora.org>
>> ---
>>   drivers/hwtracing/coresight/coresight-etm4x-core.c | 4 ++--
>>   1 file changed, 2 insertions(+), 2 deletions(-)
>> 
>> diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c 
>> b/drivers/hwtracing/coresight/coresight-etm4x-core.c
>> index de76d57850bc..abd706b216ac 100644
>> --- a/drivers/hwtracing/coresight/coresight-etm4x-core.c
>> +++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c
>> @@ -1243,7 +1243,7 @@ static int etm4_cpu_save(struct etmv4_drvdata 
>> *drvdata)
>>   	state->trccidcctlr1 = readl(drvdata->base + TRCCIDCCTLR1);
>>     	state->trcvmidcctlr0 = readl(drvdata->base + TRCVMIDCCTLR0);
>> -	state->trcvmidcctlr0 = readl(drvdata->base + TRCVMIDCCTLR1);
>> +	state->trcvmidcctlr1 = readl(drvdata->base + TRCVMIDCCTLR1);
>>     	state->trcclaimset = readl(drvdata->base + TRCCLAIMCLR);
>>   @@ -1353,7 +1353,7 @@ static void etm4_cpu_restore(struct 
>> etmv4_drvdata *drvdata)
>>   	writel_relaxed(state->trccidcctlr1, drvdata->base + TRCCIDCCTLR1);
>>     	writel_relaxed(state->trcvmidcctlr0, drvdata->base + 
>> TRCVMIDCCTLR0);
>> -	writel_relaxed(state->trcvmidcctlr0, drvdata->base + TRCVMIDCCTLR1);
>> +	writel_relaxed(state->trcvmidcctlr1, drvdata->base + TRCVMIDCCTLR1);
>> 
> 
> Reviewed-by: Suzuki K Poulose <suzuki.poulose@arm.com>

Thanks,
Sai

-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a 
member
of Code Aurora Forum, hosted by The Linux Foundation

_______________________________________________
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] 5+ messages in thread

end of thread, other threads:[~2020-09-28 11:24 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-09-27 16:20 [PATCH 0/2] Coresight ETF NULL pointer dereference and ETM save/restore fixes Sai Prakash Ranjan
2020-09-27 16:20 ` [RFC PATCH 1/2] coresight: tmc-etf: Fix NULL pointer dereference in tmc_enable_etf_sink_perf() Sai Prakash Ranjan
2020-09-27 16:20 ` [PATCH 2/2] coresight: etm4x: Fix save and restore of TRCVMIDCCTLR1 register Sai Prakash Ranjan
2020-09-28 11:05   ` Suzuki K Poulose
2020-09-28 11:22     ` Sai Prakash Ranjan

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).