Re: [PATCH bpf-next v2 5/5] x86/bpf: enable EXECMEM_ROX_CACHE for BPF allocations

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Uninitialized kernel memory from vmalloc/kvmalloc is copied into executable ROX BPF images (dispatcher and trampoline), causing an information leak and a wild execution risk.
--

--- Patch [5]: [PATCH bpf-next v2 5/5] x86/bpf: enable EXECMEM_ROX_CACHE for BPF allocations ---
commit 12eba9f58f961b936cd08cfa17b30375eb214d2a
Author: Mike Rapoport (Microsoft) <rppt@kernel.org>

bpf: dispatcher: allocate bpf_dispatcher->rw_image with vmalloc()

This commit changes the allocation of bpf_dispatcher->rw_image to use
vmalloc() instead of bpf_jit_alloc_exec(), avoiding a permissions dance
when EXECMEM_BPF is backed by ROX caches. It relies on the fact that rel32
offsets are calculated against the actual ROX image, so the temporary
buffer does not need to reside in the module address range.

> diff --git a/kernel/bpf/dispatcher.c b/kernel/bpf/dispatcher.c
> index ea2d60dc1feeb..d85b3e8ad2e4e 100644
> --- a/kernel/bpf/dispatcher.c
> +++ b/kernel/bpf/dispatcher.c
> @@ -148,7 +148,10 @@ void bpf_dispatcher_change_prog(struct bpf_dispatcher *d, struct bpf_prog *from,
>  		d->image = bpf_prog_pack_alloc(PAGE_SIZE, bpf_jit_fill_hole_with_zero, false);
>  		if (!d->image)
>  			goto out;
> -		d->rw_image = bpf_jit_alloc_exec(PAGE_SIZE);
> +		/* d->rw_image doesn't need to be in module memory range, so we
> +		 * can use vmalloc.
> +		 */
> +		d->rw_image = vmalloc(PAGE_SIZE);

[Severity: High]
Does this code leak uninitialized kernel memory into the executable ROX BPF
image?

By changing the allocation to vmalloc() without zeroing, any trailing bytes
beyond the emitted instructions are left uninitialized.

When this buffer is later used in bpf_dispatcher_update(), a full 2048 bytes
are copied:

kernel/bpf/dispatcher.c:bpf_dispatcher_update() {
	if (bpf_dispatcher_prepare(d, new, tmp))
		return;
	if (IS_ERR(bpf_arch_text_copy(new, tmp, PAGE_SIZE / 2)))
		return;
}

Because the uninitialized bytes (which can decode into random executable
gadgets) replace the intended trap or padding instructions (e.g., INT3 or
zeroes), does this also create a wild execution risk if execution falls
through past the generated code?

This is a pre-existing issue, but the same pattern appears in
arch_prepare_bpf_trampoline():

arch/x86/net/bpf_jit_comp.c:arch_prepare_bpf_trampoline() {
	/* rw_image doesn't need to be in module memory range, so we can
	 * use kvmalloc.
	 */
	rw_image = kvmalloc(size, GFP_KERNEL);
	if (!rw_image)
		return -ENOMEM;

	ret = __arch_prepare_bpf_trampoline(im, rw_image, rw_image + size, image, m,
					    flags, tnodes, func_addr);
	if (ret < 0)
		goto out;

	tmp = bpf_arch_text_copy(image, rw_image, size);
}

Because kvmalloc() does not zero-initialize the memory, is BPF_INSN_SAFETY
padding left uninitialized before being unconditionally copied to the ROX
image?

>  		if (!d->rw_image) {
>  			bpf_prog_pack_free(d->image, PAGE_SIZE);
>  			d->image = NULL;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260711-execmem-x86-rox-bpf-v0-v2-0-bfd956d35119@kernel.org?part=5