linux-kernel - Re: possible deadlock in __static_key_slow

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-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20200305202440.GC3348@worktop.programming.kicks-ass.net>
Date:   Thu, 5 Mar 2020 21:24:40 +0100
From:   Peter Zijlstra <peterz@...radead.org>
To:     syzbot <syzbot+61ffbb75d30176841f76@...kaller.appspotmail.com>
Cc:     bristot@...hat.com, kuba@...nel.org, linux-kernel@...r.kernel.org,
        mingo@...nel.org, simon.horman@...ronome.com,
        syzkaller-bugs@...glegroups.com
Subject: Re: possible deadlock in __static_key_slow_dec

On Thu, Mar 05, 2020 at 09:35:10AM -0800, syzbot wrote:
> Hello,
> 
> syzbot found the following crash on:
> 
> HEAD commit:    63623fd4 Merge tag 'for-linus' of git://git.kernel.org/pub..
> git tree:       upstream
> console output: https://syzkaller.appspot.com/x/log.txt?x=154474f9e00000
> kernel config:  https://syzkaller.appspot.com/x/.config?x=5d2e033af114153f
> dashboard link: https://syzkaller.appspot.com/bug?extid=61ffbb75d30176841f76
> compiler:       clang version 10.0.0 (https://github.com/llvm/llvm-project/ c2443155a0fb245c8f17f2c1c72b6ea391e86e81)
> syz repro:      https://syzkaller.appspot.com/x/repro.syz?x=14f0efa1e00000
> C reproducer:   https://syzkaller.appspot.com/x/repro.c?x=119cf3b5e00000
> 
> IMPORTANT: if you fix the bug, please add the following tag to the commit:
> Reported-by: syzbot+61ffbb75d30176841f76@...kaller.appspotmail.com

>  lock_acquire+0x154/0x250 kernel/locking/lockdep.c:4484
>  percpu_down_read include/linux/percpu-rwsem.h:40 [inline]
>  cpus_read_lock+0x3e/0x130 kernel/cpu.c:292
>  __static_key_slow_dec+0x14/0x90 kernel/jump_label.c:254
>  static_key_slow_dec+0x50/0xa0 kernel/jump_label.c:270
>  sw_perf_event_destroy+0x78/0x170 kernel/events/core.c:8840
>  _free_event+0x825/0xdc0 kernel/events/core.c:4616
>  put_event kernel/events/core.c:4710 [inline]
>  perf_mmap_close+0xc04/0xea0 kernel/events/core.c:5754
>  remove_vma mm/mmap.c:177 [inline]
>  remove_vma_list mm/mmap.c:2568 [inline]
>  __do_munmap+0x1006/0x14b0 mm/mmap.c:2812
>  do_munmap mm/mmap.c:2820 [inline]
>  mmap_region+0x8c8/0x1c40 mm/mmap.c:1713
>  do_mmap+0xa8f/0x1100 mm/mmap.c:1543
>  do_mmap_pgoff include/linux/mm.h:2334 [inline]
>  vm_mmap_pgoff+0x13d/0x1d0 mm/util.c:506

This seems to be by far the simplest to break, it also has some actual
benefits too.

Something like the (compile only tested) below ought to help. I'll try
and poke at the reproducer in the morning.

---
 include/linux/perf_event.h |  8 ++++----
 kernel/events/core.c       | 11 +++++++----
 2 files changed, 11 insertions(+), 8 deletions(-)

diff --git a/include/linux/perf_event.h b/include/linux/perf_event.h
index 547773f5894e..a5684b2e481d 100644
--- a/include/linux/perf_event.h
+++ b/include/linux/perf_event.h
@@ -1103,7 +1103,7 @@ static inline int is_exclusive_pmu(struct pmu *pmu)
 	return pmu->capabilities & PERF_PMU_CAP_EXCLUSIVE;
 }

-extern struct static_key perf_swevent_enabled[PERF_COUNT_SW_MAX];
+extern struct static_key_deferred perf_swevent_enabled[PERF_COUNT_SW_MAX];

 extern void ___perf_sw_event(u32, u64, struct pt_regs *, u64);
 extern void __perf_sw_event(u32, u64, struct pt_regs *, u64);
@@ -1134,7 +1134,7 @@ static inline void perf_fetch_caller_regs(struct pt_regs *regs)
 static __always_inline void
 perf_sw_event(u32 event_id, u64 nr, struct pt_regs *regs, u64 addr)
 {
-	if (static_key_false(&perf_swevent_enabled[event_id]))
+	if (static_key_false(&perf_swevent_enabled[event_id].key))
 		__perf_sw_event(event_id, nr, regs, addr);
 }

@@ -1148,7 +1148,7 @@ DECLARE_PER_CPU(struct pt_regs, __perf_regs[4]);
 static __always_inline void
 perf_sw_event_sched(u32 event_id, u64 nr, u64 addr)
 {
-	if (static_key_false(&perf_swevent_enabled[event_id])) {
+	if (static_key_false(&perf_swevent_enabled[event_id].key)) {
 		struct pt_regs *regs = this_cpu_ptr(&__perf_regs[0]);

 		perf_fetch_caller_regs(regs);
@@ -1161,7 +1161,7 @@ extern struct static_key_false perf_sched_events;
 static __always_inline bool
 perf_sw_migrate_enabled(void)
 {
-	if (static_key_false(&perf_swevent_enabled[PERF_COUNT_SW_CPU_MIGRATIONS]))
+	if (static_key_false(&perf_swevent_enabled[PERF_COUNT_SW_CPU_MIGRATIONS].key))
 		return true;
 	return false;
 }
diff --git a/kernel/events/core.c b/kernel/events/core.c
index e453589da97c..8ce277ec0b85 100644
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -8829,7 +8829,7 @@ static int swevent_hlist_get(void)
 	return err;
 }

-struct static_key perf_swevent_enabled[PERF_COUNT_SW_MAX];
+struct static_key_deferred perf_swevent_enabled[PERF_COUNT_SW_MAX];

 static void sw_perf_event_destroy(struct perf_event *event)
 {
@@ -8837,7 +8837,7 @@ static void sw_perf_event_destroy(struct perf_event *event)

 	WARN_ON(event->parent);

-	static_key_slow_dec(&perf_swevent_enabled[event_id]);
+	static_key_slow_dec_deferred(&perf_swevent_enabled[event_id]);
 	swevent_hlist_put();
 }

@@ -8873,7 +8873,7 @@ static int perf_swevent_init(struct perf_event *event)
 		if (err)
 			return err;

-		static_key_slow_inc(&perf_swevent_enabled[event_id]);
+		static_key_slow_inc(&perf_swevent_enabled[event_id].key);
 		event->destroy = sw_perf_event_destroy;
 	}

@@ -12376,7 +12376,10 @@ int perf_event_init_task(struct task_struct *child)
 static void __init perf_event_init_all_cpus(void)
 {
 	struct swevent_htable *swhash;
-	int cpu;
+	int i, cpu;
+
+	for (i = 0; i < PERF_COUNT_SW_MAX; i++)
+		jump_label_rate_limit(&perf_swevent_enabled[i], HZ/10);

 	zalloc_cpumask_var(&perf_online_mask, GFP_KERNEL);