From: Sai Prakash Ranjan <saiprakash.ranjan@codeaurora.org> To: Mathieu Poirier <mathieu.poirier@linaro.org>, Suzuki K Poulose <suzuki.poulose@arm.com>, Mike Leach <mike.leach@linaro.org>, Peter Zijlstra <peterz@infradead.org>, Ingo Molnar <mingo@redhat.com>, Arnaldo Carvalho de Melo <acme@kernel.org>, Mark Rutland <mark.rutland@arm.com>, Alexander Shishkin <alexander.shishkin@linux.intel.com>, Jiri Olsa <jolsa@redhat.com>, Namhyung Kim <namhyung@kernel.org> Cc: coresight@lists.linaro.org, Stephen Boyd <swboyd@chromium.org>, linux-arm-msm@vger.kernel.org, linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org, Sai Prakash Ranjan <saiprakash.ranjan@codeaurora.org> Subject: [PATCHv2 2/4] coresight: tmc-etf: Fix NULL ptr dereference in tmc_enable_etf_sink_perf() Date: Thu, 22 Oct 2020 16:27:52 +0530 [thread overview] Message-ID: <aa6e571156d6e26e54da0bb3015ba474e4a08da0.1603363729.git.saiprakash.ranjan@codeaurora.org> (raw) In-Reply-To: <cover.1603363729.git.saiprakash.ranjan@codeaurora.org> There was a report of NULL pointer dereference in ETF enable path for perf CS mode with PID monitoring. It is almost 100% reproducible when the process to monitor is something very active such as chrome and with ETF as the sink and not ETR. Currently in a bid to find the pid, the owner is dereferenced via task_pid_nr() call in tmc_enable_etf_sink_perf() and with owner being NULL, we get a NULL pointer dereference. Looking at the ETR and other places in the kernel, ETF and the ETB are the only places trying to dereference the task(owner) in tmc_enable_etf_sink_perf() which is also called from the sched_in path as in the call trace. Owner(task) is NULL even in the case of ETR in tmc_enable_etr_sink_perf(), but since we cache the PID in alloc_buffer() callback and it is done as part of etm_setup_aux() when allocating buffer for ETR sink, we never dereference this NULL pointer and we are safe. So lets do the same thing with ETF and cache the PID to which the cs_buffer belongs in tmc_alloc_etf_buffer() as done for ETR. This will also remove the unnecessary function calls(task_pid_nr()) since we are caching the PID. In addition to this, add a check to validate event->owner which will prevent any possible NULL pointer dereferences and check for kernel events. Easily reproducible running below: 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 <snip>... 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 Fixes: 880af782c6e8 ("coresight: tmc-etf: Add support for CPU-wide trace scenarios") Suggested-by: Suzuki K Poulose <suzuki.poulose@arm.com> Signed-off-by: Sai Prakash Ranjan <saiprakash.ranjan@codeaurora.org> --- drivers/hwtracing/coresight/coresight-priv.h | 2 ++ drivers/hwtracing/coresight/coresight-tmc-etf.c | 8 +++++++- 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/drivers/hwtracing/coresight/coresight-priv.h b/drivers/hwtracing/coresight/coresight-priv.h index 65a29293b6cb..f5f654ea2994 100644 --- a/drivers/hwtracing/coresight/coresight-priv.h +++ b/drivers/hwtracing/coresight/coresight-priv.h @@ -87,6 +87,7 @@ enum cs_mode { * struct cs_buffer - keep track of a recording session' specifics * @cur: index of the current buffer * @nr_pages: max number of pages granted to us + * @pid: PID this cs_buffer belongs to * @offset: offset within the current buffer * @data_size: how much we collected in this run * @snapshot: is this run in snapshot mode @@ -95,6 +96,7 @@ enum cs_mode { struct cs_buffers { unsigned int cur; unsigned int nr_pages; + pid_t pid; unsigned long offset; local_t data_size; bool snapshot; diff --git a/drivers/hwtracing/coresight/coresight-tmc-etf.c b/drivers/hwtracing/coresight/coresight-tmc-etf.c index 44402d413ebb..86ff0dda0444 100644 --- a/drivers/hwtracing/coresight/coresight-tmc-etf.c +++ b/drivers/hwtracing/coresight/coresight-tmc-etf.c @@ -227,6 +227,7 @@ static int tmc_enable_etf_sink_perf(struct coresight_device *csdev, void *data) unsigned long flags; struct tmc_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent); struct perf_output_handle *handle = data; + struct cs_buffers *buf = etm_perf_sink_config(handle); spin_lock_irqsave(&drvdata->spinlock, flags); do { @@ -243,7 +244,7 @@ static int tmc_enable_etf_sink_perf(struct coresight_device *csdev, void *data) } /* Get a handle on the pid of the process to monitor */ - pid = task_pid_nr(handle->event->owner); + pid = buf->pid; if (drvdata->pid != -1 && drvdata->pid != pid) { ret = -EBUSY; @@ -391,6 +392,10 @@ static void *tmc_alloc_etf_buffer(struct coresight_device *csdev, { int node; struct cs_buffers *buf; + struct task_struct *task = READ_ONCE(event->owner); + + if (!task || is_kernel_event(event)) + return NULL; node = (event->cpu == -1) ? NUMA_NO_NODE : cpu_to_node(event->cpu); @@ -399,6 +404,7 @@ static void *tmc_alloc_etf_buffer(struct coresight_device *csdev, if (!buf) return NULL; + buf->pid = task_pid_nr(task); buf->snapshot = overwrite; buf->nr_pages = nr_pages; buf->data_pages = pages; -- QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation
WARNING: multiple messages have this Message-ID (diff)
From: Sai Prakash Ranjan <saiprakash.ranjan@codeaurora.org> To: Mathieu Poirier <mathieu.poirier@linaro.org>, Suzuki K Poulose <suzuki.poulose@arm.com>, Mike Leach <mike.leach@linaro.org>, Peter Zijlstra <peterz@infradead.org>, Ingo Molnar <mingo@redhat.com>, Arnaldo Carvalho de Melo <acme@kernel.org>, Mark Rutland <mark.rutland@arm.com>, Alexander Shishkin <alexander.shishkin@linux.intel.com>, Jiri Olsa <jolsa@redhat.com>, Namhyung Kim <namhyung@kernel.org> Cc: Sai Prakash Ranjan <saiprakash.ranjan@codeaurora.org>, linux-arm-msm@vger.kernel.org, coresight@lists.linaro.org, linux-kernel@vger.kernel.org, Stephen Boyd <swboyd@chromium.org>, linux-arm-kernel@lists.infradead.org Subject: [PATCHv2 2/4] coresight: tmc-etf: Fix NULL ptr dereference in tmc_enable_etf_sink_perf() Date: Thu, 22 Oct 2020 16:27:52 +0530 [thread overview] Message-ID: <aa6e571156d6e26e54da0bb3015ba474e4a08da0.1603363729.git.saiprakash.ranjan@codeaurora.org> (raw) In-Reply-To: <cover.1603363729.git.saiprakash.ranjan@codeaurora.org> There was a report of NULL pointer dereference in ETF enable path for perf CS mode with PID monitoring. It is almost 100% reproducible when the process to monitor is something very active such as chrome and with ETF as the sink and not ETR. Currently in a bid to find the pid, the owner is dereferenced via task_pid_nr() call in tmc_enable_etf_sink_perf() and with owner being NULL, we get a NULL pointer dereference. Looking at the ETR and other places in the kernel, ETF and the ETB are the only places trying to dereference the task(owner) in tmc_enable_etf_sink_perf() which is also called from the sched_in path as in the call trace. Owner(task) is NULL even in the case of ETR in tmc_enable_etr_sink_perf(), but since we cache the PID in alloc_buffer() callback and it is done as part of etm_setup_aux() when allocating buffer for ETR sink, we never dereference this NULL pointer and we are safe. So lets do the same thing with ETF and cache the PID to which the cs_buffer belongs in tmc_alloc_etf_buffer() as done for ETR. This will also remove the unnecessary function calls(task_pid_nr()) since we are caching the PID. In addition to this, add a check to validate event->owner which will prevent any possible NULL pointer dereferences and check for kernel events. Easily reproducible running below: 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 <snip>... 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 Fixes: 880af782c6e8 ("coresight: tmc-etf: Add support for CPU-wide trace scenarios") Suggested-by: Suzuki K Poulose <suzuki.poulose@arm.com> Signed-off-by: Sai Prakash Ranjan <saiprakash.ranjan@codeaurora.org> --- drivers/hwtracing/coresight/coresight-priv.h | 2 ++ drivers/hwtracing/coresight/coresight-tmc-etf.c | 8 +++++++- 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/drivers/hwtracing/coresight/coresight-priv.h b/drivers/hwtracing/coresight/coresight-priv.h index 65a29293b6cb..f5f654ea2994 100644 --- a/drivers/hwtracing/coresight/coresight-priv.h +++ b/drivers/hwtracing/coresight/coresight-priv.h @@ -87,6 +87,7 @@ enum cs_mode { * struct cs_buffer - keep track of a recording session' specifics * @cur: index of the current buffer * @nr_pages: max number of pages granted to us + * @pid: PID this cs_buffer belongs to * @offset: offset within the current buffer * @data_size: how much we collected in this run * @snapshot: is this run in snapshot mode @@ -95,6 +96,7 @@ enum cs_mode { struct cs_buffers { unsigned int cur; unsigned int nr_pages; + pid_t pid; unsigned long offset; local_t data_size; bool snapshot; diff --git a/drivers/hwtracing/coresight/coresight-tmc-etf.c b/drivers/hwtracing/coresight/coresight-tmc-etf.c index 44402d413ebb..86ff0dda0444 100644 --- a/drivers/hwtracing/coresight/coresight-tmc-etf.c +++ b/drivers/hwtracing/coresight/coresight-tmc-etf.c @@ -227,6 +227,7 @@ static int tmc_enable_etf_sink_perf(struct coresight_device *csdev, void *data) unsigned long flags; struct tmc_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent); struct perf_output_handle *handle = data; + struct cs_buffers *buf = etm_perf_sink_config(handle); spin_lock_irqsave(&drvdata->spinlock, flags); do { @@ -243,7 +244,7 @@ static int tmc_enable_etf_sink_perf(struct coresight_device *csdev, void *data) } /* Get a handle on the pid of the process to monitor */ - pid = task_pid_nr(handle->event->owner); + pid = buf->pid; if (drvdata->pid != -1 && drvdata->pid != pid) { ret = -EBUSY; @@ -391,6 +392,10 @@ static void *tmc_alloc_etf_buffer(struct coresight_device *csdev, { int node; struct cs_buffers *buf; + struct task_struct *task = READ_ONCE(event->owner); + + if (!task || is_kernel_event(event)) + return NULL; node = (event->cpu == -1) ? NUMA_NO_NODE : cpu_to_node(event->cpu); @@ -399,6 +404,7 @@ static void *tmc_alloc_etf_buffer(struct coresight_device *csdev, if (!buf) return NULL; + buf->pid = task_pid_nr(task); buf->snapshot = overwrite; buf->nr_pages = nr_pages; buf->data_pages = pages; -- 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
next prev parent reply other threads:[~2020-10-22 10:58 UTC|newest] Thread overview: 63+ messages / expand[flat|nested] mbox.gz Atom feed top 2020-10-22 10:57 [PATCHv2 0/4] coresight: etf/etb10/etr: Fix NULL pointer dereference crashes Sai Prakash Ranjan 2020-10-22 10:57 ` Sai Prakash Ranjan 2020-10-22 10:57 ` [PATCHv2 1/4] perf/core: Export is_kernel_event() Sai Prakash Ranjan 2020-10-22 10:57 ` Sai Prakash Ranjan 2020-10-31 7:35 ` kernel test robot 2020-10-22 10:57 ` Sai Prakash Ranjan [this message] 2020-10-22 10:57 ` [PATCHv2 2/4] coresight: tmc-etf: Fix NULL ptr dereference in tmc_enable_etf_sink_perf() Sai Prakash Ranjan 2020-10-22 11:32 ` Peter Zijlstra 2020-10-22 11:32 ` Peter Zijlstra 2020-10-22 12:49 ` Sai Prakash Ranjan 2020-10-22 12:49 ` Sai Prakash Ranjan 2020-10-22 13:34 ` Peter Zijlstra 2020-10-22 13:34 ` Peter Zijlstra 2020-10-22 14:23 ` Sai Prakash Ranjan 2020-10-22 14:23 ` Sai Prakash Ranjan 2020-10-22 13:30 ` Suzuki Poulose 2020-10-22 13:30 ` Suzuki Poulose 2020-10-22 15:06 ` Peter Zijlstra 2020-10-22 15:06 ` Peter Zijlstra 2020-10-22 15:32 ` Suzuki Poulose 2020-10-22 15:32 ` Suzuki Poulose 2020-10-22 21:20 ` Mathieu Poirier 2020-10-22 21:20 ` Mathieu Poirier 2020-10-23 7:39 ` Peter Zijlstra 2020-10-23 7:39 ` Peter Zijlstra 2020-10-23 8:49 ` Suzuki Poulose 2020-10-23 8:49 ` Suzuki Poulose 2020-10-23 9:23 ` Peter Zijlstra 2020-10-23 9:23 ` Peter Zijlstra 2020-10-23 10:49 ` Suzuki Poulose 2020-10-23 10:49 ` Suzuki Poulose 2020-10-23 9:41 ` Peter Zijlstra 2020-10-23 9:41 ` Peter Zijlstra 2020-10-23 10:34 ` Suzuki Poulose 2020-10-23 10:34 ` Suzuki Poulose 2020-10-23 10:54 ` Peter Zijlstra 2020-10-23 10:54 ` Peter Zijlstra 2020-10-23 12:56 ` Suzuki Poulose 2020-10-23 12:56 ` Suzuki Poulose 2020-10-23 13:16 ` Peter Zijlstra 2020-10-23 13:16 ` Peter Zijlstra 2020-10-23 13:29 ` Suzuki Poulose 2020-10-23 13:29 ` Suzuki Poulose 2020-10-23 13:44 ` Peter Zijlstra 2020-10-23 13:44 ` Peter Zijlstra 2020-10-23 20:37 ` Mathieu Poirier 2020-10-23 20:37 ` Mathieu Poirier 2020-10-30 7:59 ` Sai Prakash Ranjan 2020-10-30 7:59 ` Sai Prakash Ranjan 2020-10-30 16:48 ` Mathieu Poirier 2020-10-30 16:48 ` Mathieu Poirier 2020-10-30 17:26 ` Sai Prakash Ranjan 2020-10-30 17:26 ` Sai Prakash Ranjan 2020-11-04 17:03 ` Mathieu Poirier 2020-11-04 17:03 ` Mathieu Poirier 2020-10-22 10:57 ` [PATCHv2 3/4] coresight: etb10: Fix possible NULL ptr dereference in etb_enable_perf() Sai Prakash Ranjan 2020-10-22 10:57 ` Sai Prakash Ranjan 2020-10-22 10:57 ` [PATCHv2 4/4] coresight: tmc-etr: Fix possible NULL ptr dereference in get_perf_etr_buf_cpu_wide() Sai Prakash Ranjan 2020-10-22 10:57 ` Sai Prakash Ranjan 2020-10-22 11:10 ` [PATCHv2 0/4] coresight: etf/etb10/etr: Fix NULL pointer dereference crashes Sai Prakash Ranjan 2020-10-22 11:10 ` Sai Prakash Ranjan 2020-10-22 11:23 ` Sai Prakash Ranjan 2020-10-22 11:23 ` Sai Prakash Ranjan
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=aa6e571156d6e26e54da0bb3015ba474e4a08da0.1603363729.git.saiprakash.ranjan@codeaurora.org \ --to=saiprakash.ranjan@codeaurora.org \ --cc=acme@kernel.org \ --cc=alexander.shishkin@linux.intel.com \ --cc=coresight@lists.linaro.org \ --cc=jolsa@redhat.com \ --cc=linux-arm-kernel@lists.infradead.org \ --cc=linux-arm-msm@vger.kernel.org \ --cc=linux-kernel@vger.kernel.org \ --cc=mark.rutland@arm.com \ --cc=mathieu.poirier@linaro.org \ --cc=mike.leach@linaro.org \ --cc=mingo@redhat.com \ --cc=namhyung@kernel.org \ --cc=peterz@infradead.org \ --cc=suzuki.poulose@arm.com \ --cc=swboyd@chromium.org \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.