From: Peter Zijlstra <peterz@infradead.org>
To: Suzuki Poulose <suzuki.poulose@arm.com>
Cc: Mathieu Poirier <mathieu.poirier@linaro.org>,
Sai Prakash Ranjan <saiprakash.ranjan@codeaurora.org>,
Mike Leach <mike.leach@linaro.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>,
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
Subject: Re: [PATCHv2 2/4] coresight: tmc-etf: Fix NULL ptr dereference in tmc_enable_etf_sink_perf()
Date: Fri, 23 Oct 2020 11:23:37 +0200 [thread overview]
Message-ID: <20201023092337.GQ2611@hirez.programming.kicks-ass.net> (raw)
In-Reply-To: <174e6461-4d46-cb65-c094-c06ee3b21568@arm.com>
On Fri, Oct 23, 2020 at 09:49:53AM +0100, Suzuki Poulose wrote:
> On 10/23/20 8:39 AM, Peter Zijlstra wrote:
> > So then I don't understand the !->owner issue, that only happens when
> > the task dies, which cannot be concurrent with event creation. Are you
>
> Part of the patch from Sai, fixes this by avoiding the dereferencing
> after event creation (by caching it). But the kernel events needs
> fixing.
>
> One follow up question on the !->owner issue. Given the ->owner is
> dying, does it prevent events from being scheduled ? Or is there a delay
> between that and eventually stopping the events. In this case, we hit
> the issue when :
>
> A A or B ?
>
> event_start()
> ... event->owner = NULL
>
> READ_ONCE(event->owner);
>
> Is this expected ?
Yeah, teardown is a bit of an effort. Also, you can pass an fd over a
unix socket to another process, so this isn't something you can rely on
in any case.
The perf tool doesn't do it, but the kernel infra should be able to deal
with someone doing a perf-deamon of sorts, where you can request a perf
event and recieve a fd from it.
Imagine the fun ;-)
> > As for the kernel events.. why do you care about the actual task_struct
> > * in there? I see you're using it to grab the task-pid, but how is that
> > useful?
>
> Correct, kernel events are something that the driver didn't account for.
> May be we could handle this case with a "special pid" and simply
> disallow sharing (which is fine I believe, given there are not grouping
> for the kernel created events).
Why do you need a pid in the first place? Can't you use the "task_struct
*" as a value?
WARNING: multiple messages have this Message-ID (diff)
From: Peter Zijlstra <peterz@infradead.org>
To: Suzuki Poulose <suzuki.poulose@arm.com>
Cc: Mark Rutland <mark.rutland@arm.com>,
Sai Prakash Ranjan <saiprakash.ranjan@codeaurora.org>,
Mathieu Poirier <mathieu.poirier@linaro.org>,
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()
Date: Fri, 23 Oct 2020 11:23:37 +0200 [thread overview]
Message-ID: <20201023092337.GQ2611@hirez.programming.kicks-ass.net> (raw)
In-Reply-To: <174e6461-4d46-cb65-c094-c06ee3b21568@arm.com>
On Fri, Oct 23, 2020 at 09:49:53AM +0100, Suzuki Poulose wrote:
> On 10/23/20 8:39 AM, Peter Zijlstra wrote:
> > So then I don't understand the !->owner issue, that only happens when
> > the task dies, which cannot be concurrent with event creation. Are you
>
> Part of the patch from Sai, fixes this by avoiding the dereferencing
> after event creation (by caching it). But the kernel events needs
> fixing.
>
> One follow up question on the !->owner issue. Given the ->owner is
> dying, does it prevent events from being scheduled ? Or is there a delay
> between that and eventually stopping the events. In this case, we hit
> the issue when :
>
> A A or B ?
>
> event_start()
> ... event->owner = NULL
>
> READ_ONCE(event->owner);
>
> Is this expected ?
Yeah, teardown is a bit of an effort. Also, you can pass an fd over a
unix socket to another process, so this isn't something you can rely on
in any case.
The perf tool doesn't do it, but the kernel infra should be able to deal
with someone doing a perf-deamon of sorts, where you can request a perf
event and recieve a fd from it.
Imagine the fun ;-)
> > As for the kernel events.. why do you care about the actual task_struct
> > * in there? I see you're using it to grab the task-pid, but how is that
> > useful?
>
> Correct, kernel events are something that the driver didn't account for.
> May be we could handle this case with a "special pid" and simply
> disallow sharing (which is fine I believe, given there are not grouping
> for the kernel created events).
Why do you need a pid in the first place? Can't you use the "task_struct
*" as a value?
_______________________________________________
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-23 9:23 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 ` [PATCHv2 2/4] coresight: tmc-etf: Fix NULL ptr dereference in tmc_enable_etf_sink_perf() Sai Prakash Ranjan
2020-10-22 10:57 ` 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 [this message]
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=20201023092337.GQ2611@hirez.programming.kicks-ass.net \
--to=peterz@infradead.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=saiprakash.ranjan@codeaurora.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: link
Be 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.