linux-kernel - [PATCH] perf: Fix perf_event_validate

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Date: Fri, 15 Dec 2023 11:24:50 +0000
From: Mark Rutland <mark.rutland@....com>
To: linux-kernel@...r.kernel.org,
	Ingo Molnar <mingo@...hat.com>,
	Borislav Petkov <bp@...en8.de>,
	Peter Zijlstra <peterz@...radead.org>
Cc: lucas.demarchi@...el.com,
	mark.rutland@....com,
	pengfei.xu@...el.com
Subject: [PATCH] perf: Fix perf_event_validate_size() lockdep splat

When lockdep is enabled, the for_each_sibling_event(sibling, event)
macro checks that event->ctx->mutex is held. When creating a new group
leader event, we call perf_event_validate_size() on a partially
initialized event where event->ctx is NULL, and so when
for_each_sibling_event() attempts to check event->ctx->mutex, we get a
splat, as reported by Lucas De Marchi:

  WARNING: CPU: 8 PID: 1471 at kernel/events/core.c:1950 __do_sys_perf_event_open+0xf37/0x1080

This only happens for a new event which is its own group_leader, and in
this case there cannot be any sibling events. Thus it's safe to skip the
check for siblings, which avoids having to make invasive and ugly
changes to for_each_sibling_event().

Avoid the splat by bailing out early when the new event is its own
group_leader.

Fixes: 382c27f4ed28f803 ("perf: Fix perf_event_validate_size()")
Reported-by: Lucas De Marchi <lucas.demarchi@...el.com>
Closes: https://lore.kernel.org/lkml/20231214000620.3081018-1-lucas.demarchi@intel.com/
Reported-by: Pengfei Xu <pengfei.xu@...el.com>
Closes: https://lore.kernel.org/lkml/ZXpm6gQ%2Fd59jGsuW@xpf.sh.intel.com/
Signed-off-by: Mark Rutland <mark.rutland@....com>
Cc: Ingo Molnar <mingo@...hat.com>
Cc: Borislav Petkov <bp@...en8.de>
Cc: Peter Zijlstra <peterz@...radead.org>
---
 kernel/events/core.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

Hi Ingo, Boris, Peter,

I'm not sure who's still around and who has disappeared for the
holidays, but I'm hoping at least one of you is able to queue this. I've
tested the patch on arm64 with Syzkaller (and syz-repro); before this
patch it hits the splat near-instantly, and after this patch all seems
well.

The broken commit was merged in v6.7-rc5 via:

  https://lore.kernel.org/lkml/20231210105949.GAZXWaJe6DeHU9+ofl@fat_crate.local/

... in merge commit:

  537ccb5d28d6f398 ("Merge tag 'perf_urgent_for_v6.7_rc5' of git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip")

Mark.

diff --git a/kernel/events/core.c b/kernel/events/core.c
index c9d123e13b579..9efd0d7775e7c 100644
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -1947,6 +1947,16 @@ static bool perf_event_validate_size(struct perf_event *event)
 				   group_leader->nr_siblings + 1) > 16*1024)
 		return false;

+	/*
+	 * When creating a new group leader, group_leader->ctx is initialized
+	 * after the size has been validated, but we cannot safely use
+	 * for_each_sibling_event() until group_leader->ctx is set. A new group
+	 * leader cannot have any siblings yet, so we can safely skip checking
+	 * the non-existent siblings.
+	 */
+	if (event == group_leader)
+		return true;
+
 	for_each_sibling_event(sibling, group_leader) {
 		if (__perf_event_read_size(sibling->attr.read_format,
 					   group_leader->nr_siblings + 1) > 16*1024)
-- 
2.30.2