linux-kernel - [PATCH] perf/x86/uncore: Allow single pmu/box within events group

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]

Message-ID: <20161118001528.GA28231@krava>
Date:   Fri, 18 Nov 2016 01:15:28 +0100
From:   Jiri Olsa <jolsa@...hat.com>
To:     Peter Zijlstra <peterz@...radead.org>
Cc:     "Liang, Kan" <kan.liang@...el.com>,
        Andi Kleen <andi@...stfloor.org>,
        Vince Weaver <vince@...ter.net>,
        lkml <linux-kernel@...r.kernel.org>,
        Ingo Molnar <mingo@...nel.org>,
        Thomas Gleixner <tglx@...utronix.de>
Subject: [PATCH] perf/x86/uncore: Allow single pmu/box within events group

hi,
I was regularly hitting bug in uncore_validate_group when running
fuzzer. Attached patch makes it go away.

The problem is that in uncore_validate_group we could access uncore
boxes regs data with event's regs data which was initialized for
another box. Attached patch makes the check fail when it detects
wrong box.

Before I realized that the generic code won't allow for more pmu/box
contexts within events group, I reworked uncore_validate_group logic
to allow that, it's in here:
  https://git.kernel.org/cgit/linux/kernel/git/jolsa/perf.git/log/?h=perf/uncore_fix

I'm not sure wether it's useful to allow different uncore boxes
within events group, but it should not be problem to enable it.

Also I still don't understand how can we touch group sibling events
within event_init callback.. I must be missing something, because
fuzzer does not complain about this ;-)

thanks,
jirka


---
Current uncore_validate_group code expects all events within
the group to have same pmu.

This leads to constraint code using wrong boxes which leads
in my case to touching uninitialized spinlocks, but could
be probably worse.. depends on type and box details.

I get lockdep warning below for following perf stat:
  # perf stat -vv -e '{uncore_cbox_0/config=0x0334/,uncore_qpi_0/event=1/}' -a sleep 1

INFO: trying to register non-static key.
the code is fine but needs lockdep annotation.
turning off the locking correctness validator.
CPU: 12 PID: 3727 Comm: perf_fuzzer Tainted: G        W       4.9.0-rc1+ #10
Hardware name: IBM System x3650 M4 : -[7915E2G]-/00Y7683, BIOS -[VVE124AUS-1.30]- 11/21/2012
 ffffc90002ca3ae0 ffffffff81399565 0000000000000000 0000000000000000
 ffffc90002ca3b50 ffffffff810d0a3a 0000000000000006 ffff880276285eb8
 ffffc90002ca3bc8 ffff8802711a8b98 ffff8802711a8000 ffff8802711a8b98
Call Trace:
 [<ffffffff81399565>] dump_stack+0x68/0x93
 [<ffffffff810d0a3a>] register_lock_class+0x50a/0x510
 [<ffffffff810d2d78>] __lock_acquire+0x88/0x12a0
 [<ffffffff811a2cfc>] ? ___perf_sw_event+0x16c/0x280
 [<ffffffff810f091d>] ? debug_lockdep_rcu_enabled+0x1d/0x20
 [<ffffffff810d43b9>] lock_acquire+0xe9/0x1d0
 [<ffffffffa0186679>] ? uncore_get_constraint+0x59/0x100 [intel_uncore]
 [<ffffffff817075e3>] _raw_spin_lock_irqsave+0x43/0x60
 [<ffffffffa0186679>] ? uncore_get_constraint+0x59/0x100 [intel_uncore]
 [<ffffffffa0186679>] uncore_get_constraint+0x59/0x100 [intel_uncore]
 [<ffffffffa01853fd>] uncore_assign_events+0x6d/0x250 [intel_uncore]
 [<ffffffffa0185df7>] uncore_pmu_event_init+0x187/0x220 [intel_uncore]
 [<ffffffff81197443>] perf_try_init_event+0x43/0x90
 [<ffffffff8119b65e>] perf_event_alloc+0x55e/0xc50
 [<ffffffff8119b568>] ? perf_event_alloc+0x468/0xc50
 [<ffffffff8119f7db>] SYSC_perf_event_open+0x40b/0xf90
 [<ffffffff811a2f69>] SyS_perf_event_open+0x9/0x10
 [<ffffffff81002eb6>] do_syscall_64+0x66/0x1d0
 [<ffffffff81707f24>] entry_SYSCALL64_slow_path+0x25/0x25

The reason is that leader event will get extra reg configuration
which touches regs of second event's box.

This patch makes the uncore_validate_group check fail when it detects
wrong box.

Signed-off-by: Jiri Olsa <jolsa@...hat.com>
---
diff --git a/arch/x86/events/intel/uncore.c b/arch/x86/events/intel/uncore.c
index efca2685d876..a81b5a39b79e 100644
--- a/arch/x86/events/intel/uncore.c
+++ b/arch/x86/events/intel/uncore.c
@@ -597,6 +597,10 @@ static int uncore_validate_group(struct intel_uncore_pmu *pmu,
 	struct intel_uncore_box *fake_box;
 	int ret = -EINVAL, n;
 
+	/* All events in group must be from the same uncore box. */
+	if (leader->pmu->type != pmu->pmu.type)
+		return -EINVAL;
+
 	fake_box = uncore_alloc_box(pmu->type, NUMA_NO_NODE);
 	if (!fake_box)
 		return -ENOMEM;