linux-kernel - [PATCH] bpf: don't call mmap_read

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 for Android: free password hash cracker in your pocket

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

Date: Sat, 8 Jun 2024 19:53:58 +0900
From: Tetsuo Handa <penguin-kernel@...ove.SAKURA.ne.jp>
To: Alexei Starovoitov <ast@...nel.org>,
        Daniel Borkmann <daniel@...earbox.net>,
        Andrii Nakryiko <andrii@...nel.org>,
        Martin KaFai Lau <martin.lau@...ux.dev>,
        Eduard Zingerman
 <eddyz87@...il.com>, Song Liu <song@...nel.org>,
        Yonghong Song <yonghong.song@...ux.dev>,
        John Fastabend <john.fastabend@...il.com>,
        KP Singh <kpsingh@...nel.org>, Stanislav Fomichev <sdf@...gle.com>,
        Hao Luo <haoluo@...gle.com>, Jiri Olsa <jolsa@...nel.org>
Cc: bpf <bpf@...r.kernel.org>, LKML <linux-kernel@...r.kernel.org>
Subject: [PATCH] bpf: don't call mmap_read_trylock() from IRQ context

syzbot is reporting that the same local lock is held when trying to
hold mmap sem from both IRQ enabled context and IRQ context.

Since all callers use bpf_mmap_unlock_get_irq_work() before calling
mmap_read_trylock(), test in_hardirq() at bpf_mmap_unlock_get_irq_work()
in order to make sure that mmap_read_trylock() won't be called from IRQ
context.

  asm_exc_page_fault() => exc_page_fault() => handle_page_fault()
  => do_user_addr_fault() => handle_mm_fault() => __handle_mm_fault()
  => handle_pte_fault() => do_pte_missing() => do_anonymous_page()
  => vmf_anon_prepare() => mmap_read_trylock()
  => __mmap_lock_trace_start_locking()
  => __mmap_lock_do_trace_start_locking() => local_lock_acquire()
  => lock_acquire()

  sysvec_irq_work() => instr_sysvec_irq_work() => __sysvec_irq_work()
  => irq_work_run() => irq_work_run_list() => irq_work_single()
  => do_bpf_send_signal() => group_send_sig_info() => rcu_read_unlock()
  => rcu_lock_release() => lock_release() => trace_lock_release()
  => perf_trace_lock() => perf_trace_run_bpf_submit() => trace_call_bpf()
  => bpf_prog_run_array() => bpf_prog_run() => __bpf_prog_run()
  => bpf_dispatcher_nop_func() => bpf_prog_e6cf5f9c69743609()
  => __bpf_get_stack() => stack_map_get_build_id_offset()
  => mmap_read_trylock() => __mmap_lock_trace_acquire_returned()
  => __mmap_lock_do_trace_acquire_returned() => local_lock_acquire()
  => lock_acquire()

WARNING: inconsistent lock state
6.10.0-rc2-syzkaller-00222-gd30d0e49da71 #0 Not tainted
--------------------------------
inconsistent {HARDIRQ-ON-W} -> {IN-HARDIRQ-W} usage.
syz-executor.2/10910 [HC1[1]:SC0[0]:HE0:SE1] takes:
ffff8880b9538828 (lock#10){?.+.}-{2:2}, at: local_lock_acquire include/linux/local_lock_internal.h:29 [inline]
ffff8880b9538828 (lock#10){?.+.}-{2:2}, at: __mmap_lock_do_trace_acquire_returned+0x8f/0x630 mm/mmap_lock.c:237

other info that might help us debug this:
 Possible unsafe locking scenario:

       CPU0
       ----
  lock(lock#10);
  <Interrupt>
    lock(lock#10);

 *** DEADLOCK ***

Reported-by: syzbot <syzbot+a225ee3df7e7f9372dbe@...kaller.appspotmail.com>
Closes: https://syzkaller.appspot.com/bug?extid=a225ee3df7e7f9372dbe
Signed-off-by: Tetsuo Handa <penguin-kernel@...ove.SAKURA.ne.jp>
---
Example is https://syzkaller.appspot.com/text?tag=CrashReport&x=1649a2e2980000 .
But not using this example, for this link will disappear eventually.

 kernel/bpf/mmap_unlock_work.h | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/kernel/bpf/mmap_unlock_work.h b/kernel/bpf/mmap_unlock_work.h
index 5d18d7d85bef..337eb314d918 100644
--- a/kernel/bpf/mmap_unlock_work.h
+++ b/kernel/bpf/mmap_unlock_work.h
@@ -26,7 +26,13 @@ static inline bool bpf_mmap_unlock_get_irq_work(struct mmap_unlock_irq_work **wo
 	struct mmap_unlock_irq_work *work = NULL;
 	bool irq_work_busy = false;
 
-	if (irqs_disabled()) {
+	if (in_hardirq()) {
+		/*
+		 * IRQ context does not allow to trylock mmap sem.
+		 * Force the fallback code.
+		 */
+		irq_work_busy = true;
+	} else if (irqs_disabled()) {
 		if (!IS_ENABLED(CONFIG_PREEMPT_RT)) {
 			work = this_cpu_ptr(&mmap_unlock_work);
 			if (irq_work_is_busy(&work->irq_work)) {
-- 
2.34.1