[PATCH bpf] bpf: fix UAF of trampoline progs before image release

[PATCH bpf] bpf: fix UAF of trampoline progs before image release

[Junseo Lim]

arch_prepare_bpf_trampoline() embeds program pointers in the generated
image and passes them to __bpf_prog_enter_recur(). After
bpf_trampoline_update() replaces the image, bpf_tramp_image_put() can keep
the old image executable past a normal RCU grace period.

A detached non-sleepable prog can therefore be freed before a preempted
task reaches rcu_read_lock_dont_migrate() in __bpf_prog_enter_recur(),
leading to a use-after-free.

Keep image-local prog refs and drop them from bpf_tramp_image_free().

Fixes: e21aa341785c ("bpf: Fix fexit trampoline.")
Reported-by: Sechang Lim <rhkrqnwk98@gmail.com>
Signed-off-by: Junseo Lim <zirajs7@gmail.com>
---
This issue was found by a custom fuzzer developed by
Sechang Lim <rhkrqnwk98@gmail.com>.

This issue was reproduced on bpf/master commit a13307e97d5c.
Below is the KASAN report:

    ==================================================================
    BUG: KASAN: vmalloc-out-of-bounds in __bpf_prog_enter_recur+0x3a5/0x3f0
    Read of size 8 at addr ffffc90000081040 by task candidate/114
    
    CPU: 0 UID: 0 PID: 114 Comm: candidate Not tainted 7.2.0-rc6-00341-ga13307e97d5c #120 PREEMPT(full)
    Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Arch Linux 1.17.0-2-2 04/01/2014
    Call Trace:
     <TASK>
     dump_stack_lvl+0xb0/0x110
     print_report+0x14b/0x4a4
     ? preempt_schedule_notrace_thunk+0x16/0x30
     ? srso_alias_return_thunk+0x5/0xfbef5
     ? preempt_schedule_notrace_thunk+0x16/0x30
     kasan_report+0x108/0x130
     ? __bpf_prog_enter_recur+0x3a5/0x3f0
     ? __bpf_prog_enter_recur+0x3a5/0x3f0
     __bpf_prog_enter_recur+0x3a5/0x3f0
     bpf_trampoline_6442508591+0x32/0xa7
     __x64_sys_futex+0x9/0x410
     do_syscall_64+0xae/0x5e0
     ? srso_alias_return_thunk+0x5/0xfbef5
     ? srso_alias_return_thunk+0x5/0xfbef5
     entry_SYSCALL_64_after_hwframe+0x76/0x7e
    RIP: 0033:0x429f4d
    Code: d5 48 8d 3c 0a eb 91 66 0f 1f 44 00 00 f3 0f 1e fa 48 89 f8 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 c7 c1 e8 ff ff ff f7 d8 64 89 01 48
    RSP: 002b:00007f2332067128 EFLAGS: 00000246 ORIG_RAX: 00000000000000ca
    RAX: ffffffffffffffda RBX: 00007f2332067ce4 RCX: 0000000000429f4d
    RDX: 0000000000000000 RSI: 0000000000000001 RDI: 0000000000000000
    RBP: 00007f23320672b0 R08: 0000000000000000 R09: 0000000000000000
    R10: 0000000000000000 R11: 0000000000000246 R12: 0000000000000021
    R13: 00007ffd9ad0b660 R14: 0000000000000010 R15: 00007ffd9ad0b757
     </TASK>
    
    The buggy address belongs to a vmalloc virtual mapping
    Memory state around the buggy address:
     ffffc90000080f00: f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8
     ffffc90000080f80: f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8
    >ffffc90000081000: f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8
                                               ^
     ffffc90000081080: f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8
     ffffc90000081100: f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8
    ==================================================================

 include/linux/bpf.h     |  5 +++++
 kernel/bpf/trampoline.c | 20 ++++++++++++++++++++
 2 files changed, 25 insertions(+)

diff --git a/include/linux/bpf.h b/include/linux/bpf.h
index 7719f6528445..bacb6bc2e27b 100644
--- a/include/linux/bpf.h
+++ b/include/linux/bpf.h
@@ -1376,6 +1376,11 @@ struct bpf_tramp_image {
 		struct rcu_head rcu;
 		struct work_struct work;
 	};
+#ifdef CONFIG_PREEMPTION
+	/* Programs called from this image must outlive deferred image freeing. */
+	struct bpf_prog *progs[BPF_MAX_TRAMP_LINKS];
+	int nr_progs;
+#endif
 };
 
 struct bpf_trampoline {
diff --git a/kernel/bpf/trampoline.c b/kernel/bpf/trampoline.c
index 1a721fc4bef5..ff6b331c965b 100644
--- a/kernel/bpf/trampoline.c
+++ b/kernel/bpf/trampoline.c
@@ -531,6 +531,13 @@ bpf_trampoline_get_progs(const struct bpf_trampoline *tr, int *total, bool *ip_a
 
 static void bpf_tramp_image_free(struct bpf_tramp_image *im)
 {
+#ifdef CONFIG_PREEMPTION
+	int i;
+
+	for (i = 0; i < im->nr_progs; i++)
+		bpf_prog_put(im->progs[i]);
+#endif
+
 	bpf_image_ksym_del(&im->ksym);
 	arch_free_bpf_trampoline(im->image, im->size);
 	bpf_jit_uncharge_modmem(im->size);
@@ -678,6 +685,9 @@ static int bpf_trampoline_update(struct bpf_trampoline *tr, bool lock_direct_mut
 	u32 orig_flags = tr->flags;
 	bool ip_arg = false;
 	int err, total, size;
+#ifdef CONFIG_PREEMPTION
+	int i, kind;
+#endif
 
 	tnodes = bpf_trampoline_get_progs(tr, &total, &ip_arg);
 	if (IS_ERR(tnodes))
@@ -740,6 +750,16 @@ static int bpf_trampoline_update(struct bpf_trampoline *tr, bool lock_direct_mut
 		goto out;
 	}
 
+#ifdef CONFIG_PREEMPTION
+	for (kind = 0; kind < BPF_TRAMP_MAX; kind++)
+		for (i = 0; i < tnodes[kind].nr_nodes; i++) {
+			struct bpf_prog *prog = tnodes[kind].nodes[i]->link->prog;
+
+			bpf_prog_inc(prog);
+			im->progs[im->nr_progs++] = prog;
+		}
+#endif
+
 	err = arch_prepare_bpf_trampoline(im, im->image, im->image + size,
 					  &tr->func.model, tr->flags, tnodes,
 					  tr->func.addr);
-- 
2.55.0

Re: [PATCH bpf] bpf: fix UAF of trampoline progs before image release

[bot+bpf-ci]

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

> diff --git a/include/linux/bpf.h b/include/linux/bpf.h
> index 7719f6528445..bacb6bc2e27b 100644
> --- a/include/linux/bpf.h
> +++ b/include/linux/bpf.h
> @@ -1376,6 +1376,11 @@ struct bpf_tramp_image {
>  		struct rcu_head rcu;
>  		struct work_struct work;
>  	};
> +#ifdef CONFIG_PREEMPTION
> +	/* Programs called from this image must outlive deferred image freeing. */
> +	struct bpf_prog *progs[BPF_MAX_TRAMP_LINKS];
> +	int nr_progs;
> +#endif
>  };

This isn't a bug, but since struct bpf_tramp_image is also instantiated on
the stack by arch_bpf_trampoline_size() on several architectures, would a
separately kmalloc'd array of size total (or reusing the tnodes array that
bpf_trampoline_update() already has) keep both the struct and those stack
frames from growing by around 312 bytes?

> diff --git a/kernel/bpf/trampoline.c b/kernel/bpf/trampoline.c
> index 1a721fc4bef5..ff6b331c965b 100644
> --- a/kernel/bpf/trampoline.c
> +++ b/kernel/bpf/trampoline.c
> @@ -531,6 +531,13 @@ bpf_trampoline_get_progs(const struct bpf_trampoline *tr, int *total, bool *ip_a
>
>  static void bpf_tramp_image_free(struct bpf_tramp_image *im)
>  {
> +#ifdef CONFIG_PREEMPTION
> +	int i;
> +
> +	for (i = 0; i < im->nr_progs; i++)
> +		bpf_prog_put(im->progs[i]);
> +#endif
> +
>  	bpf_image_ksym_del(&im->ksym);
>  	arch_free_bpf_trampoline(im->image, im->size);
>  	bpf_jit_uncharge_modmem(im->size);
> @@ -678,6 +685,9 @@ static int bpf_trampoline_update(struct bpf_trampoline *tr, bool lock_direct_mut
>  	u32 orig_flags = tr->flags;
>  	bool ip_arg = false;
>  	int err, total, size;
> +#ifdef CONFIG_PREEMPTION
> +	int i, kind;
> +#endif
>
>  	tnodes = bpf_trampoline_get_progs(tr, &total, &ip_arg);
>  	if (IS_ERR(tnodes))
> @@ -740,6 +750,16 @@ static int bpf_trampoline_update(struct bpf_trampoline *tr, bool lock_direct_mut
>  		goto out;
>  	}
>
> +#ifdef CONFIG_PREEMPTION
> +	for (kind = 0; kind < BPF_TRAMP_MAX; kind++)
> +		for (i = 0; i < tnodes[kind].nr_nodes; i++) {
> +			struct bpf_prog *prog = tnodes[kind].nodes[i]->link->prog;
> +
> +			bpf_prog_inc(prog);
> +			im->progs[im->nr_progs++] = prog;
> +		}
> +#endif
> +
>  	err = arch_prepare_bpf_trampoline(im, im->image, im->image + size,
>  					  &tr->func.model, tr->flags, tnodes,
>  					  tr->func.addr);

This isn't a bug, but could the changelog say a sentence about why the
new references are only needed under CONFIG_PREEMPTION, mirroring the
!PREEMPT reasoning already written out in bpf_tramp_image_put()?


---
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/31872362891

Re: [PATCH bpf] bpf: fix UAF of trampoline progs before image release

[Leon Hwang]

On 15/8/26 15:19, Junseo Lim wrote:
[...]
> diff --git a/include/linux/bpf.h b/include/linux/bpf.h
> index 7719f6528445..bacb6bc2e27b 100644
> --- a/include/linux/bpf.h
> +++ b/include/linux/bpf.h
> @@ -1376,6 +1376,11 @@ struct bpf_tramp_image {
>  		struct rcu_head rcu;
>  		struct work_struct work;
>  	};
> +#ifdef CONFIG_PREEMPTION
> +	/* Programs called from this image must outlive deferred image freeing. */
> +	struct bpf_prog *progs[BPF_MAX_TRAMP_LINKS];
> +	int nr_progs;
> +#endif
>  };
>  
>  struct bpf_trampoline {
> diff --git a/kernel/bpf/trampoline.c b/kernel/bpf/trampoline.c
> index 1a721fc4bef5..ff6b331c965b 100644
> --- a/kernel/bpf/trampoline.c
> +++ b/kernel/bpf/trampoline.c
> @@ -531,6 +531,13 @@ bpf_trampoline_get_progs(const struct bpf_trampoline *tr, int *total, bool *ip_a
>  
>  static void bpf_tramp_image_free(struct bpf_tramp_image *im)
>  {
> +#ifdef CONFIG_PREEMPTION
> +	int i;
> +
> +	for (i = 0; i < im->nr_progs; i++)
> +		bpf_prog_put(im->progs[i]);
> +#endif

Looks better to factor out a helper for it, e.g.
bpf_tramp_image_put_progs().

> +
>  	bpf_image_ksym_del(&im->ksym);
>  	arch_free_bpf_trampoline(im->image, im->size);
>  	bpf_jit_uncharge_modmem(im->size);
> @@ -678,6 +685,9 @@ static int bpf_trampoline_update(struct bpf_trampoline *tr, bool lock_direct_mut
>  	u32 orig_flags = tr->flags;
>  	bool ip_arg = false;
>  	int err, total, size;
> +#ifdef CONFIG_PREEMPTION
> +	int i, kind;
> +#endif
>  
>  	tnodes = bpf_trampoline_get_progs(tr, &total, &ip_arg);
>  	if (IS_ERR(tnodes))
> @@ -740,6 +750,16 @@ static int bpf_trampoline_update(struct bpf_trampoline *tr, bool lock_direct_mut
>  		goto out;
>  	}
>  
> +#ifdef CONFIG_PREEMPTION
> +	for (kind = 0; kind < BPF_TRAMP_MAX; kind++)
> +		for (i = 0; i < tnodes[kind].nr_nodes; i++) {
> +			struct bpf_prog *prog = tnodes[kind].nodes[i]->link->prog;
> +
> +			bpf_prog_inc(prog);
> +			im->progs[im->nr_progs++] = prog;
> +		}
> +#endif

Ditto.

Thanks,
Leon

> +
>  	err = arch_prepare_bpf_trampoline(im, im->image, im->image + size,
>  					  &tr->func.model, tr->flags, tnodes,
>  					  tr->func.addr);