linux-kernel - [PATCH] kernel: bpf: stackmap: fix a possible sleep-in-atomic bug in bpf_mmap_unlock_get_irq

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: <20230315041721.1034794-1-starmiku1207184332@gmail.com>
Date:   Wed, 15 Mar 2023 04:17:21 +0000
From:   starmiku1207184332@...il.com
To:     ast@...nel.org, daniel@...earbox.net, andrii@...nel.org,
        martin.lau@...ux.dev, song@...nel.org, yhs@...com,
        john.fastabend@...il.com, kpsingh@...nel.org, sdf@...gle.com,
        haoluo@...gle.com, jolsa@...nel.org
Cc:     bpf@...r.kernel.org, linux-kernel@...r.kernel.org,
        baijiaju1990@...look.com, Teng Qi <starmiku1207184332@...il.com>
Subject: [PATCH] kernel: bpf: stackmap: fix a possible sleep-in-atomic bug in bpf_mmap_unlock_get_irq_work()

From: Teng Qi <starmiku1207184332@...il.com>

bpf_mmap_unlock_get_irq_work() and bpf_mmap_unlock_mm() cooperate to safely
acquire mm->mmap_lock safely. The code in bpf_mmap_unlock_get_irq_work():
 struct mmap_unlock_irq_work *work = NULL;
 bool irq_work_busy = false;
 if (irqs_disabled()) {
 	if (!IS_ENABLED(CONFIG_PREEMPT_RT)) {
 		work = this_cpu_ptr(&mmap_unlock_work);
 		if (irq_work_is_busy(&work->irq_work)) {
 			irq_work_busy = true;
 		}
 	} else {
 		irq_work_busy = true;
 	}
 }
 *work_ptr = work;

shows that the pointer of struct mmap_unlock_irq_work "work" is not NULL if
irqs_disabled() == true and IS_ENABLED(CONFIG_PREEMPT_RT) == false or NULL in
other cases. The "work" will be passed to bpf_mmap_unlock_mm() as the argument.
The code in bpf_mmap_unlock_mm():
 if (!work) {
 	mmap_read_unlock(mm);
 } else {
 	work->mm = mm;
 	rwsem_release(&mm->mmap_lock.dep_map, _RET_IP_);
 	irq_work_queue(&work->irq_work);
 }

shows that mm->mmap_lock is released directly if "work" is NULL. Otherwise,
irq_work_queue is called to avoid calling mmap_read_unlock() in an irq disabled
context because of its possible sleep operation. However, mmap_read_unlock()
is unsafely called in a preempt disabled context when spin_lock() or
rcu_read_lock() has been called.

We found that some ebpf helpers that call these two functions may be invoked in
preempt disabled contexts through various hooks. We can give an example:
 SEC("kprobe/kmem_cache_free")
 int bpf_prog1(struct pt_regs *ctx)
 {
 	char buff[50];
 	bpf_get_stack(ctx, buff, sizeof(struct bpf_stack_build_id),
	              BPF_F_USER_BUILD_ID | BPF_F_USER_STACK);
 	return 0;
 }

The hook "kprobe/kmem_cache_free" is often called in preempt disabled contexts
by many modules. To fix this possible bug, we add in_atomic() in
bpf_mmap_unlock_get_irq_work().

Signed-off-by: Teng Qi <starmiku1207184332@...il.com>
---
 kernel/bpf/mmap_unlock_work.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel/bpf/mmap_unlock_work.h b/kernel/bpf/mmap_unlock_work.h
index 5d18d7d85bef..3d472d24d88f 100644
--- a/kernel/bpf/mmap_unlock_work.h
+++ b/kernel/bpf/mmap_unlock_work.h
@@ -26,7 +26,7 @@ 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_atomic() || irqs_disabled()) {
 		if (!IS_ENABLED(CONFIG_PREEMPT_RT)) {
 			work = this_cpu_ptr(&mmap_unlock_work);
 			if (irq_work_is_busy(&work->irq_work)) {
-- 
2.25.1