All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/3] objtool: Generate ORC data for __pfx code
@ 2023-04-12 20:26 Josh Poimboeuf
  2023-04-12 20:26 ` [PATCH 1/3] objtool: Separate prefix code from stack validation code Josh Poimboeuf
                   ` (2 more replies)
  0 siblings, 3 replies; 14+ messages in thread
From: Josh Poimboeuf @ 2023-04-12 20:26 UTC (permalink / raw)
  To: x86; +Cc: linux-kernel, Peter Zijlstra, Miroslav Benes

Josh Poimboeuf (3):
  objtool: Separate prefix code from stack validation code
  x86/linkage: Fix padding for typed functions
  objtool: Generate ORC data for __pfx code

 arch/x86/include/asm/linkage.h |   2 +-
 tools/objtool/check.c          | 102 +++++++++++++++++++++------------
 2 files changed, 65 insertions(+), 39 deletions(-)

-- 
2.39.2


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

* [PATCH 1/3] objtool: Separate prefix code from stack validation code
  2023-04-12 20:26 [PATCH 0/3] objtool: Generate ORC data for __pfx code Josh Poimboeuf
@ 2023-04-12 20:26 ` Josh Poimboeuf
  2023-04-13  9:30   ` Peter Zijlstra
  2023-04-14 14:47   ` [tip: objtool/core] " tip-bot2 for Josh Poimboeuf
  2023-04-12 20:26 ` [PATCH 2/3] x86/linkage: Fix padding for typed functions Josh Poimboeuf
  2023-04-12 20:26 ` [PATCH 3/3] objtool: Generate ORC data for __pfx code Josh Poimboeuf
  2 siblings, 2 replies; 14+ messages in thread
From: Josh Poimboeuf @ 2023-04-12 20:26 UTC (permalink / raw)
  To: x86; +Cc: linux-kernel, Peter Zijlstra, Miroslav Benes

Simplify the prefix code and make it a standalone feature.

Signed-off-by: Josh Poimboeuf <jpoimboe@kernel.org>
---
 tools/objtool/check.c | 88 ++++++++++++++++++++++++-------------------
 1 file changed, 50 insertions(+), 38 deletions(-)

diff --git a/tools/objtool/check.c b/tools/objtool/check.c
index 53940e2ac632..2f3136145b2e 100644
--- a/tools/objtool/check.c
+++ b/tools/objtool/check.c
@@ -4120,54 +4120,61 @@ static bool ignore_unreachable_insn(struct objtool_file *file, struct instructio
 	return false;
 }
 
-static int add_prefix_symbol(struct objtool_file *file, struct symbol *func,
-			     struct instruction *insn)
+static int add_prefix_symbol(struct objtool_file *file, struct symbol *func)
 {
-	if (!opts.prefix)
-		return 0;
+	struct instruction *insn, *prev;
 
-	for (;;) {
-		struct instruction *prev = prev_insn_same_sec(file, insn);
-		u64 offset;
+	insn = find_insn(file, func->sec, func->offset);
+	if (!insn)
+		return -1;
 
-		if (!prev)
-			break;
+	for (prev = prev_insn_same_sec(file, insn);
+	     prev;
+	     prev = prev_insn_same_sec(file, prev)) {
+		u64 offset;
 
 		if (prev->type != INSN_NOP)
-			break;
+			return -1;
 
 		offset = func->offset - prev->offset;
-		if (offset >= opts.prefix) {
-			if (offset == opts.prefix) {
-				/*
-				 * Since the sec->symbol_list is ordered by
-				 * offset (see elf_add_symbol()) the added
-				 * symbol will not be seen by the iteration in
-				 * validate_section().
-				 *
-				 * Hence the lack of list_for_each_entry_safe()
-				 * there.
-				 *
-				 * The direct concequence is that prefix symbols
-				 * don't get visited (because pointless), except
-				 * for the logic in ignore_unreachable_insn()
-				 * that needs the terminating insn to be visited
-				 * otherwise it will report the hole.
-				 *
-				 * Hence mark the first instruction of the
-				 * prefix symbol as visisted.
-				 */
-				prev->visited |= VISITED_BRANCH;
-				elf_create_prefix_symbol(file->elf, func, opts.prefix);
-			}
-			break;
-		}
-		insn = prev;
+
+		if (offset > opts.prefix)
+			return -1;
+
+		if (offset < opts.prefix)
+			continue;
+
+		elf_create_prefix_symbol(file->elf, func, opts.prefix);
+		break;
 	}
 
+	if (!prev)
+		return -1;
+
 	return 0;
 }
 
+static int add_prefix_symbols(struct objtool_file *file)
+{
+	struct section *sec;
+	struct symbol *func;
+	int ret, warnings = 0;
+
+	for_each_sec(file, sec) {
+		if (!(sec->sh.sh_flags & SHF_EXECINSTR))
+			continue;
+
+		list_for_each_entry(func, &sec->symbol_list, list) {
+			if (func->type != STT_FUNC)
+				continue;
+
+			add_prefix_symbol(file, func);
+		}
+	}
+
+	return warnings;
+}
+
 static int validate_symbol(struct objtool_file *file, struct section *sec,
 			   struct symbol *sym, struct insn_state *state)
 {
@@ -4186,8 +4193,6 @@ static int validate_symbol(struct objtool_file *file, struct section *sec,
 	if (!insn || insn->ignore || insn->visited)
 		return 0;
 
-	add_prefix_symbol(file, sym, insn);
-
 	state->uaccess = sym->uaccess_safe;
 
 	ret = validate_branch(file, insn_func(insn), insn, *state);
@@ -4744,6 +4749,13 @@ int check(struct objtool_file *file)
 		warnings += ret;
 	}
 
+	if (opts.prefix) {
+		ret = add_prefix_symbols(file);
+		if (ret < 0)
+			return ret;
+		warnings += ret;
+	}
+
 	if (opts.ibt) {
 		ret = create_ibt_endbr_seal_sections(file);
 		if (ret < 0)
-- 
2.39.2


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

* [PATCH 2/3] x86/linkage: Fix padding for typed functions
  2023-04-12 20:26 [PATCH 0/3] objtool: Generate ORC data for __pfx code Josh Poimboeuf
  2023-04-12 20:26 ` [PATCH 1/3] objtool: Separate prefix code from stack validation code Josh Poimboeuf
@ 2023-04-12 20:26 ` Josh Poimboeuf
  2023-04-14 14:47   ` [tip: objtool/core] " tip-bot2 for Josh Poimboeuf
  2023-04-12 20:26 ` [PATCH 3/3] objtool: Generate ORC data for __pfx code Josh Poimboeuf
  2 siblings, 1 reply; 14+ messages in thread
From: Josh Poimboeuf @ 2023-04-12 20:26 UTC (permalink / raw)
  To: x86; +Cc: linux-kernel, Peter Zijlstra, Miroslav Benes

CFI typed functions are failing to get padded properly for
CONFIG_CALL_PADDING.

Signed-off-by: Josh Poimboeuf <jpoimboe@kernel.org>
---
 arch/x86/include/asm/linkage.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/x86/include/asm/linkage.h b/arch/x86/include/asm/linkage.h
index dd9b8118f784..0953aa32a324 100644
--- a/arch/x86/include/asm/linkage.h
+++ b/arch/x86/include/asm/linkage.h
@@ -99,7 +99,7 @@
 
 /* SYM_TYPED_FUNC_START -- use for indirectly called globals, w/ CFI type */
 #define SYM_TYPED_FUNC_START(name)				\
-	SYM_TYPED_START(name, SYM_L_GLOBAL, SYM_A_ALIGN)	\
+	SYM_TYPED_START(name, SYM_L_GLOBAL, SYM_F_ALIGN)	\
 	ENDBR
 
 /* SYM_FUNC_START -- use for global functions */
-- 
2.39.2


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

* [PATCH 3/3] objtool: Generate ORC data for __pfx code
  2023-04-12 20:26 [PATCH 0/3] objtool: Generate ORC data for __pfx code Josh Poimboeuf
  2023-04-12 20:26 ` [PATCH 1/3] objtool: Separate prefix code from stack validation code Josh Poimboeuf
  2023-04-12 20:26 ` [PATCH 2/3] x86/linkage: Fix padding for typed functions Josh Poimboeuf
@ 2023-04-12 20:26 ` Josh Poimboeuf
  2023-04-13 11:23   ` Peter Zijlstra
                     ` (2 more replies)
  2 siblings, 3 replies; 14+ messages in thread
From: Josh Poimboeuf @ 2023-04-12 20:26 UTC (permalink / raw)
  To: x86; +Cc: linux-kernel, Peter Zijlstra, Miroslav Benes

Allow unwinding from prefix code by copying the CFI from the starting
instruction of the corresponding function.  Even when the NOPs are
replaced, they're still stack-invariant instructions so the same ORC
entry can be reused everywhere.

Signed-off-by: Josh Poimboeuf <jpoimboe@kernel.org>
---
 tools/objtool/check.c | 16 +++++++++++++++-
 1 file changed, 15 insertions(+), 1 deletion(-)

diff --git a/tools/objtool/check.c b/tools/objtool/check.c
index 2f3136145b2e..3f27a0278bf8 100644
--- a/tools/objtool/check.c
+++ b/tools/objtool/check.c
@@ -4123,6 +4123,7 @@ static bool ignore_unreachable_insn(struct objtool_file *file, struct instructio
 static int add_prefix_symbol(struct objtool_file *file, struct symbol *func)
 {
 	struct instruction *insn, *prev;
+	struct cfi_state *cfi;
 
 	insn = find_insn(file, func->sec, func->offset);
 	if (!insn)
@@ -4151,6 +4152,19 @@ static int add_prefix_symbol(struct objtool_file *file, struct symbol *func)
 	if (!prev)
 		return -1;
 
+	if (!insn->cfi) {
+		/*
+		 * This can happen if stack validation isn't enabled or the
+		 * function is annotated with STACK_FRAME_NON_STANDARD.
+		 */
+		return 0;
+	}
+
+	/* Propagate insn->cfi to the prefix code */
+	cfi = cfi_hash_find_or_add(insn->cfi);
+	for (; prev != insn; prev = next_insn_same_sec(file, prev))
+		prev->cfi = cfi;
+
 	return 0;
 }
 
@@ -4158,7 +4172,7 @@ static int add_prefix_symbols(struct objtool_file *file)
 {
 	struct section *sec;
 	struct symbol *func;
-	int ret, warnings = 0;
+	int warnings = 0;
 
 	for_each_sec(file, sec) {
 		if (!(sec->sh.sh_flags & SHF_EXECINSTR))
-- 
2.39.2


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

* Re: [PATCH 1/3] objtool: Separate prefix code from stack validation code
  2023-04-12 20:26 ` [PATCH 1/3] objtool: Separate prefix code from stack validation code Josh Poimboeuf
@ 2023-04-13  9:30   ` Peter Zijlstra
  2023-04-13 15:27     ` Josh Poimboeuf
  2023-04-14 14:47   ` [tip: objtool/core] " tip-bot2 for Josh Poimboeuf
  1 sibling, 1 reply; 14+ messages in thread
From: Peter Zijlstra @ 2023-04-13  9:30 UTC (permalink / raw)
  To: Josh Poimboeuf; +Cc: x86, linux-kernel, Miroslav Benes

On Wed, Apr 12, 2023 at 01:26:13PM -0700, Josh Poimboeuf wrote:
> Simplify the prefix code and make it a standalone feature.

The main thing being that you moved it all after
validate_reachable_instructions() ?


> +static int add_prefix_symbols(struct objtool_file *file)
> +{
> +	struct section *sec;
> +	struct symbol *func;
> +	int ret, warnings = 0;
> +
> +	for_each_sec(file, sec) {
> +		if (!(sec->sh.sh_flags & SHF_EXECINSTR))
> +			continue;
> +
> +		list_for_each_entry(func, &sec->symbol_list, list) {

One of the other patches did a sec_for_each_symbol() thing.

> +			if (func->type != STT_FUNC)
> +				continue;
> +
> +			add_prefix_symbol(file, func);
> +		}
> +	}
> +
> +	return warnings;
> +}

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

* Re: [PATCH 3/3] objtool: Generate ORC data for __pfx code
  2023-04-12 20:26 ` [PATCH 3/3] objtool: Generate ORC data for __pfx code Josh Poimboeuf
@ 2023-04-13 11:23   ` Peter Zijlstra
  2023-04-13 11:24   ` Peter Zijlstra
  2023-04-14 14:47   ` [tip: objtool/core] " tip-bot2 for Josh Poimboeuf
  2 siblings, 0 replies; 14+ messages in thread
From: Peter Zijlstra @ 2023-04-13 11:23 UTC (permalink / raw)
  To: Josh Poimboeuf; +Cc: x86, linux-kernel, Miroslav Benes

On Wed, Apr 12, 2023 at 01:26:15PM -0700, Josh Poimboeuf wrote:

> @@ -4158,7 +4172,7 @@ static int add_prefix_symbols(struct objtool_file *file)
>  {
>  	struct section *sec;
>  	struct symbol *func;
> -	int ret, warnings = 0;
> +	int warnings = 0;
>  
>  	for_each_sec(file, sec) {
>  		if (!(sec->sh.sh_flags & SHF_EXECINSTR))

Stray hunk that should go in the first patch I suppose.

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

* Re: [PATCH 3/3] objtool: Generate ORC data for __pfx code
  2023-04-12 20:26 ` [PATCH 3/3] objtool: Generate ORC data for __pfx code Josh Poimboeuf
  2023-04-13 11:23   ` Peter Zijlstra
@ 2023-04-13 11:24   ` Peter Zijlstra
  2023-04-13 15:29     ` Josh Poimboeuf
  2023-04-14 14:47   ` [tip: objtool/core] " tip-bot2 for Josh Poimboeuf
  2 siblings, 1 reply; 14+ messages in thread
From: Peter Zijlstra @ 2023-04-13 11:24 UTC (permalink / raw)
  To: Josh Poimboeuf; +Cc: x86, linux-kernel, Miroslav Benes

On Wed, Apr 12, 2023 at 01:26:15PM -0700, Josh Poimboeuf wrote:
> Allow unwinding from prefix code by copying the CFI from the starting
> instruction of the corresponding function.  Even when the NOPs are
> replaced, they're still stack-invariant instructions so the same ORC
> entry can be reused everywhere.
> 
> Signed-off-by: Josh Poimboeuf <jpoimboe@kernel.org>
> ---
>  tools/objtool/check.c | 16 +++++++++++++++-
>  1 file changed, 15 insertions(+), 1 deletion(-)
> 
> diff --git a/tools/objtool/check.c b/tools/objtool/check.c
> index 2f3136145b2e..3f27a0278bf8 100644
> --- a/tools/objtool/check.c
> +++ b/tools/objtool/check.c
> @@ -4123,6 +4123,7 @@ static bool ignore_unreachable_insn(struct objtool_file *file, struct instructio
>  static int add_prefix_symbol(struct objtool_file *file, struct symbol *func)
>  {
>  	struct instruction *insn, *prev;
> +	struct cfi_state *cfi;
>  
>  	insn = find_insn(file, func->sec, func->offset);
>  	if (!insn)
> @@ -4151,6 +4152,19 @@ static int add_prefix_symbol(struct objtool_file *file, struct symbol *func)
>  	if (!prev)
>  		return -1;
>  
> +	if (!insn->cfi) {
> +		/*
> +		 * This can happen if stack validation isn't enabled or the
> +		 * function is annotated with STACK_FRAME_NON_STANDARD.
> +		 */
> +		return 0;
> +	}
> +
> +	/* Propagate insn->cfi to the prefix code */
> +	cfi = cfi_hash_find_or_add(insn->cfi);
> +	for (; prev != insn; prev = next_insn_same_sec(file, prev))
> +		prev->cfi = cfi;
> +
>  	return 0;
>  }

FWIW, this makes the whole thing hard rely on the prefix being single
byte NOPs -- which they are, but perhaps we should assert this?

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

* Re: [PATCH 1/3] objtool: Separate prefix code from stack validation code
  2023-04-13  9:30   ` Peter Zijlstra
@ 2023-04-13 15:27     ` Josh Poimboeuf
  0 siblings, 0 replies; 14+ messages in thread
From: Josh Poimboeuf @ 2023-04-13 15:27 UTC (permalink / raw)
  To: Peter Zijlstra; +Cc: x86, linux-kernel, Miroslav Benes

On Thu, Apr 13, 2023 at 11:30:31AM +0200, Peter Zijlstra wrote:
> On Wed, Apr 12, 2023 at 01:26:13PM -0700, Josh Poimboeuf wrote:
> > Simplify the prefix code and make it a standalone feature.
> 
> The main thing being that you moved it all after
> validate_reachable_instructions() ?

The main thing being that I extricated it from validate_symbol() so it's
a proper independent feature.

> > +static int add_prefix_symbols(struct objtool_file *file)
> > +{
> > +	struct section *sec;
> > +	struct symbol *func;
> > +	int ret, warnings = 0;
> > +
> > +	for_each_sec(file, sec) {
> > +		if (!(sec->sh.sh_flags & SHF_EXECINSTR))
> > +			continue;
> > +
> > +		list_for_each_entry(func, &sec->symbol_list, list) {
> 
> One of the other patches did a sec_for_each_symbol() thing.

Too many patch sets floating around ;-)

-- 
Josh

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

* Re: [PATCH 3/3] objtool: Generate ORC data for __pfx code
  2023-04-13 11:24   ` Peter Zijlstra
@ 2023-04-13 15:29     ` Josh Poimboeuf
  2023-04-13 19:24       ` Peter Zijlstra
  0 siblings, 1 reply; 14+ messages in thread
From: Josh Poimboeuf @ 2023-04-13 15:29 UTC (permalink / raw)
  To: Peter Zijlstra; +Cc: x86, linux-kernel, Miroslav Benes

On Thu, Apr 13, 2023 at 01:24:26PM +0200, Peter Zijlstra wrote:
> > +	if (!insn->cfi) {
> > +		/*
> > +		 * This can happen if stack validation isn't enabled or the
> > +		 * function is annotated with STACK_FRAME_NON_STANDARD.
> > +		 */
> > +		return 0;
> > +	}
> > +
> > +	/* Propagate insn->cfi to the prefix code */
> > +	cfi = cfi_hash_find_or_add(insn->cfi);
> > +	for (; prev != insn; prev = next_insn_same_sec(file, prev))
> > +		prev->cfi = cfi;
> > +
> >  	return 0;
> >  }
> 
> FWIW, this makes the whole thing hard rely on the prefix being single
> byte NOPs -- which they are, but perhaps we should assert this?

Couldn't they be any stack-invariant instructions?

-- 
Josh

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

* Re: [PATCH 3/3] objtool: Generate ORC data for __pfx code
  2023-04-13 15:29     ` Josh Poimboeuf
@ 2023-04-13 19:24       ` Peter Zijlstra
  2023-04-13 19:30         ` Josh Poimboeuf
  0 siblings, 1 reply; 14+ messages in thread
From: Peter Zijlstra @ 2023-04-13 19:24 UTC (permalink / raw)
  To: Josh Poimboeuf; +Cc: x86, linux-kernel, Miroslav Benes

On Thu, Apr 13, 2023 at 08:29:33AM -0700, Josh Poimboeuf wrote:
> On Thu, Apr 13, 2023 at 01:24:26PM +0200, Peter Zijlstra wrote:
> > > +	if (!insn->cfi) {
> > > +		/*
> > > +		 * This can happen if stack validation isn't enabled or the
> > > +		 * function is annotated with STACK_FRAME_NON_STANDARD.
> > > +		 */
> > > +		return 0;
> > > +	}
> > > +
> > > +	/* Propagate insn->cfi to the prefix code */
> > > +	cfi = cfi_hash_find_or_add(insn->cfi);
> > > +	for (; prev != insn; prev = next_insn_same_sec(file, prev))
> > > +		prev->cfi = cfi;
> > > +
> > >  	return 0;
> > >  }
> > 
> > FWIW, this makes the whole thing hard rely on the prefix being single
> > byte NOPs -- which they are, but perhaps we should assert this?
> 
> Couldn't they be any stack-invariant instructions?

Hmm, I was thikning that since we don't know the size of the
instructions being written, we need CFI for all offsets. But perhaps,
since we do a left-match on IP, only one entry at the __pfx+0 location
would work?


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

* Re: [PATCH 3/3] objtool: Generate ORC data for __pfx code
  2023-04-13 19:24       ` Peter Zijlstra
@ 2023-04-13 19:30         ` Josh Poimboeuf
  0 siblings, 0 replies; 14+ messages in thread
From: Josh Poimboeuf @ 2023-04-13 19:30 UTC (permalink / raw)
  To: Peter Zijlstra; +Cc: x86, linux-kernel, Miroslav Benes

On Thu, Apr 13, 2023 at 09:24:49PM +0200, Peter Zijlstra wrote:
> On Thu, Apr 13, 2023 at 08:29:33AM -0700, Josh Poimboeuf wrote:
> > On Thu, Apr 13, 2023 at 01:24:26PM +0200, Peter Zijlstra wrote:
> > > > +	if (!insn->cfi) {
> > > > +		/*
> > > > +		 * This can happen if stack validation isn't enabled or the
> > > > +		 * function is annotated with STACK_FRAME_NON_STANDARD.
> > > > +		 */
> > > > +		return 0;
> > > > +	}
> > > > +
> > > > +	/* Propagate insn->cfi to the prefix code */
> > > > +	cfi = cfi_hash_find_or_add(insn->cfi);
> > > > +	for (; prev != insn; prev = next_insn_same_sec(file, prev))
> > > > +		prev->cfi = cfi;
> > > > +
> > > >  	return 0;
> > > >  }
> > > 
> > > FWIW, this makes the whole thing hard rely on the prefix being single
> > > byte NOPs -- which they are, but perhaps we should assert this?
> > 
> > Couldn't they be any stack-invariant instructions?
> 
> Hmm, I was thikning that since we don't know the size of the
> instructions being written, we need CFI for all offsets. But perhaps,
> since we do a left-match on IP, only one entry at the __pfx+0 location
> would work?

Right, while in objtool (almost) every insn has insn->cfi, the actual
ORC entries only get created at the boundaries of change.

-- 
Josh

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

* [tip: objtool/core] objtool: Generate ORC data for __pfx code
  2023-04-12 20:26 ` [PATCH 3/3] objtool: Generate ORC data for __pfx code Josh Poimboeuf
  2023-04-13 11:23   ` Peter Zijlstra
  2023-04-13 11:24   ` Peter Zijlstra
@ 2023-04-14 14:47   ` tip-bot2 for Josh Poimboeuf
  2 siblings, 0 replies; 14+ messages in thread
From: tip-bot2 for Josh Poimboeuf @ 2023-04-14 14:47 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: Josh Poimboeuf, Peter Zijlstra (Intel), x86, linux-kernel

The following commit has been merged into the objtool/core branch of tip:

Commit-ID:     5743654f5e2ebd56df99f56fca5ba4b23fe3c815
Gitweb:        https://git.kernel.org/tip/5743654f5e2ebd56df99f56fca5ba4b23fe3c815
Author:        Josh Poimboeuf <jpoimboe@kernel.org>
AuthorDate:    Wed, 12 Apr 2023 13:26:15 -07:00
Committer:     Peter Zijlstra <peterz@infradead.org>
CommitterDate: Fri, 14 Apr 2023 16:08:30 +02:00

objtool: Generate ORC data for __pfx code

Allow unwinding from prefix code by copying the CFI from the starting
instruction of the corresponding function.  Even when the NOPs are
replaced, they're still stack-invariant instructions so the same ORC
entry can be reused everywhere.

Signed-off-by: Josh Poimboeuf <jpoimboe@kernel.org>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Link: https://lkml.kernel.org/r/bc3344e51f3e87102f1301a0be0f72a7689ea4a4.1681331135.git.jpoimboe@kernel.org
---
 tools/objtool/check.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/tools/objtool/check.c b/tools/objtool/check.c
index 8ee4d51..df634da 100644
--- a/tools/objtool/check.c
+++ b/tools/objtool/check.c
@@ -4117,6 +4117,7 @@ static bool ignore_unreachable_insn(struct objtool_file *file, struct instructio
 static int add_prefix_symbol(struct objtool_file *file, struct symbol *func)
 {
 	struct instruction *insn, *prev;
+	struct cfi_state *cfi;
 
 	insn = find_insn(file, func->sec, func->offset);
 	if (!insn)
@@ -4145,6 +4146,19 @@ static int add_prefix_symbol(struct objtool_file *file, struct symbol *func)
 	if (!prev)
 		return -1;
 
+	if (!insn->cfi) {
+		/*
+		 * This can happen if stack validation isn't enabled or the
+		 * function is annotated with STACK_FRAME_NON_STANDARD.
+		 */
+		return 0;
+	}
+
+	/* Propagate insn->cfi to the prefix code */
+	cfi = cfi_hash_find_or_add(insn->cfi);
+	for (; prev != insn; prev = next_insn_same_sec(file, prev))
+		prev->cfi = cfi;
+
 	return 0;
 }
 

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

* [tip: objtool/core] x86/linkage: Fix padding for typed functions
  2023-04-12 20:26 ` [PATCH 2/3] x86/linkage: Fix padding for typed functions Josh Poimboeuf
@ 2023-04-14 14:47   ` tip-bot2 for Josh Poimboeuf
  0 siblings, 0 replies; 14+ messages in thread
From: tip-bot2 for Josh Poimboeuf @ 2023-04-14 14:47 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: Josh Poimboeuf, Peter Zijlstra (Intel), x86, linux-kernel

The following commit has been merged into the objtool/core branch of tip:

Commit-ID:     4a2c3448ed3d362431c249ec0eb0f90281804ea8
Gitweb:        https://git.kernel.org/tip/4a2c3448ed3d362431c249ec0eb0f90281804ea8
Author:        Josh Poimboeuf <jpoimboe@kernel.org>
AuthorDate:    Wed, 12 Apr 2023 13:26:14 -07:00
Committer:     Peter Zijlstra <peterz@infradead.org>
CommitterDate: Fri, 14 Apr 2023 16:08:30 +02:00

x86/linkage: Fix padding for typed functions

CFI typed functions are failing to get padded properly for
CONFIG_CALL_PADDING.

Signed-off-by: Josh Poimboeuf <jpoimboe@kernel.org>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Link: https://lkml.kernel.org/r/721f0da48d2a49fe907225711b8b76a2b787f9a8.1681331135.git.jpoimboe@kernel.org
---
 arch/x86/include/asm/linkage.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/x86/include/asm/linkage.h b/arch/x86/include/asm/linkage.h
index dd9b811..0953aa3 100644
--- a/arch/x86/include/asm/linkage.h
+++ b/arch/x86/include/asm/linkage.h
@@ -99,7 +99,7 @@
 
 /* SYM_TYPED_FUNC_START -- use for indirectly called globals, w/ CFI type */
 #define SYM_TYPED_FUNC_START(name)				\
-	SYM_TYPED_START(name, SYM_L_GLOBAL, SYM_A_ALIGN)	\
+	SYM_TYPED_START(name, SYM_L_GLOBAL, SYM_F_ALIGN)	\
 	ENDBR
 
 /* SYM_FUNC_START -- use for global functions */

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

* [tip: objtool/core] objtool: Separate prefix code from stack validation code
  2023-04-12 20:26 ` [PATCH 1/3] objtool: Separate prefix code from stack validation code Josh Poimboeuf
  2023-04-13  9:30   ` Peter Zijlstra
@ 2023-04-14 14:47   ` tip-bot2 for Josh Poimboeuf
  1 sibling, 0 replies; 14+ messages in thread
From: tip-bot2 for Josh Poimboeuf @ 2023-04-14 14:47 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: Josh Poimboeuf, Peter Zijlstra (Intel), x86, linux-kernel

The following commit has been merged into the objtool/core branch of tip:

Commit-ID:     bd456a1bedd20cebd37493f8cb0291294a7356ea
Gitweb:        https://git.kernel.org/tip/bd456a1bedd20cebd37493f8cb0291294a7356ea
Author:        Josh Poimboeuf <jpoimboe@kernel.org>
AuthorDate:    Wed, 12 Apr 2023 13:26:13 -07:00
Committer:     Peter Zijlstra <peterz@infradead.org>
CommitterDate: Fri, 14 Apr 2023 16:08:29 +02:00

objtool: Separate prefix code from stack validation code

Simplify the prefix code by moving it after
validate_reachable_instructions().

Signed-off-by: Josh Poimboeuf <jpoimboe@kernel.org>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Link: https://lkml.kernel.org/r/d7f31ac2de462d0cd7b1db01b7ecb525c057c8f6.1681331135.git.jpoimboe@kernel.org
---
 tools/objtool/check.c | 88 +++++++++++++++++++++++-------------------
 1 file changed, 50 insertions(+), 38 deletions(-)

diff --git a/tools/objtool/check.c b/tools/objtool/check.c
index 1cf2e28..8ee4d51 100644
--- a/tools/objtool/check.c
+++ b/tools/objtool/check.c
@@ -4114,54 +4114,61 @@ static bool ignore_unreachable_insn(struct objtool_file *file, struct instructio
 	return false;
 }
 
-static int add_prefix_symbol(struct objtool_file *file, struct symbol *func,
-			     struct instruction *insn)
+static int add_prefix_symbol(struct objtool_file *file, struct symbol *func)
 {
-	if (!opts.prefix)
-		return 0;
+	struct instruction *insn, *prev;
 
-	for (;;) {
-		struct instruction *prev = prev_insn_same_sec(file, insn);
-		u64 offset;
+	insn = find_insn(file, func->sec, func->offset);
+	if (!insn)
+		return -1;
 
-		if (!prev)
-			break;
+	for (prev = prev_insn_same_sec(file, insn);
+	     prev;
+	     prev = prev_insn_same_sec(file, prev)) {
+		u64 offset;
 
 		if (prev->type != INSN_NOP)
-			break;
+			return -1;
 
 		offset = func->offset - prev->offset;
-		if (offset >= opts.prefix) {
-			if (offset == opts.prefix) {
-				/*
-				 * Since the sec->symbol_list is ordered by
-				 * offset (see elf_add_symbol()) the added
-				 * symbol will not be seen by the iteration in
-				 * validate_section().
-				 *
-				 * Hence the lack of list_for_each_entry_safe()
-				 * there.
-				 *
-				 * The direct concequence is that prefix symbols
-				 * don't get visited (because pointless), except
-				 * for the logic in ignore_unreachable_insn()
-				 * that needs the terminating insn to be visited
-				 * otherwise it will report the hole.
-				 *
-				 * Hence mark the first instruction of the
-				 * prefix symbol as visisted.
-				 */
-				prev->visited |= VISITED_BRANCH;
-				elf_create_prefix_symbol(file->elf, func, opts.prefix);
-			}
-			break;
-		}
-		insn = prev;
+
+		if (offset > opts.prefix)
+			return -1;
+
+		if (offset < opts.prefix)
+			continue;
+
+		elf_create_prefix_symbol(file->elf, func, opts.prefix);
+		break;
 	}
 
+	if (!prev)
+		return -1;
+
 	return 0;
 }
 
+static int add_prefix_symbols(struct objtool_file *file)
+{
+	struct section *sec;
+	struct symbol *func;
+	int warnings = 0;
+
+	for_each_sec(file, sec) {
+		if (!(sec->sh.sh_flags & SHF_EXECINSTR))
+			continue;
+
+		sec_for_each_sym(sec, func) {
+			if (func->type != STT_FUNC)
+				continue;
+
+			add_prefix_symbol(file, func);
+		}
+	}
+
+	return warnings;
+}
+
 static int validate_symbol(struct objtool_file *file, struct section *sec,
 			   struct symbol *sym, struct insn_state *state)
 {
@@ -4180,8 +4187,6 @@ static int validate_symbol(struct objtool_file *file, struct section *sec,
 	if (!insn || insn->ignore || insn->visited)
 		return 0;
 
-	add_prefix_symbol(file, sym, insn);
-
 	state->uaccess = sym->uaccess_safe;
 
 	ret = validate_branch(file, insn_func(insn), insn, *state);
@@ -4621,6 +4626,13 @@ int check(struct objtool_file *file)
 		warnings += ret;
 	}
 
+	if (opts.prefix) {
+		ret = add_prefix_symbols(file);
+		if (ret < 0)
+			return ret;
+		warnings += ret;
+	}
+
 	if (opts.ibt) {
 		ret = create_ibt_endbr_seal_sections(file);
 		if (ret < 0)

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

end of thread, other threads:[~2023-04-14 14:48 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-04-12 20:26 [PATCH 0/3] objtool: Generate ORC data for __pfx code Josh Poimboeuf
2023-04-12 20:26 ` [PATCH 1/3] objtool: Separate prefix code from stack validation code Josh Poimboeuf
2023-04-13  9:30   ` Peter Zijlstra
2023-04-13 15:27     ` Josh Poimboeuf
2023-04-14 14:47   ` [tip: objtool/core] " tip-bot2 for Josh Poimboeuf
2023-04-12 20:26 ` [PATCH 2/3] x86/linkage: Fix padding for typed functions Josh Poimboeuf
2023-04-14 14:47   ` [tip: objtool/core] " tip-bot2 for Josh Poimboeuf
2023-04-12 20:26 ` [PATCH 3/3] objtool: Generate ORC data for __pfx code Josh Poimboeuf
2023-04-13 11:23   ` Peter Zijlstra
2023-04-13 11:24   ` Peter Zijlstra
2023-04-13 15:29     ` Josh Poimboeuf
2023-04-13 19:24       ` Peter Zijlstra
2023-04-13 19:30         ` Josh Poimboeuf
2023-04-14 14:47   ` [tip: objtool/core] " tip-bot2 for Josh Poimboeuf

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.