[PATCH bpf-next] bpf: cleanup aux->used_maps after jit

[PATCH bpf-next] bpf: cleanup aux->used_maps after jit

[Anton Protopopov]

In commit b4ce5923e780 ("bpf, x86: add new map type: instructions array")
env->used_map was copied to func[i]->aux->used_maps before jitting.
Clear these fields out after jitting such that pointer to freed memory
(env->used_maps is freed later) are not kept in a live data structure.

The reason why the copies were initially added is explained in
https://lore.kernel.org/bpf/20251105090410.1250500-1-a.s.protopopov@gmail.com

Suggested-by: Alexei Starovoitov <ast@kernel.org>
Fixes: b4ce5923e780 ("bpf, x86: add new map type: instructions array")
Signed-off-by: Anton Protopopov <a.s.protopopov@gmail.com>
---
 kernel/bpf/verifier.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index 2e170be647bd..766695491bc5 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -22266,6 +22266,15 @@ static int jit_subprogs(struct bpf_verifier_env *env)
 		cond_resched();
 	}
 
+	/*
+	 * Cleanup func[i]->aux fields which aren't required
+	 * or can become invalid in future
+	 */
+	for (i = 0; i < env->subprog_cnt; i++) {
+		func[i]->aux->used_maps = NULL;
+		func[i]->aux->used_map_cnt = 0;
+	}
+
 	/* finally lock prog and jit images for all functions and
 	 * populate kallsysm. Begin at the first subprogram, since
 	 * bpf_prog_load will add the kallsyms for the main program.
-- 
2.34.1

Re: [PATCH bpf-next] bpf: cleanup aux->used_maps after jit

[bot+bpf-ci]

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

> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> index 2e170be64..766695491 100644
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c
> @@ -22266,6 +22266,15 @@ static int jit_subprogs(struct bpf_verifier_env *env)
>  		cond_resched();
>  	}
>
> +	/*
> +	 * Cleanup func[i]->aux fields which aren't required
> +	 * or can become invalid in future
> +	 */
> +	for (i = 0; i < env->subprog_cnt; i++) {
> +		func[i]->aux->used_maps = NULL;
> +		func[i]->aux->used_map_cnt = 0;
> +	}
> +

The patch correctly fixes the use-after-free issue. However, this isn't a
bug, but should this have a Cc: stable@vger.kernel.org tag? The bug being
fixed affects released kernels where bpf_prog_free_deferred() will call
bpf_free_used_maps() on the dangling func[i]->aux->used_maps pointer,
potentially causing kfree() to be called on already-freed or invalid memory.

>  	/* finally lock prog and jit images for all functions and
>  	 * populate kallsysm. Begin at the first subprogram, since
>  	 * bpf_prog_load will add the kallsyms for the main program.


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

Re: [PATCH bpf-next] bpf: cleanup aux->used_maps after jit

[Chris Mason]

On 11/24/25 10:30 AM, bot+bpf-ci@kernel.org wrote:
>> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
>> index 2e170be64..766695491 100644
>> --- a/kernel/bpf/verifier.c
>> +++ b/kernel/bpf/verifier.c
>> @@ -22266,6 +22266,15 @@ static int jit_subprogs(struct bpf_verifier_env *env)
>>  		cond_resched();
>>  	}
>>
>> +	/*
>> +	 * Cleanup func[i]->aux fields which aren't required
>> +	 * or can become invalid in future
>> +	 */
>> +	for (i = 0; i < env->subprog_cnt; i++) {
>> +		func[i]->aux->used_maps = NULL;
>> +		func[i]->aux->used_map_cnt = 0;
>> +	}
>> +
> 
> The patch correctly fixes the use-after-free issue. However, this isn't a
> bug, but should this have a Cc: stable@vger.kernel.org tag? The bug being
> fixed affects released kernels where bpf_prog_free_deferred() will call
> bpf_free_used_maps() on the dangling func[i]->aux->used_maps pointer,
> potentially causing kfree() to be called on already-freed or invalid memory.

I took a pull request for the review prompts this morning that adds
Fixes: suggestions and verification.  If Alexei or others here would
rather have this disabled for the BPF reviews, I'll make them default to
off.

-chris

Re: [PATCH bpf-next] bpf: cleanup aux->used_maps after jit

[Alexei Starovoitov]

On Mon, Nov 24, 2025 at 7:56 AM Chris Mason <clm@meta.com> wrote:
>
> On 11/24/25 10:30 AM, bot+bpf-ci@kernel.org wrote:
> >> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> >> index 2e170be64..766695491 100644
> >> --- a/kernel/bpf/verifier.c
> >> +++ b/kernel/bpf/verifier.c
> >> @@ -22266,6 +22266,15 @@ static int jit_subprogs(struct bpf_verifier_env *env)
> >>              cond_resched();
> >>      }
> >>
> >> +    /*
> >> +     * Cleanup func[i]->aux fields which aren't required
> >> +     * or can become invalid in future
> >> +     */
> >> +    for (i = 0; i < env->subprog_cnt; i++) {
> >> +            func[i]->aux->used_maps = NULL;
> >> +            func[i]->aux->used_map_cnt = 0;
> >> +    }
> >> +
> >
> > The patch correctly fixes the use-after-free issue. However, this isn't a
> > bug, but should this have a Cc: stable@vger.kernel.org tag? The bug being
> > fixed affects released kernels where bpf_prog_free_deferred() will call
> > bpf_free_used_maps() on the dangling func[i]->aux->used_maps pointer,
> > potentially causing kfree() to be called on already-freed or invalid memory.
>
> I took a pull request for the review prompts this morning that adds
> Fixes: suggestions and verification.  If Alexei or others here would
> rather have this disabled for the BPF reviews, I'll make them default to
> off.

Disable it pls.
In this case Fixes tag point to a commit in bpf-next,
so nothing to backport and, in general, we pretty much
never do "cc: stable" unless it's a critical fix.
I believe Greg and Sasha don't rely on cc: stable much.
Automation will figure things out based on Fixes tag.
cc: stable is an additional signal, but not mandatory,
and we prefer to avoid extra lines in the commit log when they
are not necessary.

Fixes tag is a different story. We definitely want it for fixes
and related changes even when they're not necessarily a fix.
But a generic AI comment "should there be a Fixes tag?" will not
be useful. If it can say "Should there be Fixes: sha ("bpf: ..")"
that would be nice.

Re: [PATCH bpf-next] bpf: cleanup aux->used_maps after jit

[Chris Mason]

On 11/24/25 11:55 AM, Alexei Starovoitov wrote:
> On Mon, Nov 24, 2025 at 7:56 AM Chris Mason <clm@meta.com> wrote:
>>
>> On 11/24/25 10:30 AM, bot+bpf-ci@kernel.org wrote:
>>>> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
>>>> index 2e170be64..766695491 100644
>>>> --- a/kernel/bpf/verifier.c
>>>> +++ b/kernel/bpf/verifier.c
>>>> @@ -22266,6 +22266,15 @@ static int jit_subprogs(struct bpf_verifier_env *env)
>>>>              cond_resched();
>>>>      }
>>>>
>>>> +    /*
>>>> +     * Cleanup func[i]->aux fields which aren't required
>>>> +     * or can become invalid in future
>>>> +     */
>>>> +    for (i = 0; i < env->subprog_cnt; i++) {
>>>> +            func[i]->aux->used_maps = NULL;
>>>> +            func[i]->aux->used_map_cnt = 0;
>>>> +    }
>>>> +
>>>
>>> The patch correctly fixes the use-after-free issue. However, this isn't a
>>> bug, but should this have a Cc: stable@vger.kernel.org tag? The bug being
>>> fixed affects released kernels where bpf_prog_free_deferred() will call
>>> bpf_free_used_maps() on the dangling func[i]->aux->used_maps pointer,
>>> potentially causing kfree() to be called on already-freed or invalid memory.
>>
>> I took a pull request for the review prompts this morning that adds
>> Fixes: suggestions and verification.  If Alexei or others here would
>> rather have this disabled for the BPF reviews, I'll make them default to
>> off.
> 
> Disable it pls.
> In this case Fixes tag point to a commit in bpf-next,
> so nothing to backport and, in general, we pretty much
> never do "cc: stable" unless it's a critical fix.
> I believe Greg and Sasha don't rely on cc: stable much.
> Automation will figure things out based on Fixes tag.
> cc: stable is an additional signal, but not mandatory,
> and we prefer to avoid extra lines in the commit log when they
> are not necessary.
> 
> Fixes tag is a different story. We definitely want it for fixes
> and related changes even when they're not necessarily a fix.
> But a generic AI comment "should there be a Fixes tag?" will not
> be useful. If it can say "Should there be Fixes: sha ("bpf: ..")"
> that would be nice.

Agreed, it shouldn't be hard for AI to suggest the Fixes: tag and the
commit being fixed.  I'll work on this.

I've disabled the Fixes: prompt by default, looks like one more review
got in before I turned it off.

-chris

Re: [PATCH bpf-next] bpf: cleanup aux->used_maps after jit

[patchwork-bot+netdevbpf]

Hello:

This patch was applied to bpf/bpf-next.git (master)
by Alexei Starovoitov <ast@kernel.org>:

On Mon, 24 Nov 2025 15:15:15 +0000 you wrote:
> In commit b4ce5923e780 ("bpf, x86: add new map type: instructions array")
> env->used_map was copied to func[i]->aux->used_maps before jitting.
> Clear these fields out after jitting such that pointer to freed memory
> (env->used_maps is freed later) are not kept in a live data structure.
> 
> The reason why the copies were initially added is explained in
> https://lore.kernel.org/bpf/20251105090410.1250500-1-a.s.protopopov@gmail.com
> 
> [...]

Here is the summary with links:
  - [bpf-next] bpf: cleanup aux->used_maps after jit
    https://git.kernel.org/bpf/bpf-next/c/fad804002ef3

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html