[PATCH bpf] bpf: potential double-free of env->insn_aux_data

[PATCH bpf] bpf: potential double-free of env->insn_aux_data

[Eduard Zingerman]

Function bpf_patch_insn_data() has the following structure:

  static struct bpf_prog *bpf_patch_insn_data(... env ...)
  {
        struct bpf_prog *new_prog;
        struct bpf_insn_aux_data *new_data = NULL;

        if (len > 1) {
                new_data = vrealloc(...);  // <--------- (1)
                if (!new_data)
                        return NULL;

                env->insn_aux_data = new_data;  // <---- (2)
        }

        new_prog = bpf_patch_insn_single(env->prog, off, patch, len);
        if (IS_ERR(new_prog)) {
                ...
                vfree(new_data);   // <----------------- (3)
                return NULL;
        }
        ... happy path ...
  }

In case if bpf_patch_insn_single() returns an error the `new_data`
allocated at (1) will be freed at (3). However, at (2) this pointer
is stored in `env->insn_aux_data`. Which is freed unconditionally
by verifier.c:bpf_check() on both happy and error paths.
Thus, leading to double-free.

Fix this by removing vfree() call at (3), ownership over `new_data` is
already passed to `env->insn_aux_data` at this point.

Fixes: 77620d126739 ("bpf: use realloc in bpf_patch_insn_data")
Reported-by: Chris Mason <clm@meta.com>
Signed-off-by: Eduard Zingerman <eddyz87@gmail.com>
---
 kernel/bpf/verifier.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index 9fb1f957a09374e4d148402572b872bec930f34c..92eb0f4e87a4ec3a7e303c6949cb415e3fd0b4ac 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -20781,7 +20781,6 @@ static struct bpf_prog *bpf_patch_insn_data(struct bpf_verifier_env *env, u32 of
 			verbose(env,
 				"insn %d cannot be patched due to 16-bit range\n",
 				env->insn_aux_data[off].orig_idx);
-		vfree(new_data);
 		return NULL;
 	}
 	adjust_insn_aux_data(env, new_data, new_prog, off, len);

---
base-commit: 22f20375f5b71f30c0d6896583b93b6e4bba7279
change-id: 20250912-patch-insn-data-double-free-f7be62b9c4a9

Re: [PATCH bpf] bpf: potential double-free of env->insn_aux_data

[Daniel Borkmann]

On 9/12/25 9:18 PM, Eduard Zingerman wrote:
> Function bpf_patch_insn_data() has the following structure:
> 
>    static struct bpf_prog *bpf_patch_insn_data(... env ...)
>    {
>          struct bpf_prog *new_prog;
>          struct bpf_insn_aux_data *new_data = NULL;
> 
>          if (len > 1) {
>                  new_data = vrealloc(...);  // <--------- (1)
>                  if (!new_data)
>                          return NULL;
> 
>                  env->insn_aux_data = new_data;  // <---- (2)
>          }
> 
>          new_prog = bpf_patch_insn_single(env->prog, off, patch, len);
>          if (IS_ERR(new_prog)) {
>                  ...
>                  vfree(new_data);   // <----------------- (3)
>                  return NULL;
>          }
>          ... happy path ...
>    }
> 
> In case if bpf_patch_insn_single() returns an error the `new_data`
> allocated at (1) will be freed at (3). However, at (2) this pointer
> is stored in `env->insn_aux_data`. Which is freed unconditionally
> by verifier.c:bpf_check() on both happy and error paths.
> Thus, leading to double-free.
> 
> Fix this by removing vfree() call at (3), ownership over `new_data` is
> already passed to `env->insn_aux_data` at this point.
> 
> Fixes: 77620d126739 ("bpf: use realloc in bpf_patch_insn_data")
> Reported-by: Chris Mason <clm@meta.com>
> Signed-off-by: Eduard Zingerman <eddyz87@gmail.com>
> ---
>   kernel/bpf/verifier.c | 1 -
>   1 file changed, 1 deletion(-)
> 
> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> index 9fb1f957a09374e4d148402572b872bec930f34c..92eb0f4e87a4ec3a7e303c6949cb415e3fd0b4ac 100644
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c
> @@ -20781,7 +20781,6 @@ static struct bpf_prog *bpf_patch_insn_data(struct bpf_verifier_env *env, u32 of
>   			verbose(env,
>   				"insn %d cannot be patched due to 16-bit range\n",
>   				env->insn_aux_data[off].orig_idx);
> -		vfree(new_data);

I presume you mean bpf-next tree, otherwise we'd be adding a memory leak into bpf tree?

>   		return NULL;
>   	}
>   	adjust_insn_aux_data(env, new_data, new_prog, off, len);
Thanks,
Daniel

Re: [PATCH bpf] bpf: potential double-free of env->insn_aux_data

[Eduard Zingerman]

On Mon, 2025-09-15 at 13:42 +0200, Daniel Borkmann wrote:

[...]

> > diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> > index 9fb1f957a09374e4d148402572b872bec930f34c..92eb0f4e87a4ec3a7e303c6949cb415e3fd0b4ac 100644
> > --- a/kernel/bpf/verifier.c
> > +++ b/kernel/bpf/verifier.c
> > @@ -20781,7 +20781,6 @@ static struct bpf_prog *bpf_patch_insn_data(struct bpf_verifier_env *env, u32 of
> >   			verbose(env,
> >   				"insn %d cannot be patched due to 16-bit range\n",
> >   				env->insn_aux_data[off].orig_idx);
> > -		vfree(new_data);
> 
> I presume you mean bpf-next tree, otherwise we'd be adding a memory leak into bpf tree?

Hi Daniel,

Yes, the tag should be bpf-next.
Will resend with the correct tag.
I apologize, this was sloppy on my side.

[...]

Re: [PATCH bpf] bpf: potential double-free of env->insn_aux_data

[Alexei Starovoitov]

On Mon, Sep 15, 2025 at 11:21 AM Eduard Zingerman <eddyz87@gmail.com> wrote:
>
> On Mon, 2025-09-15 at 13:42 +0200, Daniel Borkmann wrote:
>
> [...]
>
> > > diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> > > index 9fb1f957a09374e4d148402572b872bec930f34c..92eb0f4e87a4ec3a7e303c6949cb415e3fd0b4ac 100644
> > > --- a/kernel/bpf/verifier.c
> > > +++ b/kernel/bpf/verifier.c
> > > @@ -20781,7 +20781,6 @@ static struct bpf_prog *bpf_patch_insn_data(struct bpf_verifier_env *env, u32 of
> > >                     verbose(env,
> > >                             "insn %d cannot be patched due to 16-bit range\n",
> > >                             env->insn_aux_data[off].orig_idx);
> > > -           vfree(new_data);
> >
> > I presume you mean bpf-next tree, otherwise we'd be adding a memory leak into bpf tree?
>
> Hi Daniel,
>
> Yes, the tag should be bpf-next.
> Will resend with the correct tag.

No need to resend.