Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Sender: saiprakash.ranjan=codeaurora.org@mg.codeaurora.org
MIME-Version: 1.0
Content-Type: text/plain; charset=US-ASCII;
 format=flowed
Content-Transfer-Encoding: 7bit
Date:   Thu, 22 Oct 2020 18:19:37 +0530
From:   Sai Prakash Ranjan <saiprakash.ranjan@codeaurora.org>
To:     Peter Zijlstra <peterz@infradead.org>
Cc:     Mark Rutland <mark.rutland@arm.com>,
        Mathieu Poirier <mathieu.poirier@linaro.org>,
        Suzuki K Poulose <suzuki.poulose@arm.com>,
        Alexander Shishkin <alexander.shishkin@linux.intel.com>,
        linux-arm-msm@vger.kernel.org, coresight@lists.linaro.org,
        linux-kernel@vger.kernel.org,
        Arnaldo Carvalho de Melo <acme@kernel.org>,
        Stephen Boyd <swboyd@chromium.org>,
        Ingo Molnar <mingo@redhat.com>,
        Namhyung Kim <namhyung@kernel.org>,
        Jiri Olsa <jolsa@redhat.com>,
        linux-arm-kernel@lists.infradead.org,
        Mike Leach <mike.leach@linaro.org>
Subject: Re: [PATCHv2 2/4] coresight: tmc-etf: Fix NULL ptr dereference in
 tmc_enable_etf_sink_perf()
In-Reply-To: <20201022113214.GD2611@hirez.programming.kicks-ass.net>
References: <cover.1603363729.git.saiprakash.ranjan@codeaurora.org>
 <aa6e571156d6e26e54da0bb3015ba474e4a08da0.1603363729.git.saiprakash.ranjan@codeaurora.org>
 <20201022113214.GD2611@hirez.programming.kicks-ass.net>
Message-ID: <a6eadc723ae52396a655c61b9f8d4eef@codeaurora.org>
User-Agent: Roundcube Webmail/1.3.9
Precedence: bulk

On 2020-10-22 17:02, Peter Zijlstra wrote:
> On Thu, Oct 22, 2020 at 04:27:52PM +0530, Sai Prakash Ranjan wrote:
> 
>> 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.
> 
>> @@ -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;
> 
> 
> This is *wrong*... why do you care about who owns the events?
> 

The original issue was the owner being NULL and causing
a NULL pointer dereference. I did ask some time back
if it is valid for the owner to be NULL [1] and should
probably be handled in events core?

[1] 
https://lore.kernel.org/lkml/c0e1f99a0a2480dfc8d788bb424d3f08@codeaurora.org/

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


Thanks,
Sai

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