Re: [PATCH 1/4] powerpc64/bpf: Add support for instruction array

Re: [PATCH 1/4] powerpc64/bpf: Add support for instruction array

[bot+bpf-ci]

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

> diff --git a/arch/powerpc/net/bpf_jit_comp.c b/arch/powerpc/net/bpf_jit_comp.c
> index 52162e4a7f84..9c27740650b5 100644
> --- a/arch/powerpc/net/bpf_jit_comp.c
> +++ b/arch/powerpc/net/bpf_jit_comp.c
> @@ -318,6 +318,12 @@ struct bpf_prog *bpf_int_jit_compile(struct bpf_prog *fp)
>  		jit_data->hdr = hdr;
>  	}
>
> +	/*
> +	 * The bpf_prog_update_insn_ptrs function expects addrs to
> +	 * point to the first byte of the jitted instruction.
> +	 */
> +	bpf_prog_update_insn_ptrs(fp, addrs, fimage);

Is addrs still valid at this point?  Looking at the code above, in the
final-pass path where (!fp->is_func || extra_pass) is true, addrs has
already been freed:

bpf_int_jit_compile() {
    ...
    if (!fp->is_func || extra_pass) {
        ...
        bpf_prog_fill_jited_linfo(fp, addrs);
out_addrs:
        kfree(addrs);
        kfree(jit_data);
        fp->aux->jit_data = NULL;
    } else {
        jit_data->addrs = addrs;
        ...
        jit_data->hdr = hdr;
    }

    bpf_prog_update_insn_ptrs(fp, addrs, fimage);  /* <-- here */
    ...
}

The kfree(addrs) happens before this call.  The NULL check inside
bpf_prog_update_insn_ptrs() does not catch this because kfree()
does not set the pointer to NULL, so offsets[xlated_off] would read
from freed memory.

The x86 JIT avoids this by placing bpf_prog_update_insn_ptrs() inside
the if (image) block before the addrs are freed:

arch/x86/net/bpf_jit_comp.c:bpf_int_jit_compile() {
    ...
    bpf_prog_update_insn_ptrs(prog, addrs, image);
    ...
    /* addrs freed later */
    kvfree(addrs);
}

Should this call be moved before the out_addrs label, similar to
how x86 does it?

Additionally, for CONFIG_PPC64_ELF_ABI_V1 where FUNCTION_DESCR_SIZE
is 24, the addrs offsets are relative to code_base, not fimage:

    code_base = (u32 *)(image + FUNCTION_DESCR_SIZE);
    fcode_base = (u32 *)(fimage + FUNCTION_DESCR_SIZE);

Would the image argument need to be (fimage + FUNCTION_DESCR_SIZE)
rather than fimage to produce the correct absolute addresses in
insn_array->ips[]?

>  out:
>  	if (bpf_blinded)
>  		bpf_jit_prog_release_other(fp, fp == org_fp ? tmp_fp : org_fp);

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

AI-authorship-score: low
AI-authorship-explanation: Standard kernel patch conventions with specific upstream references; no AI-generation indicators.
issues-found: 2
issue-severity-score: high
issue-severity-explanation: Use-after-free on addrs array in normal JIT success path can corrupt insn_array jump targets, leading to crashes or undefined behavior.

[PATCH 0/4] powerpc64/bpf: Add support for instruction array and indirect jump

[adubey]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset=y, Size: 2081 bytes --]

From: Abhishek Dubey <adubey@linux.ibm.com>

The first 2 patch enable support for instruction array. Now, the 
instruction offset map maintain the mapping:
original inst -> xlated inst -> jited inst

The last two patch enable support for indirect jump. Any eligile
register can store jump target address for control flow to jump.

The features will be enabled on ppc32 in a separate series.

All selftest related to instruction array and indirect jump
are passing.

./test_progs-cpuv4 -n 20
#21/1    bpf_insn_array/one2one:OK
#21/2    bpf_insn_array/simple:OK
#21/3    bpf_insn_array/deletions:OK
#21/4    bpf_insn_array/deletions-with-functions:OK
#21/5    bpf_insn_array/blindness:OK
#21/6    bpf_insn_array/incorrect-index:OK
#21/7    bpf_insn_array/load-unfrozen-map:OK
#21/8    bpf_insn_array/no-map-reuse:OK
#21/9    bpf_insn_array/bpf-side-ops:OK
#21      bpf_insn_array:OK
Summary: 1/9 PASSED, 0 SKIPPED, 0 FAILED

./test_progs-cpuv4 -n 21 
#20/1    bpf_gotox/one-switch:OK
#20/2    bpf_gotox/one-switch-non-zero-sec-offset:OK
#20/3    bpf_gotox/two-switches:OK
#20/4    bpf_gotox/big-jump-table:OK
#20/5    bpf_gotox/static-global:OK
#20/6    bpf_gotox/nonstatic-global:OK
#20/7    bpf_gotox/other-sec:OK
#20/8    bpf_gotox/static-global-other-sec:OK
#20/9    bpf_gotox/nonstatic-global-other-sec:OK
#20/10   bpf_gotox/one-jump-two-maps:OK
#20/11   bpf_gotox/one-map-two-jumps:OK
#20      bpf_gotox:OK
Summary: 1/11 PASSED, 0 SKIPPED, 0 FAILED

Abhishek Dubey (4):
  powerpc64/bpf: Add support for instruction array
  selftest/bpf: Enable instruction array test for powerpc64
  powerpc64/bpf: Add support for indirect jump
  selftest/bpf: Enable gotox tests for powerpc64

 arch/powerpc/net/bpf_jit_comp.c                         | 6 ++++++
 arch/powerpc/net/bpf_jit_comp64.c                       | 8 ++++++++
 tools/testing/selftests/bpf/prog_tests/bpf_insn_array.c | 2 +-
 tools/testing/selftests/bpf/progs/verifier_gotox.c      | 2 +-
 4 files changed, 16 insertions(+), 2 deletions(-)

-- 
2.52.0

[PATCH 1/4] powerpc64/bpf: Add support for instruction array

[adubey]

From: Abhishek Dubey <adubey@linux.ibm.com>

On loading the BPF program, the verifier might adjust/omit some
instructions. The adjusted instruction offset is accounted in the
map containing original instruction -> xlated mapping. This patch
add ppc64 JIT support to additionally build the xlated->jitted
mapping for every instruction present in instruction array. This
change is needed to enable support for indirect jumps, added in a
subsequent patch.

Invoke bpf_prog_update_insn_ptrs() with offset pair of xlated_offset
and jited_offset. The offset mapping is already available, which is
being used for bpf_prog_fill_jited_linfo() and can be directly used
for bpf_prog_update_insn_ptrs() as well.

Additional details present at:

commit b4ce5923e780 ("bpf, x86: add new map type: instructions array")

Signed-off-by: Abhishek Dubey <adubey@linux.ibm.com>
---
 arch/powerpc/net/bpf_jit_comp.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/arch/powerpc/net/bpf_jit_comp.c b/arch/powerpc/net/bpf_jit_comp.c
index ebd21c75ce47..04f51f2c817c 100644
--- a/arch/powerpc/net/bpf_jit_comp.c
+++ b/arch/powerpc/net/bpf_jit_comp.c
@@ -379,6 +379,12 @@ struct bpf_prog *bpf_int_jit_compile(struct bpf_prog *fp)
 		jit_data->hdr = hdr;
 	}
 
+	/*
+	 * The bpf_prog_update_insn_ptrs function expects addrs to
+	 * point to the first byte of the jitted instruction.
+	 */
+	bpf_prog_update_insn_ptrs(fp, addrs, fimage);
+
 out:
 	if (bpf_blinded)
 		bpf_jit_prog_release_other(fp, fp == org_fp ? tmp_fp : org_fp);
-- 
2.52.0

[PATCH 2/4] selftest/bpf: Enable instruction array test for powerpc64

[adubey]

From: Abhishek Dubey <adubey@linux.ibm.com>

With instruction array now supported, enable corresponding bpf
selftest for powerpc.

Signed-off-by: Abhishek Dubey <adubey@linux.ibm.com>
---
 tools/testing/selftests/bpf/prog_tests/bpf_insn_array.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/testing/selftests/bpf/prog_tests/bpf_insn_array.c b/tools/testing/selftests/bpf/prog_tests/bpf_insn_array.c
index 93166c2da8e7..57ce6943fcfd 100644
--- a/tools/testing/selftests/bpf/prog_tests/bpf_insn_array.c
+++ b/tools/testing/selftests/bpf/prog_tests/bpf_insn_array.c
@@ -3,7 +3,7 @@
 #include <bpf/bpf.h>
 #include <test_progs.h>
 
-#if defined(__x86_64__) || defined(__aarch64__)
+#if defined(__x86_64__) || defined(__aarch64__) || defined(__powerpc64__)
 static int map_create(__u32 map_type, __u32 max_entries)
 {
 	const char *map_name = "insn_array";
-- 
2.52.0

[PATCH 3/4] powerpc64/bpf: Add support for indirect jump

[adubey]

From: Abhishek Dubey <adubey@linux.ibm.com>

Add support for a new instruction

	BPF_JMP|BPF_X|BPF_JA, SRC=0, DST=Rx, off=0, imm=0

which does an indirect jump to a location stored in Rx. The
register Rx should have type PTR_TO_INSN. This new type ensures
that the Rx register contains a value (or a range of values)
loaded from a correct jump table – map of type instruction array.

Support indirect jump to all registers in powerpc64 JIT using
the ctr register. Move Rx content to ctr register, then invoke
bctr instruction to branch to address stored in ctr register.
Skip save and restore of TOC as the jump is always within the
program context.

Signed-off-by: Abhishek Dubey <adubey@linux.ibm.com>
---
 arch/powerpc/net/bpf_jit_comp64.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/arch/powerpc/net/bpf_jit_comp64.c b/arch/powerpc/net/bpf_jit_comp64.c
index 33ea07660ebc..8771c9f23c98 100644
--- a/arch/powerpc/net/bpf_jit_comp64.c
+++ b/arch/powerpc/net/bpf_jit_comp64.c
@@ -1604,6 +1604,14 @@ int bpf_jit_build_body(struct bpf_prog *fp, u32 *image, u32 *fimage, struct code
 			addrs[++i] = ctx->idx * 4;
 			break;
 
+		/*
+		 * JUMP reg
+		 */
+		case BPF_JMP | BPF_JA | BPF_X:
+			EMIT(PPC_RAW_MTCTR(dst_reg));
+			EMIT(PPC_RAW_BCTR());
+			break;
+
 		/*
 		 * Return/Exit
 		 */
-- 
2.52.0

[PATCH 4/4] selftest/bpf: Enable gotox tests for powerpc64

[adubey]

From: Abhishek Dubey <adubey@linux.ibm.com>

With gotox instruction and jumptable now supported,
enable corresponding bpf selftest on powerpc.

Signed-off-by: Abhishek Dubey <adubey@linux.ibm.com>
---
 tools/testing/selftests/bpf/progs/verifier_gotox.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/testing/selftests/bpf/progs/verifier_gotox.c b/tools/testing/selftests/bpf/progs/verifier_gotox.c
index 607dad058ca1..c1e5b8529add 100644
--- a/tools/testing/selftests/bpf/progs/verifier_gotox.c
+++ b/tools/testing/selftests/bpf/progs/verifier_gotox.c
@@ -6,7 +6,7 @@
 #include "bpf_misc.h"
 #include "../../../include/linux/filter.h"
 
-#if defined(__TARGET_ARCH_x86) || defined(__TARGET_ARCH_arm64)
+#if defined(__TARGET_ARCH_x86) || defined(__TARGET_ARCH_arm64) || defined(__TARGET_ARCH_powerpc)
 
 #define DEFINE_SIMPLE_JUMP_TABLE_PROG(NAME, SRC_REG, OFF, IMM, OUTCOME)	\
 									\
-- 
2.52.0