[PATCH bpf-next v3] bpf: Fix mmap_lock leak in irq_work path

[PATCH bpf-next v3] bpf: Fix mmap_lock leak in irq_work path

[Sanghyun Park]

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.

Fixes: eac9153f2b58 ("bpf/stackmap: Fix deadlock with rq_lock in bpf_get_stack()")
Reported-by: syzbot+cdd6c0925e12b0af60cc@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=cdd6c0925e12b0af60cc
Reported-by: sashiko-bot@kernel.org
Closes: https://lore.kernel.org/r/20260630033745.B80201F000E9@smtp.kernel.org
Signed-off-by: Sanghyun Park <sanghyun.park.cnu@gmail.com>
---
v3:
 - Return the reserved work item directly and use ERR_PTR(-EBUSY), as suggested
   by Andrii.
 - Keep irq_work_is_busy() alongside the active reservation.
 - Update the Fixes tag.

Sun, Puranjay, Ihor,

I dropped the Tested-by, Reviewed-by, and Acked-by tags because v3 changes
the helper interface and restores the irq_work busy check. If you have a
chance, could you please review or retest this version?

v2: https://lore.kernel.org/bpf/20260730054858.209807-2-sanghyun.park.cnu@gmail.com/
 - Drop irq_work_is_busy() and rely exclusively on active, as suggested by
   Ihor.

v1: https://lore.kernel.org/bpf/20260722023004.1497923-2-sanghyun.park.cnu@gmail.com/

 kernel/bpf/mmap_unlock_work.h | 51 ++++++++++++++++++++---------------
 kernel/bpf/stackmap.c         | 28 +++++++++++--------
 kernel/bpf/task_iter.c        | 14 +++++++---
 3 files changed, 56 insertions(+), 37 deletions(-)

diff --git a/kernel/bpf/mmap_unlock_work.h b/kernel/bpf/mmap_unlock_work.h
index 5d18d7d85bef..1834db20b861 100644
--- a/kernel/bpf/mmap_unlock_work.h
+++ b/kernel/bpf/mmap_unlock_work.h
@@ -4,12 +4,15 @@
 
 #ifndef __MMAP_UNLOCK_WORK_H__
 #define __MMAP_UNLOCK_WORK_H__
+#include <linux/atomic.h>
+#include <linux/err.h>
 #include <linux/irq_work.h>
 
 /* irq_work to run mmap_read_unlock() in irq_work */
 struct mmap_unlock_irq_work {
 	struct irq_work irq_work;
 	struct mm_struct *mm;
+	atomic_t active;
 };
 
 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);
  * We cannot do mmap_read_unlock() when the irq is disabled, because of
  * risk to deadlock with rq_lock. To look up vma when the irqs are
  * disabled, we need to run mmap_read_unlock() in irq_work. We use a
- * percpu variable to do the irq_work. If the irq_work is already used
- * by another lookup, we fall over.
+ * percpu variable to do the irq_work. The active flag reserves the slot
+ * before mmap_read_trylock() and until the irq_work callback consumes mm.
  */
-static inline bool bpf_mmap_unlock_get_irq_work(struct mmap_unlock_irq_work **work_ptr)
+static inline struct mmap_unlock_irq_work *bpf_mmap_unlock_guard_get(void)
 {
-	struct mmap_unlock_irq_work *work = NULL;
-	bool irq_work_busy = false;
+	struct mmap_unlock_irq_work *work;
 
-	if (irqs_disabled()) {
-		if (!IS_ENABLED(CONFIG_PREEMPT_RT)) {
-			work = this_cpu_ptr(&mmap_unlock_work);
-			if (irq_work_is_busy(&work->irq_work)) {
-				/* cannot queue more up_read, fallback */
-				irq_work_busy = true;
-			}
-		} else {
-			/*
-			 * PREEMPT_RT does not allow to trylock mmap sem in
-			 * interrupt disabled context. Force the fallback code.
-			 */
-			irq_work_busy = true;
-		}
-	}
+	if (!irqs_disabled())
+		return NULL;
+
+	/*
+	 * PREEMPT_RT does not allow to trylock mmap sem in interrupt
+	 * disabled context. Force the fallback code.
+	 */
+	if (IS_ENABLED(CONFIG_PREEMPT_RT))
+		return ERR_PTR(-EBUSY);
+
+	work = this_cpu_ptr(&mmap_unlock_work);
+	if (irq_work_is_busy(&work->irq_work) ||
+	    atomic_cmpxchg_acquire(&work->active, 0, 1))
+		return ERR_PTR(-EBUSY);
 
-	*work_ptr = work;
-	return irq_work_busy;
+	return work;
+}
+
+static inline void
+bpf_mmap_unlock_guard_put(struct mmap_unlock_irq_work *work)
+{
+	if (work)
+		atomic_set_release(&work->active, 0);
 }
 
 static inline void bpf_mmap_unlock_mm(struct mmap_unlock_irq_work *work, struct mm_struct *mm)
diff --git a/kernel/bpf/stackmap.c b/kernel/bpf/stackmap.c
index 463f94ba1cc4..0384b32d88b5 100644
--- a/kernel/bpf/stackmap.c
+++ b/kernel/bpf/stackmap.c
@@ -414,8 +414,7 @@ static void stack_map_get_build_id_offset_sleepable(struct bpf_stack_build_id *i
 static void stack_map_get_build_id_offset(struct bpf_stack_build_id *id_offs,
 					  u32 trace_nr, bool user, bool may_fault)
 {
-	struct mmap_unlock_irq_work *work = NULL;
-	bool irq_work_busy = bpf_mmap_unlock_get_irq_work(&work);
+	struct mmap_unlock_irq_work *work;
 	bool has_user_ctx = user && current && current->mm;
 	struct stack_map_build_id_cache cache = {};
 	struct vm_area_struct *vma;
@@ -426,15 +425,16 @@ static void stack_map_get_build_id_offset(struct bpf_stack_build_id *id_offs,
 		return;
 	}
 
-	/* If the irq_work is in use, fall back to report ips. Same
-	 * fallback is used for kernel stack (!user) on a stackmap with
-	 * build_id.
-	 */
-	if (!has_user_ctx || irq_work_busy || !mmap_read_trylock(current->mm)) {
-		/* cannot access current->mm, fall back to ips */
-		for (i = 0; i < trace_nr; i++)
-			stack_map_build_id_set_ip(&id_offs[i]);
-		return;
+	if (!has_user_ctx)
+		goto fallback;
+
+	work = bpf_mmap_unlock_guard_get();
+	if (IS_ERR(work))
+		goto fallback;
+
+	if (!mmap_read_trylock(current->mm)) {
+		bpf_mmap_unlock_guard_put(work);
+		goto fallback;
 	}
 
 	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,
 						      vma->vm_pgoff);
 	}
 	bpf_mmap_unlock_mm(work, current->mm);
+	return;
+
+fallback:
+	/* cannot access current->mm, fall back to ips */
+	for (i = 0; i < trace_nr; i++)
+		stack_map_build_id_set_ip(&id_offs[i]);
 }
 
 static struct perf_callchain_entry *
diff --git a/kernel/bpf/task_iter.c b/kernel/bpf/task_iter.c
index b256fb9c1214..13e1aabe6f88 100644
--- a/kernel/bpf/task_iter.c
+++ b/kernel/bpf/task_iter.c
@@ -753,9 +753,8 @@ static struct bpf_iter_reg task_vma_reg_info = {
 BPF_CALL_5(bpf_find_vma, struct task_struct *, task, u64, start,
 	   bpf_callback_t, callback_fn, void *, callback_ctx, u64, flags)
 {
-	struct mmap_unlock_irq_work *work = NULL;
+	struct mmap_unlock_irq_work *work;
 	struct vm_area_struct *vma;
-	bool irq_work_busy = false;
 	bool __maybe_unused mmput_needed = false;
 	struct mm_struct *mm;
 	int ret = -ENOENT;
@@ -792,9 +791,14 @@ BPF_CALL_5(bpf_find_vma, struct task_struct *, task, u64, start,
 	if (!mm)
 		return -ENOENT;
 
-	irq_work_busy = bpf_mmap_unlock_get_irq_work(&work);
+	work = bpf_mmap_unlock_guard_get();
+	if (IS_ERR(work)) {
+		ret = PTR_ERR(work);
+		goto out;
+	}
 
-	if (irq_work_busy || !mmap_read_trylock(mm)) {
+	if (!mmap_read_trylock(mm)) {
+		bpf_mmap_unlock_guard_put(work);
 		ret = -EBUSY;
 		goto out;
 	}
@@ -1191,6 +1195,8 @@ static void do_mmap_read_unlock(struct irq_work *entry)
 
 	work = container_of(entry, struct mmap_unlock_irq_work, irq_work);
 	mmap_read_unlock_non_owner(work->mm);
+	work->mm = NULL;
+	bpf_mmap_unlock_guard_put(work);
 }
 
 static int __init task_iter_init(void)

-- 
2.48.1

Re: [PATCH bpf-next v3] bpf: Fix mmap_lock leak in irq_work path

[bot+bpf-ci]

[-- Attachment #1: Type: text/plain, Size: 6221 bytes --]

> diff --git a/kernel/bpf/mmap_unlock_work.h b/kernel/bpf/mmap_unlock_work.h
> index 5d18d7d85bef9..1834db20b8612 100644
> --- a/kernel/bpf/mmap_unlock_work.h
> +++ b/kernel/bpf/mmap_unlock_work.h
> @@ -4,12 +4,15 @@
>
>  #ifndef __MMAP_UNLOCK_WORK_H__
>  #define __MMAP_UNLOCK_WORK_H__
> +#include <linux/atomic.h>
> +#include <linux/err.h>
>  #include <linux/irq_work.h>
>
>  /* irq_work to run mmap_read_unlock() in irq_work */
>  struct mmap_unlock_irq_work {
>  	struct irq_work irq_work;
>  	struct mm_struct *mm;
> +	atomic_t active;
>  };
>
>  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);
>   * We cannot do mmap_read_unlock() when the irq is disabled, because of
>   * risk to deadlock with rq_lock. To look up vma when the irqs are
>   * disabled, we need to run mmap_read_unlock() in irq_work. We use a
> - * percpu variable to do the irq_work. If the irq_work is already used
> - * by another lookup, we fall over.
> + * percpu variable to do the irq_work. The active flag reserves the slot
> + * before mmap_read_trylock() and until the irq_work callback consumes mm.
>   */

The new comment states the reservation lasts "until the irq_work callback
consumes mm," but there are two cases where it ends earlier: when
irqs_disabled() is false the helper returns NULL and no slot is reserved
at all, and when mmap_read_trylock() fails the reservation is released
immediately by bpf_mmap_unlock_guard_put(). Would it read more clearly to
note that the reservation may be released early on trylock failure?

[ ... ]

> -static inline bool bpf_mmap_unlock_get_irq_work(struct mmap_unlock_irq_work **work_ptr)
> +static inline struct mmap_unlock_irq_work *bpf_mmap_unlock_guard_get(void)
>  {
> +	work = this_cpu_ptr(&mmap_unlock_work);
> +	if (irq_work_is_busy(&work->irq_work) ||
> +	    atomic_cmpxchg_acquire(&work->active, 0, 1))
> +		return ERR_PTR(-EBUSY);
>
> -	*work_ptr = work;
> -	return irq_work_busy;
> +	return work;
> +}
> +
> +static inline void
> +bpf_mmap_unlock_guard_put(struct mmap_unlock_irq_work *work)
> +{
> +	if (work)
> +		atomic_set_release(&work->active, 0);
>  }

This isn't a bug, but the rename to bpf_mmap_unlock_guard_get()/_put()
might be confusing here. In current kernel usage 'guard' typically refers
to the include/linux/cleanup.h scoped-guard family where release happens
automatically at scope exit. These helpers do the opposite: the
reservation is released manually, and in the success case ownership is
handed off to the irq_work callback in do_mmap_read_unlock() which is in
a different function. Would bpf_mmap_unlock_reserve()/_unreserve() signal
the manual cross-function lifetime more directly?

[ ... ]

> diff --git a/kernel/bpf/stackmap.c b/kernel/bpf/stackmap.c
> index 463f94ba1cc47..0384b32d88b57 100644
> --- a/kernel/bpf/stackmap.c
> +++ b/kernel/bpf/stackmap.c
> @@ -426,15 +425,16 @@ static void stack_map_get_build_id_offset(struct bpf_stack_build_id *id_offs,
>  		return;
>  	}
>
> -	/* If the irq_work is in use, fall back to report ips. Same
> -	 * fallback is used for kernel stack (!user) on a stackmap with
> -	 * build_id.
> -	 */
> -	if (!has_user_ctx || irq_work_busy || !mmap_read_trylock(current->mm)) {
> -		/* cannot access current->mm, fall back to ips */
> -		for (i = 0; i < trace_nr; i++)
> -			stack_map_build_id_set_ip(&id_offs[i]);
> -		return;
> +	if (!has_user_ctx)
> +		goto fallback;
> +
> +	work = bpf_mmap_unlock_guard_get();
> +	if (IS_ERR(work))
> +		goto fallback;
> +
> +	if (!mmap_read_trylock(current->mm)) {
> +		bpf_mmap_unlock_guard_put(work);
> +		goto fallback;
>  	}

[ ... ]

> @@ -465,6 +465,12 @@ static void stack_map_get_build_id_offset(struct bpf_stack_build_id *id_offs,
>  						      vma->vm_pgoff);
>  	}
>  	bpf_mmap_unlock_mm(work, current->mm);
> +	return;
> +
> +fallback:
> +	/* cannot access current->mm, fall back to ips */
> +	for (i = 0; i < trace_nr; i++)
> +		stack_map_build_id_set_ip(&id_offs[i]);
>  }

[ ... ]

> diff --git a/kernel/bpf/task_iter.c b/kernel/bpf/task_iter.c
> index b256fb9c1214e..13e1aabe6f886 100644
> --- a/kernel/bpf/task_iter.c
> +++ b/kernel/bpf/task_iter.c
> @@ -792,9 +791,14 @@ BPF_CALL_5(bpf_find_vma, struct task_struct *, task, u64, start,
>  	if (!mm)
>  		return -ENOENT;
>
> -	irq_work_busy = bpf_mmap_unlock_get_irq_work(&work);
> +	work = bpf_mmap_unlock_guard_get();
> +	if (IS_ERR(work)) {
> +		ret = PTR_ERR(work);
> +		goto out;
> +	}
>
> -	if (irq_work_busy || !mmap_read_trylock(mm)) {
> +	if (!mmap_read_trylock(mm)) {
> +		bpf_mmap_unlock_guard_put(work);
>  		ret = -EBUSY;
>  		goto out;
>  	}

[ ... ]

> @@ -1191,6 +1195,8 @@ static void do_mmap_read_unlock(struct irq_work *entry)
>
>  	work = container_of(entry, struct mmap_unlock_irq_work, irq_work);
>  	mmap_read_unlock_non_owner(work->mm);
> +	work->mm = NULL;
> +	bpf_mmap_unlock_guard_put(work);
>  }

The commit message states the Fixes tag points to commit eac9153f2b58
which widened the gate from in_nmi() to irqs_disabled(). Looking at the
code history, the unsafe pattern where the slot check happens before the
reservation was introduced by bae77c5eb5b2 in May 2018, but that commit
alone was not enough to trigger the bug. The reproducer depends on two
later additions: the mmap_lock tracepoints that fire inside
mmap_read_trylock(), and raw_tp programs that bypass the global per-CPU
bpf_prog_active counter and rely only on per-prog prog->active (see
kernel/trace/bpf_trace.c:1599 and include/linux/bpf.h:2158). All other
BPF entry points like kprobe, perf_event, and kprobe_multi gate on the
global bpf_prog_active and cannot nest. The commit message describes the
bug mechanism but does not mention that only raw_tp programs can actually
trigger this nesting path. Would adding one sentence about the raw_tp +
mmap_lock-tracepoint precondition make the explanation more complete for
readers trying to understand the reachability?


---
AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md

CI run summary: https://github.com/kernel-patches/bpf/actions/runs/30972744877

Re: [PATCH bpf-next v3] bpf: Fix mmap_lock leak in irq_work path

[patchwork-bot+netdevbpf]

Hello:

This patch was applied to bpf/bpf-next.git (master)
by Andrii Nakryiko <andrii@kernel.org>:

On Wed,  5 Aug 2026 12:14:25 +0900 you wrote:
> 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().
> 
> [...]

Here is the summary with links:
  - [bpf-next,v3] bpf: Fix mmap_lock leak in irq_work path
    https://git.kernel.org/bpf/bpf-next/c/51476f6a06ef

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html