bpf: Fix mmap_lock leak in irq_work path

stack_map_get_build_id_offset() introduced a per-CPU irq_work to defer mmap_read_unlock() from NMI context, and bpf_find_vma() later reused the same mmap_unlock_work. Both callers only check whether the work is busy before taking mmap_lock, so a nested caller can reuse the slot before the first caller queues it. Two read locks may then be acquired while only one deferred unlock runs, leaking a read lock and blocking exit_mmap(). Reserve the per-CPU slot before mmap_read_trylock(). Use the same wrapper in stackmap and bpf_find_vma() so both callers release the reservation on trylock failure. Keep rejecting the slot while the irq_work remains busy. Release it after the irq_work callback unlocks the mm.

Sanghyun Park committed 3 months ago

Commit fa9dcac1 parent 3f562c53 files · +56 −37Browse files

Unified · Split

3 files

kernel/bpf/mmap_unlock_work.h +29 −22
kernel/bpf/stackmap.c +17 −11
kernel/bpf/task_iter.c +10 −4

kernel/bpf/mmap_unlock_work.h+29−22

@@ -4,12 +4,15 @@ · 3 unmodified lines
4	4
5	5	#ifndef __MMAP_UNLOCK_WORK_H__
6	6	#define __MMAP_UNLOCK_WORK_H__
	7	+#include <linux/atomic.h>
	8	+#include <linux/err.h>
7	9	#include <linux/irq_work.h>
8	10
9	11	/* irq_work to run mmap_read_unlock() in irq_work */
10	12	struct mmap_unlock_irq_work {
11	13	struct irq_work irq_work;
12	14	struct mm_struct *mm;
	15	+ atomic_t active;
13	16	};
14	17
15	18	DECLARE_PER_CPU(struct mmap_unlock_irq_work, mmap_unlock_work);
@@ -18,32 +21,36 @@ DECLARE_PER_CPU(struct mmap_unlock_irq_work, mmap_unlock_work); · 2 unmodified lines
18	21	* We cannot do mmap_read_unlock() when the irq is disabled, because of
19	22	* risk to deadlock with rq_lock. To look up vma when the irqs are
20	23	* disabled, we need to run mmap_read_unlock() in irq_work. We use a
21		− * percpu variable to do the irq_work. If the irq_work is already used
22		− * by another lookup, we fall over.
	24	+ * percpu variable to do the irq_work. The active flag reserves the slot
	25	+ * before mmap_read_trylock() and until the irq_work callback consumes mm.
23	26	*/
24		−static inline bool bpf_mmap_unlock_get_irq_work(struct mmap_unlock_irq_work **work_ptr)
	27	+static inline struct mmap_unlock_irq_work *bpf_mmap_unlock_guard_get(void)
25	28	{
26		− struct mmap_unlock_irq_work *work = NULL;
27		− bool irq_work_busy = false;
	29	+ struct mmap_unlock_irq_work *work;
28	30
29		− if (irqs_disabled()) {
30		− if (!IS_ENABLED(CONFIG_PREEMPT_RT)) {
31		− work = this_cpu_ptr(&mmap_unlock_work);
32		− if (irq_work_is_busy(&work->irq_work)) {
33		− /* cannot queue more up_read, fallback */
34		− irq_work_busy = true;
35		− }
36		− } else {
37		− /*
38		− * PREEMPT_RT does not allow to trylock mmap sem in
39		− * interrupt disabled context. Force the fallback code.
40		− */
41		− irq_work_busy = true;
42		− }
43		− }
	31	+ if (!irqs_disabled())
	32	+ return NULL;
	33	+
	34	+ /*
	35	+ * PREEMPT_RT does not allow to trylock mmap sem in interrupt
	36	+ * disabled context. Force the fallback code.
	37	+ */
	38	+ if (IS_ENABLED(CONFIG_PREEMPT_RT))
	39	+ return ERR_PTR(-EBUSY);
	40	+
	41	+ work = this_cpu_ptr(&mmap_unlock_work);
	42	+ if (irq_work_is_busy(&work->irq_work) \|\|
	43	+ atomic_cmpxchg_acquire(&work->active, 0, 1))
	44	+ return ERR_PTR(-EBUSY);
44	45
45		− *work_ptr = work;
46		− return irq_work_busy;
	46	+ return work;
	47	+}
	48	+
	49	+static inline void
	50	+bpf_mmap_unlock_guard_put(struct mmap_unlock_irq_work *work)
	51	+{
	52	+ if (work)
	53	+ atomic_set_release(&work->active, 0);
47	54	}
48	55
49	56	static inline void bpf_mmap_unlock_mm(struct mmap_unlock_irq_work work, struct mm_struct mm)

kernel/bpf/stackmap.c+17−11

@@ -414,8 +414,7 @@ static void stack_map_get_build_id_offset_sleepable(struct bpf_stack_build_id *i · 413 unmodified lines
414	414	static void stack_map_get_build_id_offset(struct bpf_stack_build_id *id_offs,
415	415	u32 trace_nr, bool user, bool may_fault)
416	416	{
417		− struct mmap_unlock_irq_work *work = NULL;
418		− bool irq_work_busy = bpf_mmap_unlock_get_irq_work(&work);
	417	+ struct mmap_unlock_irq_work *work;
419	418	bool has_user_ctx = user && current && current->mm;
420	419	struct stack_map_build_id_cache cache = {};
421	420	struct vm_area_struct *vma;
@@ -426,15 +425,16 @@ static void stack_map_get_build_id_offset(struct bpf_stack_build_id *id_offs, · 4 unmodified lines
426	425	return;
427	426	}
428	427
429		− /* If the irq_work is in use, fall back to report ips. Same
430		− * fallback is used for kernel stack (!user) on a stackmap with
431		− * build_id.
432		− */
433		− if (!has_user_ctx \|\| irq_work_busy \|\| !mmap_read_trylock(current->mm)) {
434		− /* cannot access current->mm, fall back to ips */
435		− for (i = 0; i < trace_nr; i++)
436		− stack_map_build_id_set_ip(&id_offs[i]);
437		− return;
	428	+ if (!has_user_ctx)
	429	+ goto fallback;
	430	+
	431	+ work = bpf_mmap_unlock_guard_get();
	432	+ if (IS_ERR(work))
	433	+ goto fallback;
	434	+
	435	+ if (!mmap_read_trylock(current->mm)) {
	436	+ bpf_mmap_unlock_guard_put(work);
	437	+ goto fallback;
438	438	}
439	439
440	440	for (i = 0; i < trace_nr; i++) {
@@ -465,6 +465,12 @@ static void stack_map_get_build_id_offset(struct bpf_stack_build_id *id_offs, · 24 unmodified lines
465	465	vma->vm_pgoff);
466	466	}
467	467	bpf_mmap_unlock_mm(work, current->mm);
	468	+ return;
	469	+
	470	+fallback:
	471	+ /* cannot access current->mm, fall back to ips */
	472	+ for (i = 0; i < trace_nr; i++)
	473	+ stack_map_build_id_set_ip(&id_offs[i]);
468	474	}
469	475
470	476	static struct perf_callchain_entry *

kernel/bpf/task_iter.c+10−4

@@ -753,9 +753,8 @@ static struct bpf_iter_reg task_vma_reg_info = { · 752 unmodified lines
753	753	BPF_CALL_5(bpf_find_vma, struct task_struct *, task, u64, start,
754	754	bpf_callback_t, callback_fn, void *, callback_ctx, u64, flags)
755	755	{
756		− struct mmap_unlock_irq_work *work = NULL;
	756	+ struct mmap_unlock_irq_work *work;
757	757	struct vm_area_struct *vma;
758		− bool irq_work_busy = false;
759	758	bool __maybe_unused mmput_needed = false;
760	759	struct mm_struct *mm;
761	760	int ret = -ENOENT;
@@ -792,9 +791,14 @@ BPF_CALL_5(bpf_find_vma, struct task_struct *, task, u64, start, · 30 unmodified lines
792	791	if (!mm)
793	792	return -ENOENT;
794	793
795		− irq_work_busy = bpf_mmap_unlock_get_irq_work(&work);
	794	+ work = bpf_mmap_unlock_guard_get();
	795	+ if (IS_ERR(work)) {
	796	+ ret = PTR_ERR(work);
	797	+ goto out;
	798	+ }
796	799
797		− if (irq_work_busy \|\| !mmap_read_trylock(mm)) {
	800	+ if (!mmap_read_trylock(mm)) {
	801	+ bpf_mmap_unlock_guard_put(work);
798	802	ret = -EBUSY;
799	803	goto out;
800	804	}
@@ -1191,6 +1195,8 @@ static void do_mmap_read_unlock(struct irq_work *entry) · 390 unmodified lines
1191	1195
1192	1196	work = container_of(entry, struct mmap_unlock_irq_work, irq_work);
1193	1197	mmap_read_unlock_non_owner(work->mm);
	1198	+ work->mm = NULL;
	1199	+ bpf_mmap_unlock_guard_put(work);
1194	1200	}
1195	1201
1196	1202	static int __init task_iter_init(void)