linux-kernel - Re: [PATCH 1/2] x86/kprobes: Fix kprobes instruction boudary check with CONFIG

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <YxiEJUn03gAJqm71@hirez.programming.kicks-ass.net>
Date:   Wed, 7 Sep 2022 13:44:37 +0200
From:   Peter Zijlstra <peterz@...radead.org>
To:     Masami Hiramatsu <mhiramat@...nel.org>
Cc:     Steven Rostedt <rostedt@...dmis.org>,
        Ingo Molnar <mingo@...nel.org>,
        Suleiman Souhlal <suleiman@...gle.com>,
        bpf <bpf@...r.kernel.org>, linux-kernel@...r.kernel.org,
        Borislav Petkov <bp@...e.de>,
        Josh Poimboeuf <jpoimboe@...nel.org>, x86@...nel.org
Subject: Re: [PATCH 1/2] x86/kprobes: Fix kprobes instruction boudary check
 with CONFIG_RETHUNK

On Wed, Sep 07, 2022 at 12:19:06PM +0200, Peter Zijlstra wrote:
> On Wed, Sep 07, 2022 at 06:49:57PM +0900, Masami Hiramatsu wrote:
> > Yeah, this looks good to me. What I just need is to add expanding
> > queue buffer. (can we use xarray for this purpose?)
> 
> Yeah, xarray might just work.
> 
> I need to go fetch the kids from school, but if I remember I'll modify
> objtool to tell us the max number required here (for any one particular
> build obviously).

quick hack below suggests we need 405 for a x86_64 defconfig+kvm_guest.config vmlinux.o.

  $ OBJTOOL_ARGS="--stats" make O=defconfig-build/ -j12 vmlinux.o
  ...
  max_targets: 405 (hidinput_connect)
  ...

And if you look at the output of:

  $ ./scripts/objdump-func defconfig-build/vmlinux.o hidinput_connect

I'm inclined to believe this. That function is crazy. So yeah, we
definitely need something dynamic.

---
 tools/objtool/check.c               | 12 ++++++++++++
 tools/objtool/include/objtool/elf.h |  1 +
 2 files changed, 13 insertions(+)

diff --git a/tools/objtool/check.c b/tools/objtool/check.c
index e55fdf952a3a..897d3b83ab70 100644
--- a/tools/objtool/check.c
+++ b/tools/objtool/check.c
@@ -3295,6 +3295,9 @@ static struct instruction *next_insn_to_validate(struct objtool_file *file,
 	return next_insn_same_sec(file, insn);
 }
 
+static int max_targets = 0;
+static char *max_name = NULL;
+
 /*
  * Follow the branch starting at the given instruction, and recursively follow
  * any other branches (jumps).  Meanwhile, track the frame pointer state at
@@ -3312,6 +3315,14 @@ static int validate_branch(struct objtool_file *file, struct symbol *func,
 
 	sec = insn->sec;
 
+	if (!insn->visited && func) {
+		func->targets++;
+		if (func->targets > max_targets) {
+			max_targets = func->targets;
+			max_name = func->name;
+		}
+	}
+
 	while (1) {
 		next_insn = next_insn_to_validate(file, insn);
 
@@ -4305,6 +4316,7 @@ int check(struct objtool_file *file)
 		printf("nr_cfi: %ld\n", nr_cfi);
 		printf("nr_cfi_reused: %ld\n", nr_cfi_reused);
 		printf("nr_cfi_cache: %ld\n", nr_cfi_cache);
+		printf("max_targets: %d (%s)\n", max_targets, max_name);
 	}
 
 out:
diff --git a/tools/objtool/include/objtool/elf.h b/tools/objtool/include/objtool/elf.h
index 16f4067b82ae..a707becdef50 100644
--- a/tools/objtool/include/objtool/elf.h
+++ b/tools/objtool/include/objtool/elf.h
@@ -61,6 +61,7 @@ struct symbol {
 	u8 fentry            : 1;
 	u8 profiling_func    : 1;
 	struct list_head pv_target;
+	int targets;
 };
 
 struct reloc {