From: Abhishek Dubey <redacted>
Ensure that the trampoline stubs JITed at the tail of the
epilogue do not expose the dummy trampoline address stored
in the last 8 bytes(64-bit) and last 4 bytes(32-bit)
to the disassembly flow. Prevent the disassembler from
ingesting this memory address, as it may occasionally decode
into a seemingly valid but incorrect instruction. Fix this
issue by truncating the last 8/4 bytes from JITed buffers
before supplying them for disassembly.
Signed-off-by: Abhishek Dubey <redacted>
Signed-off-by: Saket Kumar Bhaskar <redacted>
Tested-by: Yeswanth Krishna Tellakula <redacted>
Reviewed-by: Hari Bathini <hbathini@linux.ibm.com>
---
.../selftests/bpf/jit_disasm_helpers.c | 25 ++++++++++++++++++-
1 file changed, 24 insertions(+), 1 deletion(-)
diff --git a/tools/testing/selftests/bpf/jit_disasm_helpers.c b/tools/testing/selftests/bpf/jit_disasm_helpers.c
index 3558fe10e28c..b291d934dd1a 100644
--- a/tools/testing/selftests/bpf/jit_disasm_helpers.c
+++ b/tools/testing/selftests/bpf/jit_disasm_helpers.c
@@ -179,9 +179,11 @@ int get_jited_program_text(int fd, char *text, size_t text_sz)
struct bpf_prog_info info = {};
__u32 info_len = sizeof(info);
__u32 jited_funcs, len, pc;
+ __u32 trunc_len = 0, disasm_len;
__u32 *func_lens = NULL;
FILE *text_out = NULL;
uint8_t *image = NULL;
+ char *triple = NULL;
int i, err = 0;
if (!llvm_initialized) {@@ -225,9 +227,30 @@ int get_jited_program_text(int fd, char *text, size_t text_sz)
if (!ASSERT_OK(err, "bpf_prog_get_info_by_fd #2"))
goto out;
+ /*
+ * last 8 bytes contains dummy_trampoline address in JIT
+ * output on 64-bit and last 4 bytes on 32-bit powerpc,
+ * which can't disassemble to a valid instruction.
+ */
+ triple = LLVMGetDefaultTargetTriple();
+ if (triple) {
+ if (strstr(triple, "powerpc64") || strstr(triple, "ppc64"))
+ trunc_len = 8;
+ else if (strstr(triple, "powerpc") || strstr(triple, "ppc"))
+ trunc_len = 4;
+ LLVMDisposeMessage(triple);
+ }
+
for (pc = 0, i = 0; i < jited_funcs; ++i) {
fprintf(text_out, "func #%d:\n", i);
- disasm_one_func(text_out, image + pc, func_lens[i]);
+
+ /*
+ * Disabled JIT have zero func_lens, hence underflow
+ */
+ disasm_len = func_lens[i] > trunc_len ?
+ func_lens[i] - trunc_len : 0;
+ disasm_one_func(text_out, image + pc, disasm_len);
+
fprintf(text_out, "\n");
pc += func_lens[i];
}--
2.55.0
selftests/bpf: Fixing powerpc JIT disassembly failure
Ensure that the trampoline stubs JITed at the tail of the
epilogue do not expose the dummy trampoline address stored
in the last 8 bytes(64-bit) and last 4 bytes(32-bit)
to the disassembly flow. Prevent the disassembler from
ingesting this memory address, as it may occasionally decode
into a seemingly valid but incorrect instruction. Fix this
issue by truncating the last 8/4 bytes from JITed buffers
before supplying them for disassembly.
This isn't a bug, but could the subject use the imperative ('Fix ...'
instead of 'Fixing ...'), and could the in-code comment and the
changelog agree on why the trailing bytes are dropped - is the hazard
that they cannot be decoded, or that they can be decoded into a
plausible-looking wrong instruction?
quoted hunk
diff --git a/tools/testing/selftests/bpf/jit_disasm_helpers.c b/tools/testing/selftests/bpf/jit_disasm_helpers.c
index 3558fe10e28c..b291d934dd1a 100644
--- a/tools/testing/selftests/bpf/jit_disasm_helpers.c
+++ b/tools/testing/selftests/bpf/jit_disasm_helpers.c
@@ -179,9 +179,11 @@ int get_jited_program_text(int fd, char *text, size_t text_sz)
struct bpf_prog_info info = {};
__u32 info_len = sizeof(info);
__u32 jited_funcs, len, pc;
+ __u32 trunc_len = 0, disasm_len;
__u32 *func_lens = NULL;
FILE *text_out = NULL;
uint8_t *image = NULL;
+ char *triple = NULL;
int i, err = 0;
if (!llvm_initialized) {
[ ... ]
quoted hunk
@@ -225,9 +227,30 @@ int get_jited_program_text(int fd, char *text, size_t text_sz)
if (!ASSERT_OK(err, "bpf_prog_get_info_by_fd #2"))
goto out;
+ /*
+ * last 8 bytes contains dummy_trampoline address in JIT
+ * output on 64-bit and last 4 bytes on 32-bit powerpc,
+ * which can't disassemble to a valid instruction.
+ */
+ triple = LLVMGetDefaultTargetTriple();
+ if (triple) {
+ if (strstr(triple, "powerpc64") || strstr(triple, "ppc64"))
+ trunc_len = 8;
+ else if (strstr(triple, "powerpc") || strstr(triple, "ppc"))
+ trunc_len = 4;
+ LLVMDisposeMessage(triple);
+ }
This isn't a bug, but would #if defined(__powerpc64__) / #elif
defined(__powerpc__) work here, matching how get_current_arch() in
test_loader.c already detects the arch, and avoiding a second
LLVMGetDefaultTargetTriple()/LLVMDisposeMessage() pair alongside the
one in disasm_one_func()?
quoted hunk
+
for (pc = 0, i = 0; i < jited_funcs; ++i) {
fprintf(text_out, "func #%d:\n", i);
- disasm_one_func(text_out, image + pc, func_lens[i]);
+
+ /*
+ * Disabled JIT have zero func_lens, hence underflow
+ */
+ disasm_len = func_lens[i] > trunc_len ?
+ func_lens[i] - trunc_len : 0;
+ disasm_one_func(text_out, image + pc, disasm_len);
+
fprintf(text_out, "\n");
pc += func_lens[i];
}
This isn't a bug, but since 32-bit powerpc has no __arch_* tag and
get_current_arch() has no case for it yet, would it be clearer to add
the 4-byte arm together with the 32-bit enablement rather than ahead of
it?
---
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/32100929603