Re: [PATCH bpf-next v2 1/5] bpf: dispatcher: allocate bpf_dispatcher->rw_image with vmalloc()

[Mike Rapoport <rppt@kernel.org>]

On Sat, Jul 11, 2026 at 09:55:04AM +0000, sashiko-bot@kernel.org wrote:
> bpf: dispatcher: allocate bpf_dispatcher->rw_image with vmalloc()
> 
> This commit changes bpf_dispatcher->rw_image allocation to use vmalloc()
> instead of bpf_jit_alloc_exec() since the rel32 offsets are calculated
> against the final image, removing the need for the temporary buffer to live
> 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]
> This isn't a bug introduced by this patch, but since we are changing the
> allocation here, does this code leak uninitialized kernel memory into the
> executable dispatcher image?
> 
> vmalloc() does not zero-initialize the memory, and neither did the previous
> bpf_jit_alloc_exec() call. When the architecture-specific code generates the
> dispatcher instructions into this buffer, it only writes a few hundred bytes.
> 
> Later, bpf_dispatcher_update() copies exactly 2048 bytes from this partially
> uninitialized temporary buffer into the executable RO+X dispatcher image:
> 
> bpf_dispatcher_update()
>    ...
>    bpf_arch_text_copy(new, tmp, PAGE_SIZE / 2);
> 
> Could this overwrite the safe int3 trap instructions placed by
> bpf_prog_pack_alloc() with random bytes from the uninitialized kernel memory,
> potentially creating unintended ROP/JOP gadgets? Would it be safer to use
> vzalloc() here instead?

This one seems real unless I'm missing something in how that memory is
initialized.

I'll change vmalloc() to vzalloc() to make this better, but the proper fix
to either initialize ->rw_image to invalid instructions or copy only the
actually generated part is out of scope for this set.
 
> >  		if (!d->rw_image) {
> >  			bpf_prog_pack_free(d->image, PAGE_SIZE);
> >  			d->image = NULL;

-- 
Sincerely yours,
Mike.