Re: [PATCH bpf] bpf, bpftool: Fix incorrect disasm pc

[Yonghong Song <yonghong.song@linux.dev>]

On 10/30/24 2:47 AM, Leon Hwang wrote:
> From: Leon Hwang <leon.hwang@linux.dev>
>
> This patch addresses the bpftool issue "Wrong callq address displayed"[0].
>
> The issue stemmed from an incorrect program counter (PC) value used during
> disassembly with LLVM or libbfd. To calculate the correct address for
> relative calls, the PC argument must reflect the actual address in the
> kernel.
>
> [0] https://github.com/libbpf/bpftool/issues/109
>
> Fixes: e1947c750ffe ("bpftool: Refactor disassembler for JIT-ed programs")
> Signed-off-by: Leon Hwang <leon.hwang@linux.dev>

The following is the LLVMDisasmInstruction() description in
llvm-c/Disassembler.h:

/**
  * Disassemble a single instruction using the disassembler context specified in
  * the parameter DC.  The bytes of the instruction are specified in the
  * parameter Bytes, and contains at least BytesSize number of bytes.  The
  * instruction is at the address specified by the PC parameter.  If a valid
  * instruction can be disassembled, its string is returned indirectly in
  * OutString whose size is specified in the parameter OutStringSize.  This
  * function returns the number of bytes in the instruction or zero if there was
  * no valid instruction.
  */
size_t LLVMDisasmInstruction(LLVMDisasmContextRef DC, uint8_t *Bytes,
                              uint64_t BytesSize, uint64_t PC,
                              char *OutString, size_t OutStringSize);

In the above, it has
   The instruction is at the address specified by the PC parameter.

To call insn itself only encodes the difference between
helper address and 'bpf_prog + call_insn pc within prog'.
So to calculate proper final call address, the bpf_prog entry address
must be provided. So we need to supply 'prog_entry_addr + pc' instead
of 'pc'.

32bit should be okay since addr is within the first 4G.

Acked-by: Yonghong Song <yonghong.song@linux.dev>

> ---
>   tools/bpf/bpftool/jit_disasm.c | 6 +++---
>   1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/tools/bpf/bpftool/jit_disasm.c b/tools/bpf/bpftool/jit_disasm.c
> index 7b8d9ec89ebd3..fe8fabba4b05f 100644
> --- a/tools/bpf/bpftool/jit_disasm.c
> +++ b/tools/bpf/bpftool/jit_disasm.c
> @@ -114,8 +114,7 @@ disassemble_insn(disasm_ctx_t *ctx, unsigned char *image, ssize_t len, int pc)
>   	char buf[256];
>   	int count;
>   
> -	count = LLVMDisasmInstruction(*ctx, image + pc, len - pc, pc,
> -				      buf, sizeof(buf));
> +	count = LLVMDisasmInstruction(*ctx, image, len, pc, buf, sizeof(buf));
>   	if (json_output)
>   		printf_json(buf);
>   	else
> @@ -360,7 +359,8 @@ int disasm_print_insn(unsigned char *image, ssize_t len, int opcodes,
>   			printf("%4x:" DISASM_SPACER, pc);
>   		}
>   
> -		count = disassemble_insn(&ctx, image, len, pc);
> +		count = disassemble_insn(&ctx, image + pc, len - pc,
> +					 func_ksym + pc);
>   
>   		if (json_output) {
>   			/* Operand array, was started in fprintf_json. Before