* [PATCH bpf-next] bpftool: Add debug mode for gen_loader.
@ 2021-12-04 19:46 Alexei Starovoitov
2021-12-05 19:42 ` Andrii Nakryiko
0 siblings, 1 reply; 3+ messages in thread
From: Alexei Starovoitov @ 2021-12-04 19:46 UTC (permalink / raw)
To: davem; +Cc: daniel, andrii, bpf, kernel-team
From: Alexei Starovoitov <ast@kernel.org>
Make -d flag functional for gen_loader style program loading.
For example:
$ bpftool prog load -L -d test_d_path.o
... // will print:
libbpf: loading ./test_d_path.o
libbpf: elf: section(3) fentry/security_inode_getattr, size 280, link 0, flags 6, type=1
...
libbpf: prog 'prog_close': found data map 0 (test_d_p.bss, sec 7, off 0) for insn 30
libbpf: gen: load_btf: size 5376
libbpf: gen: map_create: test_d_p.bss idx 0 type 2 value_type_id 118
libbpf: map 'test_d_p.bss': created successfully, fd=0
libbpf: gen: map_update_elem: idx 0
libbpf: sec 'fentry/filp_close': found 1 CO-RE relocations
libbpf: record_relo_core: prog 1 insn[15] struct file 0:1 final insn_idx 15
libbpf: gen: prog_load: type 26 insns_cnt 35 progi_idx 0
libbpf: gen: find_attach_tgt security_inode_getattr 12
libbpf: gen: prog_load: type 26 insns_cnt 37 progi_idx 1
libbpf: gen: find_attach_tgt filp_close 12
libbpf: gen: finish 0
... // at this point libbpf finished generating loader program
0: (bf) r6 = r1
1: (bf) r1 = r10
2: (07) r1 += -136
3: (b7) r2 = 136
4: (b7) r3 = 0
5: (85) call bpf_probe_read_kernel#113
6: (05) goto pc+104
... // this is the assembly dump of the loader program
390: (63) *(u32 *)(r6 +44) = r0
391: (18) r1 = map[idx:0]+5584
393: (61) r0 = *(u32 *)(r1 +0)
394: (63) *(u32 *)(r6 +24) = r0
395: (b7) r0 = 0
396: (95) exit
err 0 // the loader program was loaded and executed successfully
(null)
func#0 @0
... // CO-RE in the kernel logs:
CO-RE relocating STRUCT file: found target candidate [500]
prog '': relo #0: kind <byte_off> (0), spec is [8] STRUCT file.f_path (0:1 @ offset 16)
prog '': relo #0: matching candidate #0 [500] STRUCT file.f_path (0:1 @ offset 16)
prog '': relo #0: patched insn #15 (ALU/ALU64) imm 16 -> 16
vmlinux_cand_cache:[11]file(500),
module_cand_cache:
... // verifier logs when it was checking test_d_path.o program:
R1 type=ctx expected=fp
0: R1=ctx(id=0,off=0,imm=0) R10=fp0
; int BPF_PROG(prog_close, struct file *file, void *id)
0: (79) r6 = *(u64 *)(r1 +0)
func 'filp_close' arg0 has btf_id 500 type STRUCT 'file'
1: R1=ctx(id=0,off=0,imm=0) R6_w=ptr_file(id=0,off=0,imm=0) R10=fp0
; pid_t pid = bpf_get_current_pid_tgid() >> 32;
1: (85) call bpf_get_current_pid_tgid#14
... // if there are multiple programs being loaded by the loader program
... // only the last program in the elf file will be printed, since
... // the same verifier log_buf is used for all PROG_LOAD commands.
Signed-off-by: Alexei Starovoitov <ast@kernel.org>
---
tools/bpf/bpftool/prog.c | 18 ++++++++++--------
1 file changed, 10 insertions(+), 8 deletions(-)
diff --git a/tools/bpf/bpftool/prog.c b/tools/bpf/bpftool/prog.c
index e47e8b06cc3d..b9f42e9e9067 100644
--- a/tools/bpf/bpftool/prog.c
+++ b/tools/bpf/bpftool/prog.c
@@ -1779,12 +1779,14 @@ static int try_loader(struct gen_loader_opts *gen)
ctx = alloca(ctx_sz);
memset(ctx, 0, ctx_sz);
ctx->sz = ctx_sz;
- ctx->log_level = 1;
- ctx->log_size = log_buf_sz;
- log_buf = malloc(log_buf_sz);
- if (!log_buf)
- return -ENOMEM;
- ctx->log_buf = (long) log_buf;
+ if (verifier_logs) {
+ ctx->log_level = 1 + 2 + 4;
+ ctx->log_size = log_buf_sz;
+ log_buf = malloc(log_buf_sz);
+ if (!log_buf)
+ return -ENOMEM;
+ ctx->log_buf = (long) log_buf;
+ }
opts.ctx = ctx;
opts.data = gen->data;
opts.data_sz = gen->data_sz;
@@ -1793,9 +1795,9 @@ static int try_loader(struct gen_loader_opts *gen)
fds_before = count_open_fds();
err = bpf_load_and_run(&opts);
fd_delta = count_open_fds() - fds_before;
- if (err < 0) {
+ if (err < 0 || verifier_logs) {
fprintf(stderr, "err %d\n%s\n%s", err, opts.errstr, log_buf);
- if (fd_delta)
+ if (fd_delta && err < 0)
fprintf(stderr, "loader prog leaked %d FDs\n",
fd_delta);
}
--
2.30.2
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH bpf-next] bpftool: Add debug mode for gen_loader.
2021-12-04 19:46 [PATCH bpf-next] bpftool: Add debug mode for gen_loader Alexei Starovoitov
@ 2021-12-05 19:42 ` Andrii Nakryiko
2021-12-05 21:37 ` Alexei Starovoitov
0 siblings, 1 reply; 3+ messages in thread
From: Andrii Nakryiko @ 2021-12-05 19:42 UTC (permalink / raw)
To: Alexei Starovoitov
Cc: David S. Miller, Daniel Borkmann, Andrii Nakryiko, bpf, Kernel Team
On Sat, Dec 4, 2021 at 11:46 AM Alexei Starovoitov
<alexei.starovoitov@gmail.com> wrote:
>
> From: Alexei Starovoitov <ast@kernel.org>
>
> Make -d flag functional for gen_loader style program loading.
>
> For example:
> $ bpftool prog load -L -d test_d_path.o
> ... // will print:
> libbpf: loading ./test_d_path.o
> libbpf: elf: section(3) fentry/security_inode_getattr, size 280, link 0, flags 6, type=1
> ...
> libbpf: prog 'prog_close': found data map 0 (test_d_p.bss, sec 7, off 0) for insn 30
> libbpf: gen: load_btf: size 5376
> libbpf: gen: map_create: test_d_p.bss idx 0 type 2 value_type_id 118
> libbpf: map 'test_d_p.bss': created successfully, fd=0
> libbpf: gen: map_update_elem: idx 0
> libbpf: sec 'fentry/filp_close': found 1 CO-RE relocations
> libbpf: record_relo_core: prog 1 insn[15] struct file 0:1 final insn_idx 15
> libbpf: gen: prog_load: type 26 insns_cnt 35 progi_idx 0
> libbpf: gen: find_attach_tgt security_inode_getattr 12
> libbpf: gen: prog_load: type 26 insns_cnt 37 progi_idx 1
> libbpf: gen: find_attach_tgt filp_close 12
> libbpf: gen: finish 0
> ... // at this point libbpf finished generating loader program
> 0: (bf) r6 = r1
> 1: (bf) r1 = r10
> 2: (07) r1 += -136
> 3: (b7) r2 = 136
> 4: (b7) r3 = 0
> 5: (85) call bpf_probe_read_kernel#113
> 6: (05) goto pc+104
> ... // this is the assembly dump of the loader program
> 390: (63) *(u32 *)(r6 +44) = r0
> 391: (18) r1 = map[idx:0]+5584
> 393: (61) r0 = *(u32 *)(r1 +0)
> 394: (63) *(u32 *)(r6 +24) = r0
> 395: (b7) r0 = 0
> 396: (95) exit
> err 0 // the loader program was loaded and executed successfully
> (null)
> func#0 @0
> ... // CO-RE in the kernel logs:
> CO-RE relocating STRUCT file: found target candidate [500]
> prog '': relo #0: kind <byte_off> (0), spec is [8] STRUCT file.f_path (0:1 @ offset 16)
> prog '': relo #0: matching candidate #0 [500] STRUCT file.f_path (0:1 @ offset 16)
> prog '': relo #0: patched insn #15 (ALU/ALU64) imm 16 -> 16
> vmlinux_cand_cache:[11]file(500),
> module_cand_cache:
> ... // verifier logs when it was checking test_d_path.o program:
> R1 type=ctx expected=fp
> 0: R1=ctx(id=0,off=0,imm=0) R10=fp0
> ; int BPF_PROG(prog_close, struct file *file, void *id)
> 0: (79) r6 = *(u64 *)(r1 +0)
> func 'filp_close' arg0 has btf_id 500 type STRUCT 'file'
> 1: R1=ctx(id=0,off=0,imm=0) R6_w=ptr_file(id=0,off=0,imm=0) R10=fp0
> ; pid_t pid = bpf_get_current_pid_tgid() >> 32;
> 1: (85) call bpf_get_current_pid_tgid#14
>
> ... // if there are multiple programs being loaded by the loader program
> ... // only the last program in the elf file will be printed, since
> ... // the same verifier log_buf is used for all PROG_LOAD commands.
>
> Signed-off-by: Alexei Starovoitov <ast@kernel.org>
> ---
> tools/bpf/bpftool/prog.c | 18 ++++++++++--------
> 1 file changed, 10 insertions(+), 8 deletions(-)
>
> diff --git a/tools/bpf/bpftool/prog.c b/tools/bpf/bpftool/prog.c
> index e47e8b06cc3d..b9f42e9e9067 100644
> --- a/tools/bpf/bpftool/prog.c
> +++ b/tools/bpf/bpftool/prog.c
> @@ -1779,12 +1779,14 @@ static int try_loader(struct gen_loader_opts *gen)
> ctx = alloca(ctx_sz);
> memset(ctx, 0, ctx_sz);
> ctx->sz = ctx_sz;
> - ctx->log_level = 1;
> - ctx->log_size = log_buf_sz;
> - log_buf = malloc(log_buf_sz);
> - if (!log_buf)
> - return -ENOMEM;
> - ctx->log_buf = (long) log_buf;
> + if (verifier_logs) {
> + ctx->log_level = 1 + 2 + 4;
> + ctx->log_size = log_buf_sz;
> + log_buf = malloc(log_buf_sz);
if verifier_logs is false, log_buf will now be left uninitialized and
passed like that into free(log_buf), crashing or corrupting memory.
I've fixed it up by NULL initializaing and pushed to bpf-next.
> + if (!log_buf)
> + return -ENOMEM;
> + ctx->log_buf = (long) log_buf;
> + }
> opts.ctx = ctx;
> opts.data = gen->data;
> opts.data_sz = gen->data_sz;
> @@ -1793,9 +1795,9 @@ static int try_loader(struct gen_loader_opts *gen)
> fds_before = count_open_fds();
> err = bpf_load_and_run(&opts);
> fd_delta = count_open_fds() - fds_before;
> - if (err < 0) {
> + if (err < 0 || verifier_logs) {
> fprintf(stderr, "err %d\n%s\n%s", err, opts.errstr, log_buf);
> - if (fd_delta)
> + if (fd_delta && err < 0)
> fprintf(stderr, "loader prog leaked %d FDs\n",
> fd_delta);
> }
> --
> 2.30.2
>
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH bpf-next] bpftool: Add debug mode for gen_loader.
2021-12-05 19:42 ` Andrii Nakryiko
@ 2021-12-05 21:37 ` Alexei Starovoitov
0 siblings, 0 replies; 3+ messages in thread
From: Alexei Starovoitov @ 2021-12-05 21:37 UTC (permalink / raw)
To: Andrii Nakryiko, Alexei Starovoitov
Cc: David S. Miller, Daniel Borkmann, Andrii Nakryiko, bpf, Kernel Team
On 12/5/21 11:42 AM, Andrii Nakryiko wrote:
>>
>> diff --git a/tools/bpf/bpftool/prog.c b/tools/bpf/bpftool/prog.c
>> index e47e8b06cc3d..b9f42e9e9067 100644
>> --- a/tools/bpf/bpftool/prog.c
>> +++ b/tools/bpf/bpftool/prog.c
>> @@ -1779,12 +1779,14 @@ static int try_loader(struct gen_loader_opts *gen)
>> ctx = alloca(ctx_sz);
>> memset(ctx, 0, ctx_sz);
>> ctx->sz = ctx_sz;
>> - ctx->log_level = 1;
>> - ctx->log_size = log_buf_sz;
>> - log_buf = malloc(log_buf_sz);
>> - if (!log_buf)
>> - return -ENOMEM;
>> - ctx->log_buf = (long) log_buf;
>> + if (verifier_logs) {
>> + ctx->log_level = 1 + 2 + 4;
>> + ctx->log_size = log_buf_sz;
>> + log_buf = malloc(log_buf_sz);
>
> if verifier_logs is false, log_buf will now be left uninitialized and
> passed like that into free(log_buf), crashing or corrupting memory.
> I've fixed it up by NULL initializaing and pushed to bpf-next.
Indeed. Weird that compiler didn't complain and bpftool didn't crash.
Thanks!
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2021-12-05 21:38 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-12-04 19:46 [PATCH bpf-next] bpftool: Add debug mode for gen_loader Alexei Starovoitov
2021-12-05 19:42 ` Andrii Nakryiko
2021-12-05 21:37 ` Alexei Starovoitov
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).