* Problems with pointer offsets on ARM32
@ 2020-09-11 14:56 Luka Oreskovic
2020-09-11 18:13 ` Andrii Nakryiko
0 siblings, 1 reply; 8+ messages in thread
From: Luka Oreskovic @ 2020-09-11 14:56 UTC (permalink / raw)
To: bpf; +Cc: Luka Perkov, Juraj Vijtiuk
Greetings everyone,
I have been testing various BPF programs on the ARM32 architecture and
have encountered a strange error.
When trying to run a simple program that prints out the arguments of
the open syscall,
I found some strange behaviour with the pointer offsets when accessing
the arguments:
The output of llvm-objdump differed from the verifier error dump log.
Notice the differences in lines 0 and 1. Why is the bytecode being
altered at runtime?
I attached the program, the llvm-objdump result and the verifier dump below.
Best wishes,
Luka Oreskovic
BPF program
--------------------------------------------
#include "vmlinux.h"
#include <bpf/bpf_helpers.h>
SEC("tracepoint/syscalls/sys_enter_open")
int tracepoint__syscalls__sys_enter_open(struct trace_event_raw_sys_enter* ctx)
{
const char *arg1 = (const char *)ctx->args[0];
int arg2 = (int)ctx->args[1];
bpf_printk("Open arg 1: %s\n", arg1);
bpf_printk("Open arg 2: %d\n", arg2);
return 0;
}
char LICENSE[] SEC("license") = "GPL";
llvm-objdump of program
--------------------------------------------
Disassembly of section tracepoint/syscalls/sys_enter_open:
0000000000000000 tracepoint__syscalls__sys_enter_open:
; int arg2 = (int)ctx->args[1];
0: 79 16 18 00 00 00 00 00 r6 = *(u64 *)(r1 + 24)
; const char *arg1 = (const char *)ctx->args[0];
1: 79 13 10 00 00 00 00 00 r3 = *(u64 *)(r1 + 16)
2: 18 01 00 00 20 31 3a 20 00 00 00 00 25 73 0a 00 r1 =
2941353058775328 ll
; bpf_printk("Open arg 1: %s\n", arg1);
4: 7b 1a f8 ff 00 00 00 00 *(u64 *)(r10 - 8) = r1
5: 18 07 00 00 4f 70 65 6e 00 00 00 00 20 61 72 67 r7 =
7454127125170581583 ll
7: 7b 7a f0 ff 00 00 00 00 *(u64 *)(r10 - 16) = r7
8: bf a1 00 00 00 00 00 00 r1 = r10
9: 07 01 00 00 f0 ff ff ff r1 += -16
10: b7 02 00 00 10 00 00 00 r2 = 16
11: 85 00 00 00 06 00 00 00 call 6
12: 18 01 00 00 20 32 3a 20 00 00 00 00 25 64 0a 00 r1 =
2924860384358944 ll
; bpf_printk("Open arg 2: %d\n", arg2);
14: 7b 1a f8 ff 00 00 00 00 *(u64 *)(r10 - 8) = r1
15: 7b 7a f0 ff 00 00 00 00 *(u64 *)(r10 - 16) = r7
16: bf a1 00 00 00 00 00 00 r1 = r10
17: 07 01 00 00 f0 ff ff ff r1 += -16
18: b7 02 00 00 10 00 00 00 r2 = 16
19: bf 63 00 00 00 00 00 00 r3 = r6
20: 85 00 00 00 06 00 00 00 call 6
; return 0;
21: b7 00 00 00 00 00 00 00 r0 = 0
22: 95 00 00 00 00 00 00 00 exit
verifier output when running program
--------------------------------------------
libbpf: -- BEGIN DUMP LOG ---
libbpf:
Unrecognized arg#0 type PTR
; int arg2 = (int)ctx->args[1];
0: (79) r6 = *(u64 *)(r1 +16)
; const char *arg1 = (const char *)ctx->args[0];
1: (79) r3 = *(u64 *)(r1 +12)
invalid bpf_context access off=12 size=8
processed 2 insns (limit 1000000) max_states_per_insn 0 total_states 0
peak_states 0 mark_read 0
libbpf: -- END LOG --
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: Problems with pointer offsets on ARM32
2020-09-11 14:56 Problems with pointer offsets on ARM32 Luka Oreskovic
@ 2020-09-11 18:13 ` Andrii Nakryiko
2020-09-14 7:55 ` Luka Oreskovic
0 siblings, 1 reply; 8+ messages in thread
From: Andrii Nakryiko @ 2020-09-11 18:13 UTC (permalink / raw)
To: Luka Oreskovic; +Cc: bpf, Luka Perkov, Juraj Vijtiuk
On Fri, Sep 11, 2020 at 9:45 AM Luka Oreskovic
<luka.oreskovic@sartura.hr> wrote:
>
> Greetings everyone,
>
> I have been testing various BPF programs on the ARM32 architecture and
> have encountered a strange error.
>
> When trying to run a simple program that prints out the arguments of
> the open syscall,
> I found some strange behaviour with the pointer offsets when accessing
> the arguments:
> The output of llvm-objdump differed from the verifier error dump log.
> Notice the differences in lines 0 and 1. Why is the bytecode being
> altered at runtime?
>
> I attached the program, the llvm-objdump result and the verifier dump below.
>
> Best wishes,
> Luka Oreskovic
>
> BPF program
> --------------------------------------------
> #include "vmlinux.h"
> #include <bpf/bpf_helpers.h>
>
> SEC("tracepoint/syscalls/sys_enter_open")
> int tracepoint__syscalls__sys_enter_open(struct trace_event_raw_sys_enter* ctx)
> {
> const char *arg1 = (const char *)ctx->args[0];
> int arg2 = (int)ctx->args[1];
>
> bpf_printk("Open arg 1: %s\n", arg1);
> bpf_printk("Open arg 2: %d\n", arg2);
>
> return 0;
> }
>
> char LICENSE[] SEC("license") = "GPL";
>
>
> llvm-objdump of program
> --------------------------------------------
> Disassembly of section tracepoint/syscalls/sys_enter_open:
>
> 0000000000000000 tracepoint__syscalls__sys_enter_open:
> ; int arg2 = (int)ctx->args[1];
> 0: 79 16 18 00 00 00 00 00 r6 = *(u64 *)(r1 + 24)
> ; const char *arg1 = (const char *)ctx->args[0];
> 1: 79 13 10 00 00 00 00 00 r3 = *(u64 *)(r1 + 16)
> 2: 18 01 00 00 20 31 3a 20 00 00 00 00 25 73 0a 00 r1 =
> 2941353058775328 ll
> ; bpf_printk("Open arg 1: %s\n", arg1);
> 4: 7b 1a f8 ff 00 00 00 00 *(u64 *)(r10 - 8) = r1
> 5: 18 07 00 00 4f 70 65 6e 00 00 00 00 20 61 72 67 r7 =
> 7454127125170581583 ll
> 7: 7b 7a f0 ff 00 00 00 00 *(u64 *)(r10 - 16) = r7
> 8: bf a1 00 00 00 00 00 00 r1 = r10
> 9: 07 01 00 00 f0 ff ff ff r1 += -16
> 10: b7 02 00 00 10 00 00 00 r2 = 16
> 11: 85 00 00 00 06 00 00 00 call 6
> 12: 18 01 00 00 20 32 3a 20 00 00 00 00 25 64 0a 00 r1 =
> 2924860384358944 ll
> ; bpf_printk("Open arg 2: %d\n", arg2);
> 14: 7b 1a f8 ff 00 00 00 00 *(u64 *)(r10 - 8) = r1
> 15: 7b 7a f0 ff 00 00 00 00 *(u64 *)(r10 - 16) = r7
> 16: bf a1 00 00 00 00 00 00 r1 = r10
> 17: 07 01 00 00 f0 ff ff ff r1 += -16
> 18: b7 02 00 00 10 00 00 00 r2 = 16
> 19: bf 63 00 00 00 00 00 00 r3 = r6
> 20: 85 00 00 00 06 00 00 00 call 6
> ; return 0;
> 21: b7 00 00 00 00 00 00 00 r0 = 0
> 22: 95 00 00 00 00 00 00 00 exit
>
>
> verifier output when running program
> --------------------------------------------
> libbpf: -- BEGIN DUMP LOG ---
> libbpf:
> Unrecognized arg#0 type PTR
> ; int arg2 = (int)ctx->args[1];
> 0: (79) r6 = *(u64 *)(r1 +16)
> ; const char *arg1 = (const char *)ctx->args[0];
> 1: (79) r3 = *(u64 *)(r1 +12)
> invalid bpf_context access off=12 size=8
> processed 2 insns (limit 1000000) max_states_per_insn 0 total_states 0
> peak_states 0 mark_read 0
>
> libbpf: -- END LOG --
One suspect would be libbpf's CO-RE relocations. Can you send full
debug libbpf logs, it will have a full log of what libbpf adjusted.
Please also include the definition of struct trace_event_raw_sys_enter
from your vmlinux.h, as well as commit that your kernel was built from
(to check the original definition).
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: Problems with pointer offsets on ARM32
2020-09-11 18:13 ` Andrii Nakryiko
@ 2020-09-14 7:55 ` Luka Oreskovic
2020-09-14 17:49 ` Andrii Nakryiko
0 siblings, 1 reply; 8+ messages in thread
From: Luka Oreskovic @ 2020-09-14 7:55 UTC (permalink / raw)
To: Andrii Nakryiko; +Cc: bpf, Luka Perkov, Juraj Vijtiuk
On Fri, Sep 11, 2020 at 8:14 PM Andrii Nakryiko
<andrii.nakryiko@gmail.com> wrote:
>
> On Fri, Sep 11, 2020 at 9:45 AM Luka Oreskovic
> <luka.oreskovic@sartura.hr> wrote:
> >
> > Greetings everyone,
> >
> > I have been testing various BPF programs on the ARM32 architecture and
> > have encountered a strange error.
> >
> > When trying to run a simple program that prints out the arguments of
> > the open syscall,
> > I found some strange behaviour with the pointer offsets when accessing
> > the arguments:
> > The output of llvm-objdump differed from the verifier error dump log.
> > Notice the differences in lines 0 and 1. Why is the bytecode being
> > altered at runtime?
> >
> > I attached the program, the llvm-objdump result and the verifier dump below.
> >
> > Best wishes,
> > Luka Oreskovic
> >
> > BPF program
> > --------------------------------------------
> > #include "vmlinux.h"
> > #include <bpf/bpf_helpers.h>
> >
> > SEC("tracepoint/syscalls/sys_enter_open")
> > int tracepoint__syscalls__sys_enter_open(struct trace_event_raw_sys_enter* ctx)
> > {
> > const char *arg1 = (const char *)ctx->args[0];
> > int arg2 = (int)ctx->args[1];
> >
> > bpf_printk("Open arg 1: %s\n", arg1);
> > bpf_printk("Open arg 2: %d\n", arg2);
> >
> > return 0;
> > }
> >
> > char LICENSE[] SEC("license") = "GPL";
> >
> >
> > llvm-objdump of program
> > --------------------------------------------
> > Disassembly of section tracepoint/syscalls/sys_enter_open:
> >
> > 0000000000000000 tracepoint__syscalls__sys_enter_open:
> > ; int arg2 = (int)ctx->args[1];
> > 0: 79 16 18 00 00 00 00 00 r6 = *(u64 *)(r1 + 24)
> > ; const char *arg1 = (const char *)ctx->args[0];
> > 1: 79 13 10 00 00 00 00 00 r3 = *(u64 *)(r1 + 16)
> > 2: 18 01 00 00 20 31 3a 20 00 00 00 00 25 73 0a 00 r1 =
> > 2941353058775328 ll
> > ; bpf_printk("Open arg 1: %s\n", arg1);
> > 4: 7b 1a f8 ff 00 00 00 00 *(u64 *)(r10 - 8) = r1
> > 5: 18 07 00 00 4f 70 65 6e 00 00 00 00 20 61 72 67 r7 =
> > 7454127125170581583 ll
> > 7: 7b 7a f0 ff 00 00 00 00 *(u64 *)(r10 - 16) = r7
> > 8: bf a1 00 00 00 00 00 00 r1 = r10
> > 9: 07 01 00 00 f0 ff ff ff r1 += -16
> > 10: b7 02 00 00 10 00 00 00 r2 = 16
> > 11: 85 00 00 00 06 00 00 00 call 6
> > 12: 18 01 00 00 20 32 3a 20 00 00 00 00 25 64 0a 00 r1 =
> > 2924860384358944 ll
> > ; bpf_printk("Open arg 2: %d\n", arg2);
> > 14: 7b 1a f8 ff 00 00 00 00 *(u64 *)(r10 - 8) = r1
> > 15: 7b 7a f0 ff 00 00 00 00 *(u64 *)(r10 - 16) = r7
> > 16: bf a1 00 00 00 00 00 00 r1 = r10
> > 17: 07 01 00 00 f0 ff ff ff r1 += -16
> > 18: b7 02 00 00 10 00 00 00 r2 = 16
> > 19: bf 63 00 00 00 00 00 00 r3 = r6
> > 20: 85 00 00 00 06 00 00 00 call 6
> > ; return 0;
> > 21: b7 00 00 00 00 00 00 00 r0 = 0
> > 22: 95 00 00 00 00 00 00 00 exit
> >
> >
> > verifier output when running program
> > --------------------------------------------
> > libbpf: -- BEGIN DUMP LOG ---
> > libbpf:
> > Unrecognized arg#0 type PTR
> > ; int arg2 = (int)ctx->args[1];
> > 0: (79) r6 = *(u64 *)(r1 +16)
> > ; const char *arg1 = (const char *)ctx->args[0];
> > 1: (79) r3 = *(u64 *)(r1 +12)
> > invalid bpf_context access off=12 size=8
> > processed 2 insns (limit 1000000) max_states_per_insn 0 total_states 0
> > peak_states 0 mark_read 0
> >
> > libbpf: -- END LOG --
>
>
> One suspect would be libbpf's CO-RE relocations. Can you send full
> debug libbpf logs, it will have a full log of what libbpf adjusted.
> Please also include the definition of struct trace_event_raw_sys_enter
> from your vmlinux.h, as well as commit that your kernel was built from
> (to check the original definition).
Here is the data you requested. I can see the reallocations done by BPF CO-RE,
but I don't understand why they would have to be done in the first place
since I am using the vmlinux.h that has been generated using the
devices vmlinux.
Even if it made sense to change the pointer offsets, they shouldn't
break the program.
Kernel commit
--------------------------------------------
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/?h=v5.9-rc3
Struct definition from vmlinux.h
--------------------------------------------
struct trace_event_raw_sys_enter {
struct trace_entry ent;
long int id;
long unsigned int args[6];
char __data[0];
};
Libbpf debug output
--------------------------------------------
ibbpf: loading object 'hello_bpf' from buffer
libbpf: elf: section(3) tracepoint/syscalls/sys_enter_open, size 184,
link 0, flags 6, type=1
libbpf: elf: found program 'tracepoint/syscalls/sys_enter_open'
libbpf: elf: section(4) .rodata.str1.1, size 32, link 0, flags 32, type=1
libbpf: elf: skipping unrecognized data section(4) .rodata.str1.1
libbpf: elf: section(5) license, size 4, link 0, flags 3, type=1
libbpf: license of hello_bpf is GPL
libbpf: elf: section(12) .BTF, size 986, link 0, flags 0, type=1
libbpf: elf: section(14) .BTF.ext, size 252, link 0, flags 0, type=1
libbpf: elf: section(21) .symtab, size 936, link 1, flags 0, type=2
libbpf: looking for externs among 39 symbols...
libbpf: collected 0 externs total
libbpf: loading kernel BTF '/sys/kernel/btf/vmlinux': 0
libbpf: sec 'tracepoint/syscalls/sys_enter_open': found 2 CO-RE relocations
libbpf: prog 'tracepoint/syscalls/sys_enter_open': relo #0: kind
<byte_off> (0), spec is [2] struct trace_event_raw_sys_ent)
libbpf: CO-RE relocating [2] struct trace_event_raw_sys_enter: found
target candidate [4639] struct trace_event_raw_sys_entr
libbpf: prog 'tracepoint/syscalls/sys_enter_open': relo #0: matching
candidate #0 [4639] struct trace_event_raw_sys_enter.a)
libbpf: prog 'tracepoint/syscalls/sys_enter_open': relo #0: patched
insn #0 (LDX/ST/STX) off 24 -> 16
libbpf: prog 'tracepoint/syscalls/sys_enter_open': relo #1: kind
<byte_off> (0), spec is [2] struct trace_event_raw_sys_ent)
libbpf: prog 'tracepoint/syscalls/sys_enter_open': relo #1: matching
candidate #0 [4639] struct trace_event_raw_sys_enter.a)
libbpf: prog 'tracepoint/syscalls/sys_enter_open': relo #1: patched
insn #1 (LDX/ST/STX) off 16 -> 12
libbpf: load bpf program failed: Permission denied
libbpf: -- BEGIN DUMP LOG ---
libbpf:
Unrecognized arg#0 type PTR
; int arg2 = (int)ctx->args[1];
0: (79) r6 = *(u64 *)(r1 +16)
; const char *arg1 = (const char *)ctx->args[0];
1: (79) r3 = *(u64 *)(r1 +12)
invalid bpf_context access off=12 size=8
processed 2 insns (limit 1000000) max_states_per_insn 0 total_states 0
peak_states 0 mark_read 0
libbpf: -- END LOG --
libbpf: failed to load program 'tracepoint/syscalls/sys_enter_open'
libbpf: failed to load object 'hello_bpf'
libbpf: failed to load BPF skeleton 'hello_bpf': -4007
failed to load BPF object -4007
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: Problems with pointer offsets on ARM32
2020-09-14 7:55 ` Luka Oreskovic
@ 2020-09-14 17:49 ` Andrii Nakryiko
2020-09-15 7:26 ` Luka Oreskovic
0 siblings, 1 reply; 8+ messages in thread
From: Andrii Nakryiko @ 2020-09-14 17:49 UTC (permalink / raw)
To: Luka Oreskovic; +Cc: bpf, Luka Perkov, Juraj Vijtiuk
On Mon, Sep 14, 2020 at 12:55 AM Luka Oreskovic
<luka.oreskovic@sartura.hr> wrote:
>
> On Fri, Sep 11, 2020 at 8:14 PM Andrii Nakryiko
> <andrii.nakryiko@gmail.com> wrote:
> >
> > On Fri, Sep 11, 2020 at 9:45 AM Luka Oreskovic
> > <luka.oreskovic@sartura.hr> wrote:
> > >
> > > Greetings everyone,
> > >
> > > I have been testing various BPF programs on the ARM32 architecture and
> > > have encountered a strange error.
> > >
> > > When trying to run a simple program that prints out the arguments of
> > > the open syscall,
> > > I found some strange behaviour with the pointer offsets when accessing
> > > the arguments:
> > > The output of llvm-objdump differed from the verifier error dump log.
> > > Notice the differences in lines 0 and 1. Why is the bytecode being
> > > altered at runtime?
> > >
> > > I attached the program, the llvm-objdump result and the verifier dump below.
> > >
> > > Best wishes,
> > > Luka Oreskovic
> > >
> > > BPF program
> > > --------------------------------------------
> > > #include "vmlinux.h"
> > > #include <bpf/bpf_helpers.h>
> > >
> > > SEC("tracepoint/syscalls/sys_enter_open")
> > > int tracepoint__syscalls__sys_enter_open(struct trace_event_raw_sys_enter* ctx)
> > > {
> > > const char *arg1 = (const char *)ctx->args[0];
> > > int arg2 = (int)ctx->args[1];
> > >
> > > bpf_printk("Open arg 1: %s\n", arg1);
> > > bpf_printk("Open arg 2: %d\n", arg2);
> > >
> > > return 0;
> > > }
> > >
> > > char LICENSE[] SEC("license") = "GPL";
> > >
> > >
> > > llvm-objdump of program
> > > --------------------------------------------
> > > Disassembly of section tracepoint/syscalls/sys_enter_open:
> > >
> > > 0000000000000000 tracepoint__syscalls__sys_enter_open:
> > > ; int arg2 = (int)ctx->args[1];
> > > 0: 79 16 18 00 00 00 00 00 r6 = *(u64 *)(r1 + 24)
> > > ; const char *arg1 = (const char *)ctx->args[0];
> > > 1: 79 13 10 00 00 00 00 00 r3 = *(u64 *)(r1 + 16)
> > > 2: 18 01 00 00 20 31 3a 20 00 00 00 00 25 73 0a 00 r1 =
> > > 2941353058775328 ll
> > > ; bpf_printk("Open arg 1: %s\n", arg1);
> > > 4: 7b 1a f8 ff 00 00 00 00 *(u64 *)(r10 - 8) = r1
> > > 5: 18 07 00 00 4f 70 65 6e 00 00 00 00 20 61 72 67 r7 =
> > > 7454127125170581583 ll
> > > 7: 7b 7a f0 ff 00 00 00 00 *(u64 *)(r10 - 16) = r7
> > > 8: bf a1 00 00 00 00 00 00 r1 = r10
> > > 9: 07 01 00 00 f0 ff ff ff r1 += -16
> > > 10: b7 02 00 00 10 00 00 00 r2 = 16
> > > 11: 85 00 00 00 06 00 00 00 call 6
> > > 12: 18 01 00 00 20 32 3a 20 00 00 00 00 25 64 0a 00 r1 =
> > > 2924860384358944 ll
> > > ; bpf_printk("Open arg 2: %d\n", arg2);
> > > 14: 7b 1a f8 ff 00 00 00 00 *(u64 *)(r10 - 8) = r1
> > > 15: 7b 7a f0 ff 00 00 00 00 *(u64 *)(r10 - 16) = r7
> > > 16: bf a1 00 00 00 00 00 00 r1 = r10
> > > 17: 07 01 00 00 f0 ff ff ff r1 += -16
> > > 18: b7 02 00 00 10 00 00 00 r2 = 16
> > > 19: bf 63 00 00 00 00 00 00 r3 = r6
> > > 20: 85 00 00 00 06 00 00 00 call 6
> > > ; return 0;
> > > 21: b7 00 00 00 00 00 00 00 r0 = 0
> > > 22: 95 00 00 00 00 00 00 00 exit
> > >
> > >
> > > verifier output when running program
> > > --------------------------------------------
> > > libbpf: -- BEGIN DUMP LOG ---
> > > libbpf:
> > > Unrecognized arg#0 type PTR
> > > ; int arg2 = (int)ctx->args[1];
> > > 0: (79) r6 = *(u64 *)(r1 +16)
> > > ; const char *arg1 = (const char *)ctx->args[0];
> > > 1: (79) r3 = *(u64 *)(r1 +12)
> > > invalid bpf_context access off=12 size=8
> > > processed 2 insns (limit 1000000) max_states_per_insn 0 total_states 0
> > > peak_states 0 mark_read 0
> > >
> > > libbpf: -- END LOG --
> >
> >
> > One suspect would be libbpf's CO-RE relocations. Can you send full
> > debug libbpf logs, it will have a full log of what libbpf adjusted.
> > Please also include the definition of struct trace_event_raw_sys_enter
> > from your vmlinux.h, as well as commit that your kernel was built from
> > (to check the original definition).
>
>
> Here is the data you requested. I can see the reallocations done by BPF CO-RE,
> but I don't understand why they would have to be done in the first place
> since I am using the vmlinux.h that has been generated using the
> devices vmlinux.
> Even if it made sense to change the pointer offsets, they shouldn't
> break the program.
Ok, there are few things at work here. I'll try to explain it and
let's see how we can fix this. This is all due to 32-bit architecture,
of course.
1. BPF "architecture" is strictly 64-bit one. This means that from the
BPF program point of view pointers are 64-bit values, long is 64-bit
as well.
2. look at struct trace_event_raw_sys_enter below: id and each element
of args[] array are longs. What this means is the following:
- from BPF side of view those are assumed to be 64-bit integers
- but in reality in ARM32 kernel and in memory they are 32-bit integers.
So the memory view of struct trace_event_raw_sys_enter differs quite a
lot, depending in which context that struct definition is used. You
can see that from the compiled BPF assembly output above and compare
to the actual memory layout, taking into account that long is 4 bytes.
For BPF args[0] is at 16 bytes, args[1] is at 24 bytes. But in reality
it's at 12 byte offset for args[0] and 16 byte offset for args[1]. So
what libbpf/CO-RE is doing is right, it actually relocates offset 16
to 12 and 24 to 16. So without CO-RE you'd be accessing completely
wrong offsets and reading garbage.
3. But read size matters as well. When compiling your BPF program
clang sees 8-byte read and emits corresponding BPF assembly
instruction. While in reality it has to be 4-byte read. Then later in
the verifier (see tp_prog_is_valid_access() in
kernel/trace/bpf_trace.c) there is a very simple check that all
offsets are multiples of access size, and 12 % 8 != 0, so the verifier
complains.
It's a bit verbose explanation, but I hope it describes the problem.
Basically, longs in a mixed BPF/32-bit land are error prone (as well
as pointers in kernel data structures, btw).
Now, a short-term fix for your use case would be to copy/paste struct
trace_event_raw_sys_enter and replace all longs with ints. That will
fix access size and offsets.
Longer-term and a more proper fix is probably for libbpf to be smarter
when emitting vmlinux.h and replace 4-byte longs with ints, knowing
that vmlinux.h is going to be used from always-64-bit BPF
architecture. I'll think a bit more about the implications of this,
and if there are no problems with that approach, I'll submit a change
soon enough. Pointers will still be a problem, though, and I'm not
sure how we can solve it yet. Ideally there would be some solution
that would cause Clang to emit 4-byte reads for pointers. I wonder if
anyone else has any ideas around that.
As a separate thing, it might be a good idea to allow 4-byte aligned
8-byte reads in the verifier on 32-bit architectures, given they are
allowed on such architectures. Though for you that would lead to
garbage in the upper 4 bytes of BPF register, which is equally bad.
>
>
> Kernel commit
> --------------------------------------------
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/?h=v5.9-rc3
>
>
> Struct definition from vmlinux.h
> --------------------------------------------
> struct trace_event_raw_sys_enter {
> struct trace_entry ent;
> long int id;
> long unsigned int args[6];
> char __data[0];
> };
>
>
[...]
> libbpf: collected 0 externs total
> libbpf: loading kernel BTF '/sys/kernel/btf/vmlinux': 0
> libbpf: sec 'tracepoint/syscalls/sys_enter_open': found 2 CO-RE relocations
> libbpf: prog 'tracepoint/syscalls/sys_enter_open': relo #0: kind
> <byte_off> (0), spec is [2] struct trace_event_raw_sys_ent)
these logs seem to be truncated, btw
> libbpf: CO-RE relocating [2] struct trace_event_raw_sys_enter: found
> target candidate [4639] struct trace_event_raw_sys_entr
> libbpf: prog 'tracepoint/syscalls/sys_enter_open': relo #0: matching
> candidate #0 [4639] struct trace_event_raw_sys_enter.a)
like here it actually should be something like:
struct trace_event_raw_sys_enter.args[0]
> libbpf: prog 'tracepoint/syscalls/sys_enter_open': relo #0: patched
> insn #0 (LDX/ST/STX) off 24 -> 16
> libbpf: prog 'tracepoint/syscalls/sys_enter_open': relo #1: kind
> <byte_off> (0), spec is [2] struct trace_event_raw_sys_ent)
> libbpf: prog 'tracepoint/syscalls/sys_enter_open': relo #1: matching
> candidate #0 [4639] struct trace_event_raw_sys_enter.a)
> libbpf: prog 'tracepoint/syscalls/sys_enter_open': relo #1: patched
> insn #1 (LDX/ST/STX) off 16 -> 12
> libbpf: load bpf program failed: Permission denied
> libbpf: -- BEGIN DUMP LOG ---
> libbpf:
> Unrecognized arg#0 type PTR
> ; int arg2 = (int)ctx->args[1];
> 0: (79) r6 = *(u64 *)(r1 +16)
> ; const char *arg1 = (const char *)ctx->args[0];
> 1: (79) r3 = *(u64 *)(r1 +12)
> invalid bpf_context access off=12 size=8
> processed 2 insns (limit 1000000) max_states_per_insn 0 total_states 0
> peak_states 0 mark_read 0
>
> libbpf: -- END LOG --
> libbpf: failed to load program 'tracepoint/syscalls/sys_enter_open'
> libbpf: failed to load object 'hello_bpf'
> libbpf: failed to load BPF skeleton 'hello_bpf': -4007
> failed to load BPF object -4007
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: Problems with pointer offsets on ARM32
2020-09-14 17:49 ` Andrii Nakryiko
@ 2020-09-15 7:26 ` Luka Oreskovic
2020-09-30 23:09 ` Andrii Nakryiko
0 siblings, 1 reply; 8+ messages in thread
From: Luka Oreskovic @ 2020-09-15 7:26 UTC (permalink / raw)
To: Andrii Nakryiko; +Cc: bpf, Luka Perkov, Juraj Vijtiuk
On Mon, Sep 14, 2020 at 7:49 PM Andrii Nakryiko
<andrii.nakryiko@gmail.com> wrote:
>
> On Mon, Sep 14, 2020 at 12:55 AM Luka Oreskovic
> <luka.oreskovic@sartura.hr> wrote:
> >
> > On Fri, Sep 11, 2020 at 8:14 PM Andrii Nakryiko
> > <andrii.nakryiko@gmail.com> wrote:
> > >
> > > On Fri, Sep 11, 2020 at 9:45 AM Luka Oreskovic
> > > <luka.oreskovic@sartura.hr> wrote:
> > > >
> > > > Greetings everyone,
> > > >
> > > > I have been testing various BPF programs on the ARM32 architecture and
> > > > have encountered a strange error.
> > > >
> > > > When trying to run a simple program that prints out the arguments of
> > > > the open syscall,
> > > > I found some strange behaviour with the pointer offsets when accessing
> > > > the arguments:
> > > > The output of llvm-objdump differed from the verifier error dump log.
> > > > Notice the differences in lines 0 and 1. Why is the bytecode being
> > > > altered at runtime?
> > > >
> > > > I attached the program, the llvm-objdump result and the verifier dump below.
> > > >
> > > > Best wishes,
> > > > Luka Oreskovic
> > > >
> > > > BPF program
> > > > --------------------------------------------
> > > > #include "vmlinux.h"
> > > > #include <bpf/bpf_helpers.h>
> > > >
> > > > SEC("tracepoint/syscalls/sys_enter_open")
> > > > int tracepoint__syscalls__sys_enter_open(struct trace_event_raw_sys_enter* ctx)
> > > > {
> > > > const char *arg1 = (const char *)ctx->args[0];
> > > > int arg2 = (int)ctx->args[1];
> > > >
> > > > bpf_printk("Open arg 1: %s\n", arg1);
> > > > bpf_printk("Open arg 2: %d\n", arg2);
> > > >
> > > > return 0;
> > > > }
> > > >
> > > > char LICENSE[] SEC("license") = "GPL";
> > > >
> > > >
> > > > llvm-objdump of program
> > > > --------------------------------------------
> > > > Disassembly of section tracepoint/syscalls/sys_enter_open:
> > > >
> > > > 0000000000000000 tracepoint__syscalls__sys_enter_open:
> > > > ; int arg2 = (int)ctx->args[1];
> > > > 0: 79 16 18 00 00 00 00 00 r6 = *(u64 *)(r1 + 24)
> > > > ; const char *arg1 = (const char *)ctx->args[0];
> > > > 1: 79 13 10 00 00 00 00 00 r3 = *(u64 *)(r1 + 16)
> > > > 2: 18 01 00 00 20 31 3a 20 00 00 00 00 25 73 0a 00 r1 =
> > > > 2941353058775328 ll
> > > > ; bpf_printk("Open arg 1: %s\n", arg1);
> > > > 4: 7b 1a f8 ff 00 00 00 00 *(u64 *)(r10 - 8) = r1
> > > > 5: 18 07 00 00 4f 70 65 6e 00 00 00 00 20 61 72 67 r7 =
> > > > 7454127125170581583 ll
> > > > 7: 7b 7a f0 ff 00 00 00 00 *(u64 *)(r10 - 16) = r7
> > > > 8: bf a1 00 00 00 00 00 00 r1 = r10
> > > > 9: 07 01 00 00 f0 ff ff ff r1 += -16
> > > > 10: b7 02 00 00 10 00 00 00 r2 = 16
> > > > 11: 85 00 00 00 06 00 00 00 call 6
> > > > 12: 18 01 00 00 20 32 3a 20 00 00 00 00 25 64 0a 00 r1 =
> > > > 2924860384358944 ll
> > > > ; bpf_printk("Open arg 2: %d\n", arg2);
> > > > 14: 7b 1a f8 ff 00 00 00 00 *(u64 *)(r10 - 8) = r1
> > > > 15: 7b 7a f0 ff 00 00 00 00 *(u64 *)(r10 - 16) = r7
> > > > 16: bf a1 00 00 00 00 00 00 r1 = r10
> > > > 17: 07 01 00 00 f0 ff ff ff r1 += -16
> > > > 18: b7 02 00 00 10 00 00 00 r2 = 16
> > > > 19: bf 63 00 00 00 00 00 00 r3 = r6
> > > > 20: 85 00 00 00 06 00 00 00 call 6
> > > > ; return 0;
> > > > 21: b7 00 00 00 00 00 00 00 r0 = 0
> > > > 22: 95 00 00 00 00 00 00 00 exit
> > > >
> > > >
> > > > verifier output when running program
> > > > --------------------------------------------
> > > > libbpf: -- BEGIN DUMP LOG ---
> > > > libbpf:
> > > > Unrecognized arg#0 type PTR
> > > > ; int arg2 = (int)ctx->args[1];
> > > > 0: (79) r6 = *(u64 *)(r1 +16)
> > > > ; const char *arg1 = (const char *)ctx->args[0];
> > > > 1: (79) r3 = *(u64 *)(r1 +12)
> > > > invalid bpf_context access off=12 size=8
> > > > processed 2 insns (limit 1000000) max_states_per_insn 0 total_states 0
> > > > peak_states 0 mark_read 0
> > > >
> > > > libbpf: -- END LOG --
> > >
> > >
> > > One suspect would be libbpf's CO-RE relocations. Can you send full
> > > debug libbpf logs, it will have a full log of what libbpf adjusted.
> > > Please also include the definition of struct trace_event_raw_sys_enter
> > > from your vmlinux.h, as well as commit that your kernel was built from
> > > (to check the original definition).
> >
> >
> > Here is the data you requested. I can see the reallocations done by BPF CO-RE,
> > but I don't understand why they would have to be done in the first place
> > since I am using the vmlinux.h that has been generated using the
> > devices vmlinux.
> > Even if it made sense to change the pointer offsets, they shouldn't
> > break the program.
>
> Ok, there are few things at work here. I'll try to explain it and
> let's see how we can fix this. This is all due to 32-bit architecture,
> of course.
>
> 1. BPF "architecture" is strictly 64-bit one. This means that from the
> BPF program point of view pointers are 64-bit values, long is 64-bit
> as well.
> 2. look at struct trace_event_raw_sys_enter below: id and each element
> of args[] array are longs. What this means is the following:
> - from BPF side of view those are assumed to be 64-bit integers
> - but in reality in ARM32 kernel and in memory they are 32-bit integers.
>
> So the memory view of struct trace_event_raw_sys_enter differs quite a
> lot, depending in which context that struct definition is used. You
> can see that from the compiled BPF assembly output above and compare
> to the actual memory layout, taking into account that long is 4 bytes.
> For BPF args[0] is at 16 bytes, args[1] is at 24 bytes. But in reality
> it's at 12 byte offset for args[0] and 16 byte offset for args[1]. So
> what libbpf/CO-RE is doing is right, it actually relocates offset 16
> to 12 and 24 to 16. So without CO-RE you'd be accessing completely
> wrong offsets and reading garbage.
> 3. But read size matters as well. When compiling your BPF program
> clang sees 8-byte read and emits corresponding BPF assembly
> instruction. While in reality it has to be 4-byte read. Then later in
> the verifier (see tp_prog_is_valid_access() in
> kernel/trace/bpf_trace.c) there is a very simple check that all
> offsets are multiples of access size, and 12 % 8 != 0, so the verifier
> complains.
>
> It's a bit verbose explanation, but I hope it describes the problem.
> Basically, longs in a mixed BPF/32-bit land are error prone (as well
> as pointers in kernel data structures, btw).
>
> Now, a short-term fix for your use case would be to copy/paste struct
> trace_event_raw_sys_enter and replace all longs with ints. That will
> fix access size and offsets.
>
> Longer-term and a more proper fix is probably for libbpf to be smarter
> when emitting vmlinux.h and replace 4-byte longs with ints, knowing
> that vmlinux.h is going to be used from always-64-bit BPF
> architecture. I'll think a bit more about the implications of this,
> and if there are no problems with that approach, I'll submit a change
> soon enough. Pointers will still be a problem, though, and I'm not
> sure how we can solve it yet. Ideally there would be some solution
> that would cause Clang to emit 4-byte reads for pointers. I wonder if
> anyone else has any ideas around that.
>
> As a separate thing, it might be a good idea to allow 4-byte aligned
> 8-byte reads in the verifier on 32-bit architectures, given they are
> allowed on such architectures. Though for you that would lead to
> garbage in the upper 4 bytes of BPF register, which is equally bad.
>
> >
> >
> > Kernel commit
> > --------------------------------------------
> > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/?h=v5.9-rc3
> >
> >
> > Struct definition from vmlinux.h
> > --------------------------------------------
> > struct trace_event_raw_sys_enter {
> > struct trace_entry ent;
> > long int id;
> > long unsigned int args[6];
> > char __data[0];
> > };
> >
> >
>
> [...]
>
> > libbpf: collected 0 externs total
> > libbpf: loading kernel BTF '/sys/kernel/btf/vmlinux': 0
> > libbpf: sec 'tracepoint/syscalls/sys_enter_open': found 2 CO-RE relocations
> > libbpf: prog 'tracepoint/syscalls/sys_enter_open': relo #0: kind
> > <byte_off> (0), spec is [2] struct trace_event_raw_sys_ent)
>
> these logs seem to be truncated, btw
>
> > libbpf: CO-RE relocating [2] struct trace_event_raw_sys_enter: found
> > target candidate [4639] struct trace_event_raw_sys_entr
> > libbpf: prog 'tracepoint/syscalls/sys_enter_open': relo #0: matching
> > candidate #0 [4639] struct trace_event_raw_sys_enter.a)
>
> like here it actually should be something like:
>
> struct trace_event_raw_sys_enter.args[0]
>
> > libbpf: prog 'tracepoint/syscalls/sys_enter_open': relo #0: patched
> > insn #0 (LDX/ST/STX) off 24 -> 16
> > libbpf: prog 'tracepoint/syscalls/sys_enter_open': relo #1: kind
> > <byte_off> (0), spec is [2] struct trace_event_raw_sys_ent)
> > libbpf: prog 'tracepoint/syscalls/sys_enter_open': relo #1: matching
> > candidate #0 [4639] struct trace_event_raw_sys_enter.a)
> > libbpf: prog 'tracepoint/syscalls/sys_enter_open': relo #1: patched
> > insn #1 (LDX/ST/STX) off 16 -> 12
> > libbpf: load bpf program failed: Permission denied
> > libbpf: -- BEGIN DUMP LOG ---
> > libbpf:
> > Unrecognized arg#0 type PTR
> > ; int arg2 = (int)ctx->args[1];
> > 0: (79) r6 = *(u64 *)(r1 +16)
> > ; const char *arg1 = (const char *)ctx->args[0];
> > 1: (79) r3 = *(u64 *)(r1 +12)
> > invalid bpf_context access off=12 size=8
> > processed 2 insns (limit 1000000) max_states_per_insn 0 total_states 0
> > peak_states 0 mark_read 0
> >
> > libbpf: -- END LOG --
> > libbpf: failed to load program 'tracepoint/syscalls/sys_enter_open'
> > libbpf: failed to load object 'hello_bpf'
> > libbpf: failed to load BPF skeleton 'hello_bpf': -4007
> > failed to load BPF object -4007
Thank you very much for your informative reply. I look forward to the changes.
Best wishes,
Luka Oreskovic
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: Problems with pointer offsets on ARM32
2020-09-15 7:26 ` Luka Oreskovic
@ 2020-09-30 23:09 ` Andrii Nakryiko
2020-10-01 13:40 ` Luka Oreskovic
0 siblings, 1 reply; 8+ messages in thread
From: Andrii Nakryiko @ 2020-09-30 23:09 UTC (permalink / raw)
To: Luka Oreskovic; +Cc: bpf, Luka Perkov, Juraj Vijtiuk
On Tue, Sep 15, 2020 at 12:26 AM Luka Oreskovic
<luka.oreskovic@sartura.hr> wrote:
>
> On Mon, Sep 14, 2020 at 7:49 PM Andrii Nakryiko
> <andrii.nakryiko@gmail.com> wrote:
> >
> > On Mon, Sep 14, 2020 at 12:55 AM Luka Oreskovic
> > <luka.oreskovic@sartura.hr> wrote:
> > >
> > > On Fri, Sep 11, 2020 at 8:14 PM Andrii Nakryiko
> > > <andrii.nakryiko@gmail.com> wrote:
> > > >
> > > > On Fri, Sep 11, 2020 at 9:45 AM Luka Oreskovic
> > > > <luka.oreskovic@sartura.hr> wrote:
> > > > >
> > > > > Greetings everyone,
> > > > >
> > > > > I have been testing various BPF programs on the ARM32 architecture and
> > > > > have encountered a strange error.
> > > > >
> > > > > When trying to run a simple program that prints out the arguments of
> > > > > the open syscall,
> > > > > I found some strange behaviour with the pointer offsets when accessing
> > > > > the arguments:
> > > > > The output of llvm-objdump differed from the verifier error dump log.
> > > > > Notice the differences in lines 0 and 1. Why is the bytecode being
> > > > > altered at runtime?
> > > > >
> > > > > I attached the program, the llvm-objdump result and the verifier dump below.
> > > > >
> > > > > Best wishes,
> > > > > Luka Oreskovic
> > > > >
> > > > > BPF program
> > > > > --------------------------------------------
> > > > > #include "vmlinux.h"
> > > > > #include <bpf/bpf_helpers.h>
> > > > >
> > > > > SEC("tracepoint/syscalls/sys_enter_open")
> > > > > int tracepoint__syscalls__sys_enter_open(struct trace_event_raw_sys_enter* ctx)
> > > > > {
> > > > > const char *arg1 = (const char *)ctx->args[0];
> > > > > int arg2 = (int)ctx->args[1];
Luka, can you apply the changes below to bpf_core_read.h header and
read these args using BPF_CORE_READ() macro:
const char *arg1 = (const char *)BPF_CORE_READ(ctx, args[0]);
int arg2 = BPF_CORE_READ(ctx, args[1]);
I'm curious if that will work (unfortunately I don't have a complete
enough setup to test this).
The patch is as follows (with broken tab<->space conversion, so please
make changes by hand):
diff --git a/tools/lib/bpf/bpf_core_read.h b/tools/lib/bpf/bpf_core_read.h
index bbcefb3ff5a5..fee6328d36c0 100644
--- a/tools/lib/bpf/bpf_core_read.h
+++ b/tools/lib/bpf/bpf_core_read.h
@@ -261,14 +261,16 @@ enum bpf_enum_value_kind {
#define ___type(...) typeof(___arrow(__VA_ARGS__))
#define ___read(read_fn, dst, src_type, src, accessor) \
- read_fn((void *)(dst), sizeof(*(dst)), &((src_type)(src))->accessor)
+ read_fn((void *)(dst), \
+ bpf_core_field_size(((src_type)(src))->accessor), \
+ &((src_type)(src))->accessor)
/* "recursively" read a sequence of inner pointers using local __t var */
#define ___rd_first(src, a) ___read(bpf_core_read, &__t, ___type(src), src, a);
#define ___rd_last(...)
\
___read(bpf_core_read, &__t, \
___type(___nolast(__VA_ARGS__)), __t, ___last(__VA_ARGS__));
-#define ___rd_p1(...) const void *__t; ___rd_first(__VA_ARGS__)
+#define ___rd_p1(...) const void *__t = (void *)0; ___rd_first(__VA_ARGS__)
#define ___rd_p2(...) ___rd_p1(___nolast(__VA_ARGS__)) ___rd_last(__VA_ARGS__)
#define ___rd_p3(...) ___rd_p2(___nolast(__VA_ARGS__)) ___rd_last(__VA_ARGS__)
#define ___rd_p4(...) ___rd_p3(___nolast(__VA_ARGS__)) ___rd_last(__VA_ARGS__)
BTW, this approach should work for reading pointers as well, it would
be nice if you can test that as well. E.g., something like the
following:
struct task_struct *t = (void *)bpf_get_current_task();
int ppid = BPF_CORE_READ(t, group_leader, tgid);
If you try it without the patch above, it should either read garbage
or zero, but not a valid parent PID (please verify that as well).
I really appreciate your help with testing, thanks!
> > > > >
> > > > > bpf_printk("Open arg 1: %s\n", arg1);
> > > > > bpf_printk("Open arg 2: %d\n", arg2);
> > > > >
> > > > > return 0;
> > > > > }
> > > > >
> > > > > char LICENSE[] SEC("license") = "GPL";
> > > > >
> > > > >
[...]
>
> Best wishes,
> Luka Oreskovic
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: Problems with pointer offsets on ARM32
2020-09-30 23:09 ` Andrii Nakryiko
@ 2020-10-01 13:40 ` Luka Oreskovic
2020-10-01 18:21 ` Andrii Nakryiko
0 siblings, 1 reply; 8+ messages in thread
From: Luka Oreskovic @ 2020-10-01 13:40 UTC (permalink / raw)
To: Andrii Nakryiko; +Cc: bpf, Luka Perkov, Juraj Vijtiuk
On Thu, Oct 1, 2020 at 1:09 AM Andrii Nakryiko
<andrii.nakryiko@gmail.com> wrote:
>
> On Tue, Sep 15, 2020 at 12:26 AM Luka Oreskovic
> <luka.oreskovic@sartura.hr> wrote:
> >
> > On Mon, Sep 14, 2020 at 7:49 PM Andrii Nakryiko
> > <andrii.nakryiko@gmail.com> wrote:
> > >
> > > On Mon, Sep 14, 2020 at 12:55 AM Luka Oreskovic
> > > <luka.oreskovic@sartura.hr> wrote:
> > > >
> > > > On Fri, Sep 11, 2020 at 8:14 PM Andrii Nakryiko
> > > > <andrii.nakryiko@gmail.com> wrote:
> > > > >
> > > > > On Fri, Sep 11, 2020 at 9:45 AM Luka Oreskovic
> > > > > <luka.oreskovic@sartura.hr> wrote:
> > > > > >
> > > > > > Greetings everyone,
> > > > > >
> > > > > > I have been testing various BPF programs on the ARM32 architecture and
> > > > > > have encountered a strange error.
> > > > > >
> > > > > > When trying to run a simple program that prints out the arguments of
> > > > > > the open syscall,
> > > > > > I found some strange behaviour with the pointer offsets when accessing
> > > > > > the arguments:
> > > > > > The output of llvm-objdump differed from the verifier error dump log.
> > > > > > Notice the differences in lines 0 and 1. Why is the bytecode being
> > > > > > altered at runtime?
> > > > > >
> > > > > > I attached the program, the llvm-objdump result and the verifier dump below.
> > > > > >
> > > > > > Best wishes,
> > > > > > Luka Oreskovic
> > > > > >
> > > > > > BPF program
> > > > > > --------------------------------------------
> > > > > > #include "vmlinux.h"
> > > > > > #include <bpf/bpf_helpers.h>
> > > > > >
> > > > > > SEC("tracepoint/syscalls/sys_enter_open")
> > > > > > int tracepoint__syscalls__sys_enter_open(struct trace_event_raw_sys_enter* ctx)
> > > > > > {
> > > > > > const char *arg1 = (const char *)ctx->args[0];
> > > > > > int arg2 = (int)ctx->args[1];
>
> Luka, can you apply the changes below to bpf_core_read.h header and
> read these args using BPF_CORE_READ() macro:
>
> const char *arg1 = (const char *)BPF_CORE_READ(ctx, args[0]);
> int arg2 = BPF_CORE_READ(ctx, args[1]);
>
> I'm curious if that will work (unfortunately I don't have a complete
> enough setup to test this).
>
> The patch is as follows (with broken tab<->space conversion, so please
> make changes by hand):
>
> diff --git a/tools/lib/bpf/bpf_core_read.h b/tools/lib/bpf/bpf_core_read.h
> index bbcefb3ff5a5..fee6328d36c0 100644
> --- a/tools/lib/bpf/bpf_core_read.h
> +++ b/tools/lib/bpf/bpf_core_read.h
> @@ -261,14 +261,16 @@ enum bpf_enum_value_kind {
> #define ___type(...) typeof(___arrow(__VA_ARGS__))
>
> #define ___read(read_fn, dst, src_type, src, accessor) \
> - read_fn((void *)(dst), sizeof(*(dst)), &((src_type)(src))->accessor)
> + read_fn((void *)(dst), \
> + bpf_core_field_size(((src_type)(src))->accessor), \
> + &((src_type)(src))->accessor)
>
> /* "recursively" read a sequence of inner pointers using local __t var */
> #define ___rd_first(src, a) ___read(bpf_core_read, &__t, ___type(src), src, a);
> #define ___rd_last(...)
> \
> ___read(bpf_core_read, &__t, \
> ___type(___nolast(__VA_ARGS__)), __t, ___last(__VA_ARGS__));
> -#define ___rd_p1(...) const void *__t; ___rd_first(__VA_ARGS__)
> +#define ___rd_p1(...) const void *__t = (void *)0; ___rd_first(__VA_ARGS__)
> #define ___rd_p2(...) ___rd_p1(___nolast(__VA_ARGS__)) ___rd_last(__VA_ARGS__)
> #define ___rd_p3(...) ___rd_p2(___nolast(__VA_ARGS__)) ___rd_last(__VA_ARGS__)
> #define ___rd_p4(...) ___rd_p3(___nolast(__VA_ARGS__)) ___rd_last(__VA_ARGS__)
>
>
>
> BTW, this approach should work for reading pointers as well, it would
> be nice if you can test that as well. E.g., something like the
> following:
>
> struct task_struct *t = (void *)bpf_get_current_task();
> int ppid = BPF_CORE_READ(t, group_leader, tgid);
>
> If you try it without the patch above, it should either read garbage
> or zero, but not a valid parent PID (please verify that as well).
>
> I really appreciate your help with testing, thanks!
>
>
> > > > > >
> > > > > > bpf_printk("Open arg 1: %s\n", arg1);
> > > > > > bpf_printk("Open arg 2: %d\n", arg2);
> > > > > >
> > > > > > return 0;
> > > > > > }
> > > > > >
> > > > > > char LICENSE[] SEC("license") = "GPL";
> > > > > >
> > > > > >
>
> [...]
>
> >
> > Best wishes,
> > Luka Oreskovic
Greetings,
I have tested your patch using the BPF_CORE_READ() macro and
everything works great!
As for the pointer read, it prints valid PIDs both with the patch and
without it.
Thank you very much for your help.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: Problems with pointer offsets on ARM32
2020-10-01 13:40 ` Luka Oreskovic
@ 2020-10-01 18:21 ` Andrii Nakryiko
0 siblings, 0 replies; 8+ messages in thread
From: Andrii Nakryiko @ 2020-10-01 18:21 UTC (permalink / raw)
To: Luka Oreskovic; +Cc: bpf, Luka Perkov, Juraj Vijtiuk
On Thu, Oct 1, 2020 at 6:40 AM Luka Oreskovic <luka.oreskovic@sartura.hr> wrote:
>
> On Thu, Oct 1, 2020 at 1:09 AM Andrii Nakryiko
> <andrii.nakryiko@gmail.com> wrote:
> >
> > On Tue, Sep 15, 2020 at 12:26 AM Luka Oreskovic
> > <luka.oreskovic@sartura.hr> wrote:
> > >
> > > On Mon, Sep 14, 2020 at 7:49 PM Andrii Nakryiko
> > > <andrii.nakryiko@gmail.com> wrote:
> > > >
> > > > On Mon, Sep 14, 2020 at 12:55 AM Luka Oreskovic
> > > > <luka.oreskovic@sartura.hr> wrote:
> > > > >
> > > > > On Fri, Sep 11, 2020 at 8:14 PM Andrii Nakryiko
> > > > > <andrii.nakryiko@gmail.com> wrote:
> > > > > >
> > > > > > On Fri, Sep 11, 2020 at 9:45 AM Luka Oreskovic
> > > > > > <luka.oreskovic@sartura.hr> wrote:
> > > > > > >
> > > > > > > Greetings everyone,
> > > > > > >
> > > > > > > I have been testing various BPF programs on the ARM32 architecture and
> > > > > > > have encountered a strange error.
> > > > > > >
> > > > > > > When trying to run a simple program that prints out the arguments of
> > > > > > > the open syscall,
> > > > > > > I found some strange behaviour with the pointer offsets when accessing
> > > > > > > the arguments:
> > > > > > > The output of llvm-objdump differed from the verifier error dump log.
> > > > > > > Notice the differences in lines 0 and 1. Why is the bytecode being
> > > > > > > altered at runtime?
> > > > > > >
> > > > > > > I attached the program, the llvm-objdump result and the verifier dump below.
> > > > > > >
> > > > > > > Best wishes,
> > > > > > > Luka Oreskovic
> > > > > > >
> > > > > > > BPF program
> > > > > > > --------------------------------------------
> > > > > > > #include "vmlinux.h"
> > > > > > > #include <bpf/bpf_helpers.h>
> > > > > > >
> > > > > > > SEC("tracepoint/syscalls/sys_enter_open")
> > > > > > > int tracepoint__syscalls__sys_enter_open(struct trace_event_raw_sys_enter* ctx)
> > > > > > > {
> > > > > > > const char *arg1 = (const char *)ctx->args[0];
> > > > > > > int arg2 = (int)ctx->args[1];
> >
> > Luka, can you apply the changes below to bpf_core_read.h header and
> > read these args using BPF_CORE_READ() macro:
> >
> > const char *arg1 = (const char *)BPF_CORE_READ(ctx, args[0]);
> > int arg2 = BPF_CORE_READ(ctx, args[1]);
> >
> > I'm curious if that will work (unfortunately I don't have a complete
> > enough setup to test this).
> >
> > The patch is as follows (with broken tab<->space conversion, so please
> > make changes by hand):
> >
> > diff --git a/tools/lib/bpf/bpf_core_read.h b/tools/lib/bpf/bpf_core_read.h
> > index bbcefb3ff5a5..fee6328d36c0 100644
> > --- a/tools/lib/bpf/bpf_core_read.h
> > +++ b/tools/lib/bpf/bpf_core_read.h
> > @@ -261,14 +261,16 @@ enum bpf_enum_value_kind {
> > #define ___type(...) typeof(___arrow(__VA_ARGS__))
> >
> > #define ___read(read_fn, dst, src_type, src, accessor) \
> > - read_fn((void *)(dst), sizeof(*(dst)), &((src_type)(src))->accessor)
> > + read_fn((void *)(dst), \
> > + bpf_core_field_size(((src_type)(src))->accessor), \
> > + &((src_type)(src))->accessor)
> >
> > /* "recursively" read a sequence of inner pointers using local __t var */
> > #define ___rd_first(src, a) ___read(bpf_core_read, &__t, ___type(src), src, a);
> > #define ___rd_last(...)
> > \
> > ___read(bpf_core_read, &__t, \
> > ___type(___nolast(__VA_ARGS__)), __t, ___last(__VA_ARGS__));
> > -#define ___rd_p1(...) const void *__t; ___rd_first(__VA_ARGS__)
> > +#define ___rd_p1(...) const void *__t = (void *)0; ___rd_first(__VA_ARGS__)
> > #define ___rd_p2(...) ___rd_p1(___nolast(__VA_ARGS__)) ___rd_last(__VA_ARGS__)
> > #define ___rd_p3(...) ___rd_p2(___nolast(__VA_ARGS__)) ___rd_last(__VA_ARGS__)
> > #define ___rd_p4(...) ___rd_p3(___nolast(__VA_ARGS__)) ___rd_last(__VA_ARGS__)
> >
> >
> >
> > BTW, this approach should work for reading pointers as well, it would
> > be nice if you can test that as well. E.g., something like the
> > following:
> >
> > struct task_struct *t = (void *)bpf_get_current_task();
> > int ppid = BPF_CORE_READ(t, group_leader, tgid);
> >
> > If you try it without the patch above, it should either read garbage
> > or zero, but not a valid parent PID (please verify that as well).
> >
> > I really appreciate your help with testing, thanks!
> >
> >
> > > > > > >
> > > > > > > bpf_printk("Open arg 1: %s\n", arg1);
> > > > > > > bpf_printk("Open arg 2: %d\n", arg2);
> > > > > > >
> > > > > > > return 0;
> > > > > > > }
> > > > > > >
> > > > > > > char LICENSE[] SEC("license") = "GPL";
> > > > > > >
> > > > > > >
> >
> > [...]
> >
> > >
> > > Best wishes,
> > > Luka Oreskovic
>
> Greetings,
>
> I have tested your patch using the BPF_CORE_READ() macro and
> everything works great!
Ok, glad it worked. It needs to be a bit more work to be applicable in
wide range of situation, but you can use it as a work around for now,
at least.
>
> As for the pointer read, it prints valid PIDs both with the patch and
> without it.
That surprised me initially, but then I realized that 32-bit kernel
will just chop off high 32 bits of BPF's 64-bit register when the
helper is getting passed a pointer, so even if those high 32-bits have
garbage (as should be the case here), it still works magically.
>
> Thank you very much for your help.
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2020-10-01 18:21 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-09-11 14:56 Problems with pointer offsets on ARM32 Luka Oreskovic
2020-09-11 18:13 ` Andrii Nakryiko
2020-09-14 7:55 ` Luka Oreskovic
2020-09-14 17:49 ` Andrii Nakryiko
2020-09-15 7:26 ` Luka Oreskovic
2020-09-30 23:09 ` Andrii Nakryiko
2020-10-01 13:40 ` Luka Oreskovic
2020-10-01 18:21 ` Andrii Nakryiko
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.