[PATCH bpf-next 1/3] libbpf: Drop redundant self-loop in emit_check_err

[PATCH bpf-next 1/3] libbpf: Drop redundant self-loop in emit_check_err

[Daniel Borkmann]

When the cleanup-label jump offset does not fit in s16, emit_check_err()
sets gen->error = -ERANGE and then emits a BPF_JMP_IMM(BPF_JA, 0, 0, -1)
self-loop.

The latter emit() is dead: gen->error is assigned on the preceding line,
and emit() then bails out early in realloc_insn_buf() the moment gen->error
is set, so the jump is never written into the instruction stream.

gen->error alone already marks the generation as failed. This is a follow-up
to 7dd62566e0d1 ("libbpf: fix off-by-one in emit_signature_match jump offset")
which removed the jump in emit_signature_match() but not in other locations.

Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
---
 tools/lib/bpf/gen_loader.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/tools/lib/bpf/gen_loader.c b/tools/lib/bpf/gen_loader.c
index 9478b8f78f26..7b95ced7bcba 100644
--- a/tools/lib/bpf/gen_loader.c
+++ b/tools/lib/bpf/gen_loader.c
@@ -293,7 +293,6 @@ static void emit_check_err(struct bpf_gen *gen)
 		emit(gen, BPF_JMP_IMM(BPF_JSLT, BPF_REG_7, 0, off));
 	} else {
 		gen->error = -ERANGE;
-		emit(gen, BPF_JMP_IMM(BPF_JA, 0, 0, -1));
 	}
 }
 
-- 
2.43.0

[PATCH bpf-next 2/3] libbpf: Skip hash computation when loader generation failed

[Daniel Borkmann]

bpf_gen__finish() calls compute_sha_update_offsets() gated only on
the gen_hash option, without first consulting gen->error. On a failed
generation this is buggy: a failed realloc_data_buf() sets gen->data_start
to NULL (leaving gen->data_cur dangling), so compute_sha_update_offsets()
runs libbpf_sha256() over a NULL buffer with a bogus length; a failed
realloc_insn_buf() likewise sets gen->insn_start to NULL and the hash
immediates get patched through that NULL base.

The computed program is discarded in either case, since the following
"if (!gen->error)" block does not publish opts->insns once an error is
set. Thus, skip the hash pass when generation has already failed.

Fixes: ea923080c145 ("libbpf: Embed and verify the metadata hash in the loader")
Reported-by: sashiko <sashiko@sashiko.dev>
Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
---
 tools/lib/bpf/gen_loader.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/tools/lib/bpf/gen_loader.c b/tools/lib/bpf/gen_loader.c
index 7b95ced7bcba..3a6e1d53f287 100644
--- a/tools/lib/bpf/gen_loader.c
+++ b/tools/lib/bpf/gen_loader.c
@@ -397,13 +397,12 @@ int bpf_gen__finish(struct bpf_gen *gen, int nr_progs, int nr_maps)
 			      blob_fd_array_off(gen, i));
 	emit(gen, BPF_MOV64_IMM(BPF_REG_0, 0));
 	emit(gen, BPF_EXIT_INSN());
-	if (OPTS_GET(gen->opts, gen_hash, false))
-		compute_sha_update_offsets(gen);
-
-	pr_debug("gen: finish %s\n", errstr(gen->error));
 	if (!gen->error) {
 		struct gen_loader_opts *opts = gen->opts;
 
+		if (OPTS_GET(opts, gen_hash, false))
+			compute_sha_update_offsets(gen);
+
 		opts->insns = gen->insn_start;
 		opts->insns_sz = gen->insn_cur - gen->insn_start;
 		opts->data = gen->data_start;
@@ -418,6 +417,7 @@ int bpf_gen__finish(struct bpf_gen *gen, int nr_progs, int nr_maps)
 				bpf_insn_bswap(insn++);
 		}
 	}
+	pr_debug("gen: finish %s\n", errstr(gen->error));
 	return gen->error;
 }
 
-- 
2.43.0

Re: [PATCH bpf-next 2/3] libbpf: Skip hash computation when loader generation failed

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [Critical] Unchecked `add_data()` failures lead to NULL pointer dereferences or heap buffer overflows in `bpf_gen__prog_load()` and `info_blob_bswap()` during endianness swapping.
--

commit 586ab83145acd4d3b6d7223bb61c05d593fe3860
Author: Daniel Borkmann <daniel@iogearbox.net>

libbpf: Skip hash computation when loader generation failed

This patch fixes a bug in bpf_gen__finish() where compute_sha_update_offsets()
was called on failed loader generation. It moves the hash computation inside
the !gen->error block to avoid running libbpf_sha256() over NULL buffers.

> diff --git a/tools/lib/bpf/gen_loader.c b/tools/lib/bpf/gen_loader.c
> index 7b95ced7bcbaf..3a6e1d53f287a 100644
> --- a/tools/lib/bpf/gen_loader.c
> +++ b/tools/lib/bpf/gen_loader.c
> @@ -397,13 +397,12 @@ int bpf_gen__finish(struct bpf_gen *gen, int nr_progs, int nr_maps)
>  			      blob_fd_array_off(gen, i));
>  	emit(gen, BPF_MOV64_IMM(BPF_REG_0, 0));
>  	emit(gen, BPF_EXIT_INSN());
> -	if (OPTS_GET(gen->opts, gen_hash, false))
> -		compute_sha_update_offsets(gen);
> -
> -	pr_debug("gen: finish %s\n", errstr(gen->error));
>  	if (!gen->error) {
>  		struct gen_loader_opts *opts = gen->opts;
>  
> +		if (OPTS_GET(opts, gen_hash, false))
> +			compute_sha_update_offsets(gen);
> +
>  		opts->insns = gen->insn_start;

[Severity: Critical]
This isn't a regression introduced by this patch, but while reviewing this
file I noticed a pre-existing issue with how generation errors are handled in
bpf_gen__prog_load() and info_blob_bswap().

If realloc_data_buf() fails inside add_data(), it frees gen->data_start,
sets it to NULL, sets gen->error, and add_data() returns 0.

However, bpf_gen__prog_load() doesn't check gen->error before swapping
endianness:

tools/lib/bpf/gen_loader.c:bpf_gen__prog_load() {
    ...
    insns_off = add_data(gen, insns, insn_cnt * sizeof(struct bpf_insn));
    ...
    if (gen->swapped_endian) {
        struct bpf_insn *insn = gen->data_start + insns_off;
        int i;

        for (i = 0; i < insn_cnt; i++, insn++)
            bpf_insn_bswap(insn);
    }
    ...
}

If add_data() failed, this evaluates to insn = NULL + 0. Can this cause a
NULL pointer dereference when accessing insn inside bpf_insn_bswap()?

Furthermore, if gen->error was previously set (e.g., from a failed
realloc_insn_buf) while gen->data_start is non-NULL, add_data() still returns
0. 

Since the loop evaluates insn = gen->data_start + 0, can this overwrite the
beginning of gen->data_start and write past the end of the buffer if it is
smaller than insn_cnt * sizeof(struct bpf_insn)?

Could we add a check for gen->error before proceeding with the endianness
swapping?

>  		opts->insns_sz = gen->insn_cur - gen->insn_start;
>  		opts->data = gen->data_start;
> @@ -418,6 +417,7 @@ int bpf_gen__finish(struct bpf_gen *gen, int nr_progs, int nr_maps)
>  				bpf_insn_bswap(insn++);
>  		}
>  	}
> +	pr_debug("gen: finish %s\n", errstr(gen->error));
>  	return gen->error;
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260529094119.307264-1-daniel@iogearbox.net?part=2

Re: [PATCH bpf-next 2/3] libbpf: Skip hash computation when loader generation failed

[Daniel Borkmann]

On 5/29/26 12:21 PM, sashiko-bot@kernel.org wrote:
[...]
> [Severity: Critical]
> This isn't a regression introduced by this patch, but while reviewing this
> file I noticed a pre-existing issue with how generation errors are handled in
> bpf_gen__prog_load() and info_blob_bswap().
> 
> If realloc_data_buf() fails inside add_data(), it frees gen->data_start,
> sets it to NULL, sets gen->error, and add_data() returns 0.
> 
> However, bpf_gen__prog_load() doesn't check gen->error before swapping
> endianness:
> 
> tools/lib/bpf/gen_loader.c:bpf_gen__prog_load() {
>      ...
>      insns_off = add_data(gen, insns, insn_cnt * sizeof(struct bpf_insn));
>      ...
>      if (gen->swapped_endian) {
>          struct bpf_insn *insn = gen->data_start + insns_off;
>          int i;
> 
>          for (i = 0; i < insn_cnt; i++, insn++)
>              bpf_insn_bswap(insn);
>      }
>      ...
> }
> 
> If add_data() failed, this evaluates to insn = NULL + 0. Can this cause a
> NULL pointer dereference when accessing insn inside bpf_insn_bswap()?
> 
> Furthermore, if gen->error was previously set (e.g., from a failed
> realloc_insn_buf) while gen->data_start is non-NULL, add_data() still returns
> 0.
> 
> Since the loop evaluates insn = gen->data_start + 0, can this overwrite the
> beginning of gen->data_start and write past the end of the buffer if it is
> smaller than insn_cnt * sizeof(struct bpf_insn)?
> 
> Could we add a check for gen->error before proceeding with the endianness
> swapping?
Yeap, for the endianess we would need something like the below ... will cook
a patch for it to address :

diff --git a/tools/lib/bpf/gen_loader.c b/tools/lib/bpf/gen_loader.c
index 19f9c05f8388..501508e5affa 100644
--- a/tools/lib/bpf/gen_loader.c
+++ b/tools/lib/bpf/gen_loader.c
@@ -1071,7 +1071,7 @@ void bpf_gen__prog_load(struct bpf_gen *gen,
                  prog_idx, prog_type, insns_off, insn_cnt, license_off);
  
         /* convert blob insns to target endianness */
-       if (gen->swapped_endian) {
+       if (gen->swapped_endian && !gen->error) {
                 struct bpf_insn *insn = gen->data_start + insns_off;
                 int i;
  
@@ -1109,7 +1109,7 @@ void bpf_gen__prog_load(struct bpf_gen *gen,
                  sizeof(struct bpf_core_relo));
  
         /* convert all info blobs to target endianness */
-       if (gen->swapped_endian)
+       if (gen->swapped_endian && !gen->error)
                 info_blob_bswap(gen, func_info, line_info, core_relos, load_attr);
  
         libbpf_strlcpy(attr.prog_name, prog_name, sizeof(attr.prog_name));

[PATCH bpf-next 3/3] libbpf: Also reset {insn,data}_cur on realloc failure

[Daniel Borkmann]

realloc_insn_buf() as well as realloc_data_buf() free and NULL
gen->insn_start / gen->data_start on -ENOMEM but leave gen->insn_cur /
gen->data_cur pointing into the old, freed buffer. Just reset the
cursors to NULL alongside the base pointers so the freed state is
coherent.

Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
---
 tools/lib/bpf/gen_loader.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/tools/lib/bpf/gen_loader.c b/tools/lib/bpf/gen_loader.c
index 3a6e1d53f287..492360ca07ea 100644
--- a/tools/lib/bpf/gen_loader.c
+++ b/tools/lib/bpf/gen_loader.c
@@ -63,6 +63,7 @@ static int realloc_insn_buf(struct bpf_gen *gen, __u32 size)
 		gen->error = -ENOMEM;
 		free(gen->insn_start);
 		gen->insn_start = NULL;
+		gen->insn_cur = NULL;
 		return -ENOMEM;
 	}
 	gen->insn_start = insn_start;
@@ -86,6 +87,7 @@ static int realloc_data_buf(struct bpf_gen *gen, __u32 size)
 		gen->error = -ENOMEM;
 		free(gen->data_start);
 		gen->data_start = NULL;
+		gen->data_cur = NULL;
 		return -ENOMEM;
 	}
 	gen->data_start = data_start;
-- 
2.43.0

Re: [PATCH bpf-next 3/3] libbpf: Also reset {insn,data}_cur on realloc failure

[bot+bpf-ci]

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

>     libbpf: Also reset {insn,data}_cur on realloc failure
>
>     realloc_insn_buf() as well as realloc_data_buf() free and NULL
>     gen->insn_start / gen->data_start on -ENOMEM but leave gen->insn_cur /
>     gen->data_cur pointing into the old, freed buffer. Just reset the
>     cursors to NULL alongside the base pointers so the freed state is
>     coherent.
>
>     Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>

This looks like a fix for a real bug where gen->insn_cur and
gen->data_cur are left pointing into a freed buffer on -ENOMEM.  Should
it carry a Fixes: tag pointing at the commit that introduced the
free()+NULL handling in realloc_insn_buf() and realloc_data_buf()?

The exact SHA could not be determined here, so it would need to be
looked up from a full bpf-next/torvalds tree.


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

Re: [PATCH bpf-next 3/3] libbpf: Also reset {insn,data}_cur on realloc failure

[Daniel Borkmann]

On 5/29/26 12:20 PM, bot+bpf-ci@kernel.org wrote:
>>      libbpf: Also reset {insn,data}_cur on realloc failure
>>
>>      realloc_insn_buf() as well as realloc_data_buf() free and NULL
>>      gen->insn_start / gen->data_start on -ENOMEM but leave gen->insn_cur /
>>      gen->data_cur pointing into the old, freed buffer. Just reset the
>>      cursors to NULL alongside the base pointers so the freed state is
>>      coherent.
>>
>>      Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
> 
> This looks like a fix for a real bug where gen->insn_cur and
> gen->data_cur are left pointing into a freed buffer on -ENOMEM.  Should
> it carry a Fixes: tag pointing at the commit that introduced the
> free()+NULL handling in realloc_insn_buf() and realloc_data_buf()?
> 
> The exact SHA could not be determined here, so it would need to be
> looked up from a full bpf-next/torvalds tree.
(I've added this given patch 2 in this series where stale data was used.)

Re: [PATCH bpf-next 1/3] libbpf: Drop redundant self-loop in emit_check_err

[patchwork-bot+netdevbpf]

Hello:

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

On Fri, 29 May 2026 11:41:16 +0200 you wrote:
> When the cleanup-label jump offset does not fit in s16, emit_check_err()
> sets gen->error = -ERANGE and then emits a BPF_JMP_IMM(BPF_JA, 0, 0, -1)
> self-loop.
> 
> The latter emit() is dead: gen->error is assigned on the preceding line,
> and emit() then bails out early in realloc_insn_buf() the moment gen->error
> is set, so the jump is never written into the instruction stream.
> 
> [...]

Here is the summary with links:
  - [bpf-next,1/3] libbpf: Drop redundant self-loop in emit_check_err
    https://git.kernel.org/bpf/bpf-next/c/e2c88266147f
  - [bpf-next,2/3] libbpf: Skip hash computation when loader generation failed
    https://git.kernel.org/bpf/bpf-next/c/3c5e2f1a8584
  - [bpf-next,3/3] libbpf: Also reset {insn,data}_cur on realloc failure
    https://git.kernel.org/bpf/bpf-next/c/d2f7bd066ed4

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