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
CFI typed functions are failing to get padded properly for
CONFIG_CALL_PADDING.
Signed-off-by: Josh Poimboeuf <[email protected]>
---
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
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 <[email protected]>
---
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
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 <[email protected]>
> ---
> 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?
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.
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
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?
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
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 <[email protected]>
AuthorDate: Wed, 12 Apr 2023 13:26:14 -07:00
Committer: Peter Zijlstra <[email protected]>
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 <[email protected]>
Signed-off-by: Peter Zijlstra (Intel) <[email protected]>
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 */
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 <[email protected]>
AuthorDate: Wed, 12 Apr 2023 13:26:15 -07:00
Committer: Peter Zijlstra <[email protected]>
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 <[email protected]>
Signed-off-by: Peter Zijlstra (Intel) <[email protected]>
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;
}