* [PATCH 0/5] objtool: The stack swizzle again
@ 2021-02-03 12:02 Peter Zijlstra
2021-02-03 12:02 ` [PATCH 1/5] objtool: Change REG_SP_INDIRECT Peter Zijlstra
` (4 more replies)
0 siblings, 5 replies; 9+ messages in thread
From: Peter Zijlstra @ 2021-02-03 12:02 UTC (permalink / raw)
To: Josh Poimboeuf, Thomas Gleixner
Cc: Miroslav Benes, Nick Desaulniers, Julien Thierry, Kees Cook, x86,
linux-kernel, peterz
Hi
The stack swizzle is current again, these patches implement objtool support for
the native x86_64 stack swizzle pattern.
It allows Thomas to use his preferred minimal stack swizzle without (much)
limitations.
We used to use SP_INDIRECT with hints back before the entry code rewrite, the
current code relies on asm with a frame-pointer setup, but that all needs to
change again.
Avoid the hint abuse and detect the pattern directly.
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 1/5] objtool: Change REG_SP_INDIRECT
2021-02-03 12:02 [PATCH 0/5] objtool: The stack swizzle again Peter Zijlstra
@ 2021-02-03 12:02 ` Peter Zijlstra
2021-02-03 14:42 ` Josh Poimboeuf
2021-02-03 12:02 ` [PATCH 2/5] objtool: More consistent use of arch_callee_saved_reg() Peter Zijlstra
` (3 subsequent siblings)
4 siblings, 1 reply; 9+ messages in thread
From: Peter Zijlstra @ 2021-02-03 12:02 UTC (permalink / raw)
To: Josh Poimboeuf, Thomas Gleixner
Cc: Miroslav Benes, Nick Desaulniers, Julien Thierry, Kees Cook, x86,
linux-kernel, peterz
Currently REG_SP_INDIRECT is unused but means (%rsp + offset),
change it to mean (%rsp) + offset.
This is somewhat unfortunate, since REG_BP_INDIRECT is used (by DRAP)
and thus needs to retain the current (%rbp + offset).
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
---
arch/x86/kernel/unwind_orc.c | 5 ++++-
tools/objtool/orc_dump.c | 2 +-
2 files changed, 5 insertions(+), 2 deletions(-)
--- a/arch/x86/kernel/unwind_orc.c
+++ b/arch/x86/kernel/unwind_orc.c
@@ -471,7 +471,7 @@ bool unwind_next_frame(struct unwind_sta
break;
case ORC_REG_SP_INDIRECT:
- sp = state->sp + orc->sp_offset;
+ sp = state->sp;
indirect = true;
break;
@@ -521,6 +521,9 @@ bool unwind_next_frame(struct unwind_sta
if (indirect) {
if (!deref_stack_reg(state, sp, &sp))
goto err;
+
+ if (orc->sp_reg == ORC_REG_SP_INDIRECT)
+ sp += orc->sp_offset;
}
/* Find IP, SP and possibly regs: */
--- a/tools/objtool/orc_dump.c
+++ b/tools/objtool/orc_dump.c
@@ -54,7 +54,7 @@ static void print_reg(unsigned int reg,
if (reg == ORC_REG_BP_INDIRECT)
printf("(bp%+d)", offset);
else if (reg == ORC_REG_SP_INDIRECT)
- printf("(sp%+d)", offset);
+ printf("(sp)%+d", offset);
else if (reg == ORC_REG_UNDEFINED)
printf("(und)");
else
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 2/5] objtool: More consistent use of arch_callee_saved_reg()
2021-02-03 12:02 [PATCH 0/5] objtool: The stack swizzle again Peter Zijlstra
2021-02-03 12:02 ` [PATCH 1/5] objtool: Change REG_SP_INDIRECT Peter Zijlstra
@ 2021-02-03 12:02 ` Peter Zijlstra
2021-02-03 12:02 ` [PATCH 3/5] objtool: Prepare for scratch regs Peter Zijlstra
` (2 subsequent siblings)
4 siblings, 0 replies; 9+ messages in thread
From: Peter Zijlstra @ 2021-02-03 12:02 UTC (permalink / raw)
To: Josh Poimboeuf, Thomas Gleixner
Cc: Miroslav Benes, Nick Desaulniers, Julien Thierry, Kees Cook, x86,
linux-kernel, peterz
Since save_regs() will only ever set a reg when it is
arch_callee_saved_reg() all the other regs will always be unused and
are effectively scratch space.
No point in comparing them.
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
---
tools/objtool/check.c | 6 ++++++
1 file changed, 6 insertions(+)
--- a/tools/objtool/check.c
+++ b/tools/objtool/check.c
@@ -1725,6 +1725,9 @@ static bool has_modified_stack_frame(str
return false;
for (i = 0; i < CFI_NUM_REGS; i++) {
+ if (!arch_callee_saved_reg(i))
+ continue;
+
if (cfi->regs[i].base != initial_func_cfi.regs[i].base ||
cfi->regs[i].offset != initial_func_cfi.regs[i].offset)
return true;
@@ -2248,6 +2251,9 @@ static bool insn_cfi_match(struct instru
} else if (memcmp(&cfi1->regs, &cfi2->regs, sizeof(cfi1->regs))) {
for (i = 0; i < CFI_NUM_REGS; i++) {
+ if (!arch_callee_saved_reg(i))
+ continue;
+
if (!memcmp(&cfi1->regs[i], &cfi2->regs[i],
sizeof(struct cfi_reg)))
continue;
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 3/5] objtool: Prepare for scratch regs
2021-02-03 12:02 [PATCH 0/5] objtool: The stack swizzle again Peter Zijlstra
2021-02-03 12:02 ` [PATCH 1/5] objtool: Change REG_SP_INDIRECT Peter Zijlstra
2021-02-03 12:02 ` [PATCH 2/5] objtool: More consistent use of arch_callee_saved_reg() Peter Zijlstra
@ 2021-02-03 12:02 ` Peter Zijlstra
2021-02-03 12:02 ` [PATCH 4/5] objtool,x86: Additionally decode: mov %rsp, (%reg) Peter Zijlstra
2021-02-03 12:02 ` [PATCH 5/5] objtool: Support stack-swizzle Peter Zijlstra
4 siblings, 0 replies; 9+ messages in thread
From: Peter Zijlstra @ 2021-02-03 12:02 UTC (permalink / raw)
To: Josh Poimboeuf, Thomas Gleixner
Cc: Miroslav Benes, Nick Desaulniers, Julien Thierry, Kees Cook, x86,
linux-kernel, peterz
Introduce __save_reg() which allows using the
!arch_callee_saved_reg()s and make sure they're wiped after every
stack op so they don't linger, allow for a single op exception so they
can be used on the very next stack-op.
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
---
tools/objtool/check.c | 27 ++++++++++++++++++++++++---
1 file changed, 24 insertions(+), 3 deletions(-)
--- a/tools/objtool/check.c
+++ b/tools/objtool/check.c
@@ -1775,15 +1775,20 @@ static int update_cfi_state_regs(struct
return 0;
}
-static void save_reg(struct cfi_state *cfi, unsigned char reg, int base, int offset)
+static void __save_reg(struct cfi_state *cfi, unsigned char reg, int base, int offset)
{
- if (arch_callee_saved_reg(reg) &&
- cfi->regs[reg].base == CFI_UNDEFINED) {
+ if (cfi->regs[reg].base == CFI_UNDEFINED) {
cfi->regs[reg].base = base;
cfi->regs[reg].offset = offset;
}
}
+static void save_reg(struct cfi_state *cfi, unsigned char reg, int base, int offset)
+{
+ if (arch_callee_saved_reg(reg))
+ __save_reg(cfi, reg, base, offset);
+}
+
static void restore_reg(struct cfi_state *cfi, unsigned char reg)
{
cfi->regs[reg].base = initial_func_cfi.regs[reg].base;
@@ -1848,6 +1853,7 @@ static int update_cfi_state(struct instr
{
struct cfi_reg *cfa = &cfi->cfa;
struct cfi_reg *regs = cfi->regs;
+ bool skip_wipe = false;
/* stack operations don't make sense with an undefined CFA */
if (cfa->base == CFI_UNDEFINED) {
@@ -2192,6 +2198,21 @@ static int update_cfi_state(struct instr
return -1;
}
+ /*
+ * Only callee saved registers are preserved; the rest is scratch space
+ * preserved at most one instruction.
+ */
+ if (!skip_wipe) {
+ int i;
+
+ for (i = 0; i < CFI_NUM_REGS; i++) {
+ if (arch_callee_saved_reg(i))
+ continue;
+
+ restore_reg(cfi, i);
+ }
+ }
+
return 0;
}
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 4/5] objtool,x86: Additionally decode: mov %rsp, (%reg)
2021-02-03 12:02 [PATCH 0/5] objtool: The stack swizzle again Peter Zijlstra
` (2 preceding siblings ...)
2021-02-03 12:02 ` [PATCH 3/5] objtool: Prepare for scratch regs Peter Zijlstra
@ 2021-02-03 12:02 ` Peter Zijlstra
2021-02-03 12:02 ` [PATCH 5/5] objtool: Support stack-swizzle Peter Zijlstra
4 siblings, 0 replies; 9+ messages in thread
From: Peter Zijlstra @ 2021-02-03 12:02 UTC (permalink / raw)
To: Josh Poimboeuf, Thomas Gleixner
Cc: Miroslav Benes, Nick Desaulniers, Julien Thierry, Kees Cook, x86,
linux-kernel, peterz
Where we already decode: mov %rsp, %reg, also decode mov %rsp, (%reg).
Nothing should match for this new stack-op.
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
---
tools/objtool/arch/x86/decode.c | 23 ++++++++++++++++++-----
1 file changed, 18 insertions(+), 5 deletions(-)
--- a/tools/objtool/arch/x86/decode.c
+++ b/tools/objtool/arch/x86/decode.c
@@ -222,14 +222,24 @@ int arch_decode_instruction(const struct
break;
case 0x89:
- if (rex_w && !rex_r && modrm_mod == 3 && modrm_reg == 4) {
+ if (rex_w && !rex_r && modrm_reg == 4) {
- /* mov %rsp, reg */
+ /* mov %rsp, */
ADD_OP(op) {
op->src.type = OP_SRC_REG;
op->src.reg = CFI_SP;
- op->dest.type = OP_DEST_REG;
- op->dest.reg = op_to_cfi_reg[modrm_rm][rex_b];
+ if (modrm_mod == 3) {
+
+ /* mov %rsp, reg */
+ op->dest.type = OP_DEST_REG;
+ op->dest.reg = op_to_cfi_reg[modrm_rm][rex_b];
+
+ } else if (modrm_mod == 0) {
+
+ /* mov %rsp, (%reg) */
+ op->dest.type = OP_DEST_REG_INDIRECT;
+ op->dest.reg = op_to_cfi_reg[modrm_rm][rex_b];
+ }
}
break;
}
@@ -259,8 +269,10 @@ int arch_decode_instruction(const struct
op->dest.reg = CFI_BP;
op->dest.offset = insn.displacement.value;
}
+ break;
+ }
- } else if (rex_w && !rex_b && modrm_rm == 4 && sib == 0x24) {
+ if (rex_w && !rex_b && modrm_rm == 4 && sib == 0x24) {
/* mov reg, disp(%rsp) */
ADD_OP(op) {
@@ -270,6 +282,7 @@ int arch_decode_instruction(const struct
op->dest.reg = CFI_SP;
op->dest.offset = insn.displacement.value;
}
+ break;
}
break;
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 5/5] objtool: Support stack-swizzle
2021-02-03 12:02 [PATCH 0/5] objtool: The stack swizzle again Peter Zijlstra
` (3 preceding siblings ...)
2021-02-03 12:02 ` [PATCH 4/5] objtool,x86: Additionally decode: mov %rsp, (%reg) Peter Zijlstra
@ 2021-02-03 12:02 ` Peter Zijlstra
4 siblings, 0 replies; 9+ messages in thread
From: Peter Zijlstra @ 2021-02-03 12:02 UTC (permalink / raw)
To: Josh Poimboeuf, Thomas Gleixner
Cc: Miroslav Benes, Nick Desaulniers, Julien Thierry, Kees Cook, x86,
linux-kernel, peterz
Natively support the stack swizzle pattern:
mov %rsp, (%[tos])
mov %[tos], %rsp
...
pop %rsp
with the constraint that %[tos] must be !arch_callee_saved_reg().
It uses the (newly minted) scratch regs to link the first two
stack-ops, and detect the SP to SP_INDIRECT swizzle.
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
---
tools/objtool/check.c | 43 +++++++++++++++++++++++++++++++++++++++++++
1 file changed, 43 insertions(+)
--- a/tools/objtool/check.c
+++ b/tools/objtool/check.c
@@ -1937,6 +1937,34 @@ static int update_cfi_state(struct instr
cfa->offset = -cfi->vals[op->src.reg].offset;
cfi->stack_size = cfa->offset;
+ } else if (cfa->base == CFI_SP &&
+ cfi->regs[op->src.reg].base == CFI_SP_INDIRECT &&
+ cfi->regs[op->src.reg].offset == cfa->offset) {
+
+ /*
+ * Stack swizzle:
+ *
+ * 1: mov %rsp, (%[tos])
+ * 2: mov %[tos], %rsp
+ * ...
+ * 3: pop %rsp
+ *
+ * Where:
+ *
+ * 1 - places a pointer to the previous
+ * stack at the Top-of-Stack of the
+ * new stack.
+ *
+ * 2 - switches to the new stack.
+ *
+ * 3 - pops the Top-of-Stack to restore
+ * the original stack.
+ *
+ * Note:
+ * %[tos] must not be a callee saved reg
+ */
+ cfa->base = CFI_SP_INDIRECT;
+
} else {
cfa->base = CFI_UNDEFINED;
cfa->offset = 0;
@@ -2028,6 +2056,13 @@ static int update_cfi_state(struct instr
case OP_SRC_POP:
case OP_SRC_POPF:
+ if (op->dest.reg == CFI_SP && cfa->base == CFI_SP_INDIRECT) {
+
+ /* pop %rsp; # restore from a stack swizzle */
+ cfa->base = CFI_SP;
+ break;
+ }
+
if (!cfi->drap && op->dest.reg == cfa->base) {
/* pop %rbp */
@@ -2154,6 +2189,14 @@ static int update_cfi_state(struct instr
/* mov reg, disp(%rsp) */
save_reg(cfi, op->src.reg, CFI_CFA,
op->dest.offset - cfi->cfa.offset);
+
+ } else if (op->src.reg == CFI_SP && op->dest.offset == 0) {
+
+ /* mov %rsp, (%reg); # setup a stack swizzle. */
+ if (!arch_callee_saved_reg(op->dest.reg)) {
+ __save_reg(cfi, op->dest.reg, CFI_SP_INDIRECT, cfi->cfa.offset);
+ skip_wipe = true;
+ }
}
break;
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/5] objtool: Change REG_SP_INDIRECT
2021-02-03 12:02 ` [PATCH 1/5] objtool: Change REG_SP_INDIRECT Peter Zijlstra
@ 2021-02-03 14:42 ` Josh Poimboeuf
2021-02-03 14:49 ` Peter Zijlstra
0 siblings, 1 reply; 9+ messages in thread
From: Josh Poimboeuf @ 2021-02-03 14:42 UTC (permalink / raw)
To: Peter Zijlstra
Cc: Thomas Gleixner, Miroslav Benes, Nick Desaulniers,
Julien Thierry, Kees Cook, x86, linux-kernel
On Wed, Feb 03, 2021 at 01:02:23PM +0100, Peter Zijlstra wrote:
> Currently REG_SP_INDIRECT is unused but means (%rsp + offset),
> change it to mean (%rsp) + offset.
>
> This is somewhat unfortunate, since REG_BP_INDIRECT is used (by DRAP)
> and thus needs to retain the current (%rbp + offset).
Offset is going to be zero, should it not work either way?
--
Josh
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/5] objtool: Change REG_SP_INDIRECT
2021-02-03 14:42 ` Josh Poimboeuf
@ 2021-02-03 14:49 ` Peter Zijlstra
2021-02-03 14:55 ` Josh Poimboeuf
0 siblings, 1 reply; 9+ messages in thread
From: Peter Zijlstra @ 2021-02-03 14:49 UTC (permalink / raw)
To: Josh Poimboeuf
Cc: Thomas Gleixner, Miroslav Benes, Nick Desaulniers,
Julien Thierry, Kees Cook, x86, linux-kernel
On Wed, Feb 03, 2021 at 08:42:15AM -0600, Josh Poimboeuf wrote:
> On Wed, Feb 03, 2021 at 01:02:23PM +0100, Peter Zijlstra wrote:
> > Currently REG_SP_INDIRECT is unused but means (%rsp + offset),
> > change it to mean (%rsp) + offset.
> >
> > This is somewhat unfortunate, since REG_BP_INDIRECT is used (by DRAP)
> > and thus needs to retain the current (%rbp + offset).
>
> Offset is going to be zero, should it not work either way?
For DRAP? I couldn't tell in a hurry. I'm ever quite clear on how DRAP
works. Some day, when I figure it out, i'll write a comment.
Anyway, if it's always 0 for DRAP, then yes, I'll change both.
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/5] objtool: Change REG_SP_INDIRECT
2021-02-03 14:49 ` Peter Zijlstra
@ 2021-02-03 14:55 ` Josh Poimboeuf
0 siblings, 0 replies; 9+ messages in thread
From: Josh Poimboeuf @ 2021-02-03 14:55 UTC (permalink / raw)
To: Peter Zijlstra
Cc: Thomas Gleixner, Miroslav Benes, Nick Desaulniers,
Julien Thierry, Kees Cook, x86, linux-kernel
On Wed, Feb 03, 2021 at 03:49:02PM +0100, Peter Zijlstra wrote:
> On Wed, Feb 03, 2021 at 08:42:15AM -0600, Josh Poimboeuf wrote:
> > On Wed, Feb 03, 2021 at 01:02:23PM +0100, Peter Zijlstra wrote:
> > > Currently REG_SP_INDIRECT is unused but means (%rsp + offset),
> > > change it to mean (%rsp) + offset.
> > >
> > > This is somewhat unfortunate, since REG_BP_INDIRECT is used (by DRAP)
> > > and thus needs to retain the current (%rbp + offset).
> >
> > Offset is going to be zero, should it not work either way?
>
> For DRAP? I couldn't tell in a hurry. I'm ever quite clear on how DRAP
> works. Some day, when I figure it out, i'll write a comment.
>
> Anyway, if it's always 0 for DRAP, then yes, I'll change both.
No, what I meant to say is that UNWIND_HINT_INDIRECT is used with no
arguments, which means that sp_offset is always zero.
--
Josh
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2021-02-03 14:57 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-02-03 12:02 [PATCH 0/5] objtool: The stack swizzle again Peter Zijlstra
2021-02-03 12:02 ` [PATCH 1/5] objtool: Change REG_SP_INDIRECT Peter Zijlstra
2021-02-03 14:42 ` Josh Poimboeuf
2021-02-03 14:49 ` Peter Zijlstra
2021-02-03 14:55 ` Josh Poimboeuf
2021-02-03 12:02 ` [PATCH 2/5] objtool: More consistent use of arch_callee_saved_reg() Peter Zijlstra
2021-02-03 12:02 ` [PATCH 3/5] objtool: Prepare for scratch regs Peter Zijlstra
2021-02-03 12:02 ` [PATCH 4/5] objtool,x86: Additionally decode: mov %rsp, (%reg) Peter Zijlstra
2021-02-03 12:02 ` [PATCH 5/5] objtool: Support stack-swizzle Peter Zijlstra
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.