public inbox for netdev@vger.kernel.org 
 help / color / mirror / Atom feed
* [PATCH bpf] bpftool: Don't crash on missing jited insns or ksyms
@ 2019-12-10 14:30 Toke Høiland-Jørgensen
  2019-12-10 17:59 ` Martin Lau
  0 siblings, 1 reply; 3+ messages in thread
From: Toke Høiland-Jørgensen @ 2019-12-10 14:30 UTC (permalink / raw)
  To: Daniel Borkmann, Alexei Starovoitov
  Cc: Toke Høiland-Jørgensen, netdev, bpf

When JIT hardening is turned on, the kernel can fail to return jited_ksyms
or jited_prog_insns, but still have positive values in nr_jited_ksyms and
jited_prog_len. This causes bpftool to crash when trying to dump the
program because it only checks the len fields not the actual pointers to
the instructions and ksyms.

Fix this by adding the missing checks.

Signed-off-by: Toke Høiland-Jørgensen <toke@redhat•com>
---
 tools/bpf/bpftool/prog.c          | 2 +-
 tools/bpf/bpftool/xlated_dumper.c | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/tools/bpf/bpftool/prog.c b/tools/bpf/bpftool/prog.c
index 4535c863d2cd..2ce9c5ba1934 100644
--- a/tools/bpf/bpftool/prog.c
+++ b/tools/bpf/bpftool/prog.c
@@ -493,7 +493,7 @@ static int do_dump(int argc, char **argv)
 
 	info = &info_linear->info;
 	if (mode == DUMP_JITED) {
-		if (info->jited_prog_len == 0) {
+		if (info->jited_prog_len == 0 || !info->jited_prog_insns) {
 			p_info("no instructions returned");
 			goto err_free;
 		}
diff --git a/tools/bpf/bpftool/xlated_dumper.c b/tools/bpf/bpftool/xlated_dumper.c
index 494d7ae3614d..5b91ee65a080 100644
--- a/tools/bpf/bpftool/xlated_dumper.c
+++ b/tools/bpf/bpftool/xlated_dumper.c
@@ -174,7 +174,7 @@ static const char *print_call(void *private_data,
 	struct kernel_sym *sym;
 
 	if (insn->src_reg == BPF_PSEUDO_CALL &&
-	    (__u32) insn->imm < dd->nr_jited_ksyms)
+	    (__u32) insn->imm < dd->nr_jited_ksyms && dd->jited_ksyms)
 		address = dd->jited_ksyms[insn->imm];
 
 	sym = kernel_syms_search(dd, address);
-- 
2.24.0


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH bpf] bpftool: Don't crash on missing jited insns or ksyms
  2019-12-10 14:30 [PATCH bpf] bpftool: Don't crash on missing jited insns or ksyms Toke Høiland-Jørgensen
@ 2019-12-10 17:59 ` Martin Lau
  2019-12-10 18:11   ` Toke Høiland-Jørgensen
  0 siblings, 1 reply; 3+ messages in thread
From: Martin Lau @ 2019-12-10 17:59 UTC (permalink / raw)
  To: Toke Høiland-Jørgensen
  Cc: Daniel Borkmann, Alexei Starovoitov, netdev@vger•kernel.org,
	bpf@vger•kernel.org

On Tue, Dec 10, 2019 at 03:30:47PM +0100, Toke Høiland-Jørgensen wrote:
> When JIT hardening is turned on, the kernel can fail to return jited_ksyms
JIT hardening means net.core.bpf_jit_harden?
From the code, it happens on the bpf_dump_raw_ok() check which is
actually "kernel.kptr_restrict" instead?

> or jited_prog_insns, but still have positive values in nr_jited_ksyms and
> jited_prog_len. This causes bpftool to crash when trying to dump the
> program because it only checks the len fields not the actual pointers to
> the instructions and ksyms.
> 
> Fix this by adding the missing checks.
Changes look good.

> 
> Signed-off-by: Toke Høiland-Jørgensen <toke@redhat•com>
> ---
>  tools/bpf/bpftool/prog.c          | 2 +-
>  tools/bpf/bpftool/xlated_dumper.c | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/tools/bpf/bpftool/prog.c b/tools/bpf/bpftool/prog.c
> index 4535c863d2cd..2ce9c5ba1934 100644
> --- a/tools/bpf/bpftool/prog.c
> +++ b/tools/bpf/bpftool/prog.c
> @@ -493,7 +493,7 @@ static int do_dump(int argc, char **argv)
>  
>  	info = &info_linear->info;
>  	if (mode == DUMP_JITED) {
> -		if (info->jited_prog_len == 0) {
> +		if (info->jited_prog_len == 0 || !info->jited_prog_insns) {
>  			p_info("no instructions returned");
>  			goto err_free;
>  		}
> diff --git a/tools/bpf/bpftool/xlated_dumper.c b/tools/bpf/bpftool/xlated_dumper.c
> index 494d7ae3614d..5b91ee65a080 100644
> --- a/tools/bpf/bpftool/xlated_dumper.c
> +++ b/tools/bpf/bpftool/xlated_dumper.c
> @@ -174,7 +174,7 @@ static const char *print_call(void *private_data,
>  	struct kernel_sym *sym;
>  
>  	if (insn->src_reg == BPF_PSEUDO_CALL &&
> -	    (__u32) insn->imm < dd->nr_jited_ksyms)
> +	    (__u32) insn->imm < dd->nr_jited_ksyms && dd->jited_ksyms)
>  		address = dd->jited_ksyms[insn->imm];
>  
>  	sym = kernel_syms_search(dd, address);
> -- 
> 2.24.0
> 

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH bpf] bpftool: Don't crash on missing jited insns or ksyms
  2019-12-10 17:59 ` Martin Lau
@ 2019-12-10 18:11   ` Toke Høiland-Jørgensen
  0 siblings, 0 replies; 3+ messages in thread
From: Toke Høiland-Jørgensen @ 2019-12-10 18:11 UTC (permalink / raw)
  To: Martin Lau
  Cc: Daniel Borkmann, Alexei Starovoitov, netdev@vger•kernel.org,
	bpf@vger•kernel.org

Martin Lau <kafai@fb•com> writes:

> On Tue, Dec 10, 2019 at 03:30:47PM +0100, Toke Høiland-Jørgensen wrote:
>> When JIT hardening is turned on, the kernel can fail to return jited_ksyms
> JIT hardening means net.core.bpf_jit_harden?
> From the code, it happens on the bpf_dump_raw_ok() check which is
> actually "kernel.kptr_restrict" instead?

Ah, yeah, you're right. I was looking through the hardening patchset and
the bpf_jit_harden setting was the first thing I hit upon; must admit I
didn't check this too closely :)

I'll send a v2 with an updated commit message...

-Toke


^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2019-12-10 18:11 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2019-12-10 14:30 [PATCH bpf] bpftool: Don't crash on missing jited insns or ksyms Toke Høiland-Jørgensen
2019-12-10 17:59 ` Martin Lau
2019-12-10 18:11   ` Toke Høiland-Jørgensen

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox