All of lore.kernel.org
 help / color / mirror / Atom feed
From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
To: stable@vger.kernel.org
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	patches@lists.linux.dev,
	syzbot+9228d6098455bb209ec8@syzkaller.appspotmail.com,
	"Peter Zijlstra (Intel)" <peterz@infradead.org>,
	Marco Elver <elver@google.com>, Sasha Levin <sashal@kernel.org>
Subject: [PATCH 5.15 13/14] perf: Fix perf_pending_task() UaF
Date: Thu, 15 Dec 2022 19:10:49 +0100	[thread overview]
Message-ID: <20221215172907.302003042@linuxfoundation.org> (raw)
In-Reply-To: <20221215172906.338769943@linuxfoundation.org>

From: Peter Zijlstra <peterz@infradead.org>

[ Upstream commit 517e6a301f34613bff24a8e35b5455884f2d83d8 ]

Per syzbot it is possible for perf_pending_task() to run after the
event is free()'d. There are two related but distinct cases:

 - the task_work was already queued before destroying the event;
 - destroying the event itself queues the task_work.

The first cannot be solved using task_work_cancel() since
perf_release() itself might be called from a task_work (____fput),
which means the current->task_works list is already empty and
task_work_cancel() won't be able to find the perf_pending_task()
entry.

The simplest alternative is extending the perf_event lifetime to cover
the task_work.

The second is just silly, queueing a task_work while you know the
event is going away makes no sense and is easily avoided by
re-arranging how the event is marked STATE_DEAD and ensuring it goes
through STATE_OFF on the way down.

Reported-by: syzbot+9228d6098455bb209ec8@syzkaller.appspotmail.com
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Tested-by: Marco Elver <elver@google.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
 kernel/events/core.c | 17 +++++++++++++----
 1 file changed, 13 insertions(+), 4 deletions(-)

diff --git a/kernel/events/core.c b/kernel/events/core.c
index 44f982b73640..5422bd77c7d4 100644
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -2367,6 +2367,7 @@ event_sched_out(struct perf_event *event,
 		    !event->pending_work) {
 			event->pending_work = 1;
 			dec = false;
+			WARN_ON_ONCE(!atomic_long_inc_not_zero(&event->refcount));
 			task_work_add(current, &event->pending_task, TWA_RESUME);
 		}
 		if (dec)
@@ -2412,6 +2413,7 @@ group_sched_out(struct perf_event *group_event,
 
 #define DETACH_GROUP	0x01UL
 #define DETACH_CHILD	0x02UL
+#define DETACH_DEAD	0x04UL
 
 /*
  * Cross CPU call to remove a performance event
@@ -2432,12 +2434,20 @@ __perf_remove_from_context(struct perf_event *event,
 		update_cgrp_time_from_cpuctx(cpuctx, false);
 	}
 
+	/*
+	 * Ensure event_sched_out() switches to OFF, at the very least
+	 * this avoids raising perf_pending_task() at this time.
+	 */
+	if (flags & DETACH_DEAD)
+		event->pending_disable = 1;
 	event_sched_out(event, cpuctx, ctx);
 	if (flags & DETACH_GROUP)
 		perf_group_detach(event);
 	if (flags & DETACH_CHILD)
 		perf_child_detach(event);
 	list_del_event(event, ctx);
+	if (flags & DETACH_DEAD)
+		event->state = PERF_EVENT_STATE_DEAD;
 
 	if (!ctx->nr_events && ctx->is_active) {
 		if (ctx == &cpuctx->ctx)
@@ -5212,9 +5222,7 @@ int perf_event_release_kernel(struct perf_event *event)
 
 	ctx = perf_event_ctx_lock(event);
 	WARN_ON_ONCE(ctx->parent_ctx);
-	perf_remove_from_context(event, DETACH_GROUP);
 
-	raw_spin_lock_irq(&ctx->lock);
 	/*
 	 * Mark this event as STATE_DEAD, there is no external reference to it
 	 * anymore.
@@ -5226,8 +5234,7 @@ int perf_event_release_kernel(struct perf_event *event)
 	 * Thus this guarantees that we will in fact observe and kill _ALL_
 	 * child events.
 	 */
-	event->state = PERF_EVENT_STATE_DEAD;
-	raw_spin_unlock_irq(&ctx->lock);
+	perf_remove_from_context(event, DETACH_GROUP|DETACH_DEAD);
 
 	perf_event_ctx_unlock(event, ctx);
 
@@ -6662,6 +6669,8 @@ static void perf_pending_task(struct callback_head *head)
 	if (rctx >= 0)
 		perf_swevent_put_recursion_context(rctx);
 	preempt_enable_notrace();
+
+	put_event(event);
 }
 
 /*              
-- 
2.35.1




  parent reply	other threads:[~2022-12-15 18:12 UTC|newest]

Thread overview: 26+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-12-15 18:10 [PATCH 5.15 00/14] 5.15.84-rc1 review Greg Kroah-Hartman
2022-12-15 18:10 ` [PATCH 5.15 01/14] x86/vdso: Conditionally export __vdso_sgx_enter_enclave() Greg Kroah-Hartman
2022-12-15 18:10 ` [PATCH 5.15 02/14] vfs: fix copy_file_range() averts filesystem freeze protection Greg Kroah-Hartman
2022-12-15 18:10 ` [PATCH 5.15 03/14] nfp: fix use-after-free in area_cache_get() Greg Kroah-Hartman
2022-12-15 18:10 ` [PATCH 5.15 04/14] ASoC: fsl_micfil: explicitly clear software reset bit Greg Kroah-Hartman
2022-12-15 18:10 ` [PATCH 5.15 05/14] ASoC: fsl_micfil: explicitly clear CHnF flags Greg Kroah-Hartman
2022-12-15 18:10 ` [PATCH 5.15 06/14] ASoC: ops: Check bounds for second channel in snd_soc_put_volsw_sx() Greg Kroah-Hartman
2022-12-15 18:10 ` [PATCH 5.15 07/14] libbpf: Use page size as max_entries when probing ring buffer map Greg Kroah-Hartman
2022-12-15 18:10 ` [PATCH 5.15 08/14] pinctrl: meditatek: Startup with the IRQs disabled Greg Kroah-Hartman
2022-12-15 18:10 ` [PATCH 5.15 09/14] can: sja1000: fix size of OCR_MODE_MASK define Greg Kroah-Hartman
2022-12-15 18:10 ` [PATCH 5.15 10/14] can: mcba_usb: Fix termination command argument Greg Kroah-Hartman
2022-12-15 18:10 ` [PATCH 5.15 11/14] net: fec: dont reset irq coalesce settings to defaults on "ip link up" Greg Kroah-Hartman
2022-12-16  9:13   ` Rasmus Villemoes
2022-12-19 10:16     ` Greg Kroah-Hartman
2022-12-15 18:10 ` [PATCH 5.15 12/14] ASoC: cs42l51: Correct PGA Volume minimum value Greg Kroah-Hartman
2022-12-15 18:10 ` Greg Kroah-Hartman [this message]
2022-12-15 18:10 ` [PATCH 5.15 14/14] nvme-pci: clear the prp2 field when not used Greg Kroah-Hartman
2022-12-15 23:47 ` [PATCH 5.15 00/14] 5.15.84-rc1 review Shuah Khan
2022-12-16  4:32 ` Bagas Sanjaya
2022-12-16 10:09 ` Allen Pais
2022-12-16 10:41 ` Naresh Kamboju
2022-12-16 13:04 ` Jon Hunter
2022-12-16 13:13 ` Sudip Mukherjee (Codethink)
2022-12-16 20:26 ` Florian Fainelli
2022-12-16 21:30 ` Guenter Roeck
2022-12-17  0:11 ` Ron Economos

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=20221215172907.302003042@linuxfoundation.org \
    --to=gregkh@linuxfoundation.org \
    --cc=elver@google.com \
    --cc=patches@lists.linux.dev \
    --cc=peterz@infradead.org \
    --cc=sashal@kernel.org \
    --cc=stable@vger.kernel.org \
    --cc=syzbot+9228d6098455bb209ec8@syzkaller.appspotmail.com \
    /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.