Re: [PATCH 4/7] objtool: Add support for return trampoline call

[Alexandre Chartre]

On 4/2/20 5:27 PM, Peter Zijlstra wrote:
> On Thu, Apr 02, 2020 at 10:22:17AM +0200, Alexandre Chartre wrote:
> > With retpoline, the return instruction is used to branch to an address
> > stored on the stack. So, unlike a regular return instruction, when a
> > retpoline return instruction is reached the stack has been modified
> > compared to what we have when the function was entered.
> >
> > Provide the mechanism to explicitly call-out such return instruction
> > so that objtool can correctly handle them.
>
> https://lkml.kernel.org/r/20200331222703.GH2452@xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx
>
> And also, the split out version:
>
>    https://git.kernel.org/pub/scm/linux/kernel/git/peterz/queue.git/commit/?h=core/objtool&id=ec9d9549901dfd2ff411676dfc624e50219e4d5a

HINT_RET_OFFSET works fine when an immediate value is pushed on the
stack. However if the value is pushed from a callee-saved register
(%rbp, %rbx, %r12-%r15) then we still have a "return with modified
stack frame" warning. That's because objtool checks callee-saved
registers pushed/popped on the stack, and we have retpoline functions
built for each register (see arch/x86/lib/retpoline.S)

So that's why I also added a bool to has_modified_stack_frame() to
no check registers:

@@ -1432,7 +1478,8 @@ static bool is_fentry_call(struct instruction *insn)
 	return false;
 }
 
-static bool has_modified_stack_frame(struct insn_state *state)
+static bool has_modified_stack_frame(struct insn_state *state,
+				     bool check_registers)
 {
 	int i;
 
@@ -1442,6 +1489,9 @@ static bool has_modified_stack_frame(struct insn_state *state)
 	    state->drap)
 		return true;
 
+	if (!check_registers)
+		return false;
+
 	for (i = 0; i < CFI_NUM_REGS; i++)
 		if (state->regs[i].base != initial_func_cfi.regs[i].base ||
 		    state->regs[i].offset != initial_func_cfi.regs[i].offset)


alex.