Skip to content

bpf: fix UAF of trampoline progs before image release - #13316

Closed
kernel-patches-daemon-bpf[bot] wants to merge 1 commit into
bpf_basefrom
series/1146410=>bpf
Closed

bpf: fix UAF of trampoline progs before image release#13316
kernel-patches-daemon-bpf[bot] wants to merge 1 commit into
bpf_basefrom
series/1146410=>bpf

Conversation

@kernel-patches-daemon-bpf

Copy link
Copy Markdown

Pull request for series with
subject: bpf: fix UAF of trampoline progs before image release
version: 1
url: https://patchwork.kernel.org/project/netdevbpf/list/?series=1146410

@kernel-patches-daemon-bpf

Copy link
Copy Markdown
Author

Upstream branch: a13307e
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=1146410
version: 1

@kernel-patches-review-bot

Copy link
Copy Markdown
commit 2fec56b377602879b31c6b61f96680bebda8f5ec
Author: Junseo Lim <zirajs7@gmail.com>

bpf: fix UAF of trampoline progs before image release

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().

> 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

In-Reply-To-Subject: bpf: fix UAF of trampoline progs before image release
CI run summary: https://github.com/kernel-patches/bpf/actions/runs/31872362891

@kernel-patches-daemon-bpf

Copy link
Copy Markdown
Author

@kernel-patches-daemon-bpf

Copy link
Copy Markdown
Author

Upstream branch: a13307e
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=1146410
version: 1

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: e21aa34 ("bpf: Fix fexit trampoline.")
Reported-by: Sechang Lim <rhkrqnwk98@gmail.com>
Signed-off-by: Junseo Lim <zirajs7@gmail.com>
@kernel-patches-daemon-bpf

Copy link
Copy Markdown
Author

At least one diff in series https://patchwork.kernel.org/project/netdevbpf/list/?series=1146410 expired. Closing PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant