stable.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* FAILED: patch "[PATCH] perf/core: Fix cgroup event list management" failed to apply to 5.10-stable tree
@ 2022-01-31  8:21 gregkh
  2022-01-31 20:41 ` Namhyung Kim
  0 siblings, 1 reply; 3+ messages in thread
From: gregkh @ 2022-01-31  8:21 UTC (permalink / raw)
  To: namhyung, peterz; +Cc: stable


The patch below does not apply to the 5.10-stable tree.
If someone wants it applied there, or to any other stable or longterm
tree, then please email the backport, including the original git commit
id to <stable@vger.kernel.org>.

thanks,

greg k-h

------------------ original commit in Linus's tree ------------------

From c5de60cd622a2607c043ba65e25a6e9998a369f9 Mon Sep 17 00:00:00 2001
From: Namhyung Kim <namhyung@kernel.org>
Date: Mon, 24 Jan 2022 11:58:08 -0800
Subject: [PATCH] perf/core: Fix cgroup event list management

The active cgroup events are managed in the per-cpu cgrp_cpuctx_list.
This list is only accessed from current cpu and not protected by any
locks.  But from the commit ef54c1a476ae ("perf: Rework
perf_event_exit_event()"), it's possible to access (actually modify)
the list from another cpu.

In the perf_remove_from_context(), it can remove an event from the
context without an IPI when the context is not active.  This is not
safe with cgroup events which can have some active events in the
context even if ctx->is_active is 0 at the moment.  The target cpu
might be in the middle of list iteration at the same time.

If the event is enabled when it's about to be closed, it might call
perf_cgroup_event_disable() and list_del() with the cgrp_cpuctx_list
on a different cpu.

This resulted in a crash due to an invalid list pointer access during
the cgroup list traversal on the cpu which the event belongs to.

Let's fallback to IPI to access the cgrp_cpuctx_list from that cpu.
Similarly, perf_install_in_context() should use IPI for the cgroup
events too.

Fixes: ef54c1a476ae ("perf: Rework perf_event_exit_event()")
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Link: https://lkml.kernel.org/r/20220124195808.2252071-1-namhyung@kernel.org

diff --git a/kernel/events/core.c b/kernel/events/core.c
index b1c1928c0e7c..76c754e45d01 100644
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -2462,7 +2462,11 @@ static void perf_remove_from_context(struct perf_event *event, unsigned long fla
 	 * event_function_call() user.
 	 */
 	raw_spin_lock_irq(&ctx->lock);
-	if (!ctx->is_active) {
+	/*
+	 * Cgroup events are per-cpu events, and must IPI because of
+	 * cgrp_cpuctx_list.
+	 */
+	if (!ctx->is_active && !is_cgroup_event(event)) {
 		__perf_remove_from_context(event, __get_cpu_context(ctx),
 					   ctx, (void *)flags);
 		raw_spin_unlock_irq(&ctx->lock);
@@ -2895,11 +2899,14 @@ perf_install_in_context(struct perf_event_context *ctx,
 	 * perf_event_attr::disabled events will not run and can be initialized
 	 * without IPI. Except when this is the first event for the context, in
 	 * that case we need the magic of the IPI to set ctx->is_active.
+	 * Similarly, cgroup events for the context also needs the IPI to
+	 * manipulate the cgrp_cpuctx_list.
 	 *
 	 * The IOC_ENABLE that is sure to follow the creation of a disabled
 	 * event will issue the IPI and reprogram the hardware.
 	 */
-	if (__perf_effective_state(event) == PERF_EVENT_STATE_OFF && ctx->nr_events) {
+	if (__perf_effective_state(event) == PERF_EVENT_STATE_OFF &&
+	    ctx->nr_events && !is_cgroup_event(event)) {
 		raw_spin_lock_irq(&ctx->lock);
 		if (ctx->task == TASK_TOMBSTONE) {
 			raw_spin_unlock_irq(&ctx->lock);


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: FAILED: patch "[PATCH] perf/core: Fix cgroup event list management" failed to apply to 5.10-stable tree
  2022-01-31  8:21 FAILED: patch "[PATCH] perf/core: Fix cgroup event list management" failed to apply to 5.10-stable tree gregkh
@ 2022-01-31 20:41 ` Namhyung Kim
  2022-02-03 17:48   ` Greg Kroah-Hartman
  0 siblings, 1 reply; 3+ messages in thread
From: Namhyung Kim @ 2022-01-31 20:41 UTC (permalink / raw)
  To: Greg Kroah-Hartman; +Cc: Peter Zijlstra, stable # 4 . 5

Hi Greg,

On Mon, Jan 31, 2022 at 12:29 AM <gregkh@linuxfoundation.org> wrote:
>
>
> The patch below does not apply to the 5.10-stable tree.
> If someone wants it applied there, or to any other stable or longterm
> tree, then please email the backport, including the original git commit
> id to <stable@vger.kernel.org>.

I think it fails because the dependent commit ef54c1a476ae ("perf:
Rework perf_event_exit_event()") was reverted already (because
of the issue this commit fixes).

Could you please try it again with the above commit?  If it still doesn't
work, please let me know.

Thanks,
Namhyung

>
> thanks,
>
> greg k-h
>
> ------------------ original commit in Linus's tree ------------------
>
> From c5de60cd622a2607c043ba65e25a6e9998a369f9 Mon Sep 17 00:00:00 2001
> From: Namhyung Kim <namhyung@kernel.org>
> Date: Mon, 24 Jan 2022 11:58:08 -0800
> Subject: [PATCH] perf/core: Fix cgroup event list management
>
> The active cgroup events are managed in the per-cpu cgrp_cpuctx_list.
> This list is only accessed from current cpu and not protected by any
> locks.  But from the commit ef54c1a476ae ("perf: Rework
> perf_event_exit_event()"), it's possible to access (actually modify)
> the list from another cpu.
>
> In the perf_remove_from_context(), it can remove an event from the
> context without an IPI when the context is not active.  This is not
> safe with cgroup events which can have some active events in the
> context even if ctx->is_active is 0 at the moment.  The target cpu
> might be in the middle of list iteration at the same time.
>
> If the event is enabled when it's about to be closed, it might call
> perf_cgroup_event_disable() and list_del() with the cgrp_cpuctx_list
> on a different cpu.
>
> This resulted in a crash due to an invalid list pointer access during
> the cgroup list traversal on the cpu which the event belongs to.
>
> Let's fallback to IPI to access the cgrp_cpuctx_list from that cpu.
> Similarly, perf_install_in_context() should use IPI for the cgroup
> events too.
>
> Fixes: ef54c1a476ae ("perf: Rework perf_event_exit_event()")
> Signed-off-by: Namhyung Kim <namhyung@kernel.org>
> Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
> Link: https://lkml.kernel.org/r/20220124195808.2252071-1-namhyung@kernel.org
>
> diff --git a/kernel/events/core.c b/kernel/events/core.c
> index b1c1928c0e7c..76c754e45d01 100644
> --- a/kernel/events/core.c
> +++ b/kernel/events/core.c
> @@ -2462,7 +2462,11 @@ static void perf_remove_from_context(struct perf_event *event, unsigned long fla
>          * event_function_call() user.
>          */
>         raw_spin_lock_irq(&ctx->lock);
> -       if (!ctx->is_active) {
> +       /*
> +        * Cgroup events are per-cpu events, and must IPI because of
> +        * cgrp_cpuctx_list.
> +        */
> +       if (!ctx->is_active && !is_cgroup_event(event)) {
>                 __perf_remove_from_context(event, __get_cpu_context(ctx),
>                                            ctx, (void *)flags);
>                 raw_spin_unlock_irq(&ctx->lock);
> @@ -2895,11 +2899,14 @@ perf_install_in_context(struct perf_event_context *ctx,
>          * perf_event_attr::disabled events will not run and can be initialized
>          * without IPI. Except when this is the first event for the context, in
>          * that case we need the magic of the IPI to set ctx->is_active.
> +        * Similarly, cgroup events for the context also needs the IPI to
> +        * manipulate the cgrp_cpuctx_list.
>          *
>          * The IOC_ENABLE that is sure to follow the creation of a disabled
>          * event will issue the IPI and reprogram the hardware.
>          */
> -       if (__perf_effective_state(event) == PERF_EVENT_STATE_OFF && ctx->nr_events) {
> +       if (__perf_effective_state(event) == PERF_EVENT_STATE_OFF &&
> +           ctx->nr_events && !is_cgroup_event(event)) {
>                 raw_spin_lock_irq(&ctx->lock);
>                 if (ctx->task == TASK_TOMBSTONE) {
>                         raw_spin_unlock_irq(&ctx->lock);
>

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: FAILED: patch "[PATCH] perf/core: Fix cgroup event list management" failed to apply to 5.10-stable tree
  2022-01-31 20:41 ` Namhyung Kim
@ 2022-02-03 17:48   ` Greg Kroah-Hartman
  0 siblings, 0 replies; 3+ messages in thread
From: Greg Kroah-Hartman @ 2022-02-03 17:48 UTC (permalink / raw)
  To: Namhyung Kim; +Cc: Peter Zijlstra, stable # 4 . 5

On Mon, Jan 31, 2022 at 12:41:26PM -0800, Namhyung Kim wrote:
> Hi Greg,
> 
> On Mon, Jan 31, 2022 at 12:29 AM <gregkh@linuxfoundation.org> wrote:
> >
> >
> > The patch below does not apply to the 5.10-stable tree.
> > If someone wants it applied there, or to any other stable or longterm
> > tree, then please email the backport, including the original git commit
> > id to <stable@vger.kernel.org>.
> 
> I think it fails because the dependent commit ef54c1a476ae ("perf:
> Rework perf_event_exit_event()") was reverted already (because
> of the issue this commit fixes).
> 
> Could you please try it again with the above commit?  If it still doesn't
> work, please let me know.

Yes, that seemed to work, thanks!

greg k-h

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2022-02-03 17:48 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-01-31  8:21 FAILED: patch "[PATCH] perf/core: Fix cgroup event list management" failed to apply to 5.10-stable tree gregkh
2022-01-31 20:41 ` Namhyung Kim
2022-02-03 17:48   ` Greg Kroah-Hartman

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