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

[Alexandre Chartre <alexandre.chartre@oracle.com>]

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@worktop.programming.kicks-ass.net
> 
> 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.