Hi,
The following patches are the result of limitation found on the CFA
management code when trying to validate arm64 frames. I tried to keep
things simple and not contradict current CFA management logic nor
introduce too many corner cases.
Changes since V2[1]:
- Simplify cfa offset checking for BP and return address
[1] https://lkml.org/lkml/2020/9/28/354
Thanks,
Julien
Julien Thierry (3):
objtool: check: Fully validate the stack frame
objtool: check: Support addition to set CFA base
objtool: check: Make SP memory operation match PUSH/POP semantics
tools/objtool/check.c | 46 +++++++++++++++++++++++++++++++++++++++----
1 file changed, 42 insertions(+), 4 deletions(-)
--
2.25.4
On arm64, the compiler can set the frame pointer either
with a move operation or with and add operation like:
add (SP + constant), BP
For a simple move operation, the CFA base is changed from SP to BP.
Handle also changing the CFA base when the frame pointer is set with
an addition instruction.
Signed-off-by: Julien Thierry <[email protected]>
---
tools/objtool/check.c | 11 +++++++++++
1 file changed, 11 insertions(+)
diff --git a/tools/objtool/check.c b/tools/objtool/check.c
index 87f10e726a75..815aeb770930 100644
--- a/tools/objtool/check.c
+++ b/tools/objtool/check.c
@@ -1898,6 +1898,17 @@ static int update_cfi_state(struct instruction *insn, struct cfi_state *cfi,
break;
}
+ if (!cfi->drap && op->src.reg == CFI_SP &&
+ op->dest.reg == CFI_BP && cfa->base == CFI_SP &&
+ check_reg_frame_pos(®s[CFI_BP], -cfa->offset + op->src.offset)) {
+
+ /* lea disp(%rsp), %rbp */
+ cfa->base = CFI_BP;
+ cfa->offset -= op->src.offset;
+ cfi->bp_scratch = false;
+ break;
+ }
+
if (op->src.reg == CFI_SP && cfa->base == CFI_SP) {
/* drap: lea disp(%rsp), %drap */
--
2.25.4
A valid stack frame should contain both the return address and the
previous frame pointer value.
On x86, the return value is placed on the stack by the calling
instructions. On other architectures, the callee need to explicitly
save the return address on the stack.
Add the necessary checks to verify a function properly sets up all the
elements of the stack frame.
Signed-off-by: Julien Thierry <[email protected]>
---
tools/objtool/check.c | 15 +++++++++++----
1 file changed, 11 insertions(+), 4 deletions(-)
diff --git a/tools/objtool/check.c b/tools/objtool/check.c
index f50ffa243c72..87f10e726a75 100644
--- a/tools/objtool/check.c
+++ b/tools/objtool/check.c
@@ -1671,12 +1671,20 @@ static bool has_modified_stack_frame(struct instruction *insn, struct insn_state
return false;
}
+static bool check_reg_frame_pos(const struct cfi_reg *reg,
+ int expected_offset)
+{
+ return reg->base == CFI_CFA &&
+ reg->offset == expected_offset;
+}
+
static bool has_valid_stack_frame(struct insn_state *state)
{
struct cfi_state *cfi = &state->cfi;
- if (cfi->cfa.base == CFI_BP && cfi->regs[CFI_BP].base == CFI_CFA &&
- cfi->regs[CFI_BP].offset == -16)
+ if (cfi->cfa.base == CFI_BP &&
+ check_reg_frame_pos(&cfi->regs[CFI_BP], -cfi->cfa.offset) &&
+ check_reg_frame_pos(&cfi->regs[CFI_RA], -cfi->cfa.offset + 8))
return true;
if (cfi->drap && cfi->regs[CFI_BP].base == CFI_BP)
@@ -1805,8 +1813,7 @@ static int update_cfi_state(struct instruction *insn, struct cfi_state *cfi,
case OP_SRC_REG:
if (op->src.reg == CFI_SP && op->dest.reg == CFI_BP &&
cfa->base == CFI_SP &&
- regs[CFI_BP].base == CFI_CFA &&
- regs[CFI_BP].offset == -cfa->offset) {
+ check_reg_frame_pos(®s[CFI_BP], -cfa->offset)) {
/* mov %rsp, %rbp */
cfa->base = op->dest.reg;
--
2.25.4
On Wed, Oct 14, 2020 at 08:37:59AM +0100, Julien Thierry wrote:
> Julien Thierry (3):
> objtool: check: Fully validate the stack frame
> objtool: check: Support addition to set CFA base
> objtool: check: Make SP memory operation match PUSH/POP semantics
Acked-by: Peter Zijlstra (Intel) <[email protected]>