[GIT PULL] livepatching for 5.9-rc5

[GIT PULL] livepatching for 5.9-rc5

[Petr Mladek]

Linus,

please pull the latest livepatching changes from

  git://git.kernel.org/pub/scm/linux/kernel/git/livepatching/livepatching tags/livepatching-for-5.9-rc5

======================================

- Workaround "unreachable instruction" objtool warnings that happen
  with some compiler versions.

----------------------------------------------------------------
Josh Poimboeuf (1):
      Revert "kbuild: use -flive-patching when CONFIG_LIVEPATCH is enabled"

 Makefile | 4 ----
 1 file changed, 4 deletions(-)


Best Regards,
Petr

Re: [GIT PULL] livepatching for 5.9-rc5

[Linus Torvalds]

Josh,

On Mon, Sep 7, 2020 at 1:20 AM Petr Mladek <pmladek@suse.com> wrote:
>
> - Workaround "unreachable instruction" objtool warnings that happen
>   with some compiler versions.

I know I said this fixes things for me, but I just realized it doesn't entirely.

I wonder how I missed the remaining one:

   arch/x86/kvm/vmx/vmx.o: warning: objtool:
vmx_handle_exit_irqoff()+0x142: unreachable instruction

so apparently gcc and objtool can still disagree even without that
'-flive-patching'.

The unreachable code in question is after the call to
handle_external_interrupt_irqoff(), and while that function is a bit
odd, in this case I think it's objtool that is wrong.

I think that what happens is that the function doesn't have a 'ret'
instruction, and instead returns by doing a tail-call to
__sanitizer_cov_trace_pc with my config. And maybe that is what
confuses objtool.

This is current tip-of-git of my tree, with a allmodconfig build (but
the actual config will then depend on things like the gcc plugins
being there too, so you may not get exactly the same thing as I do)

Josh? Am I missing something, and the objtool warning is valid? But
yes, that code is doing some very very special stuff with that thunk
call asm, so it's hard to read the asm.

                  Linus

Re: [GIT PULL] livepatching for 5.9-rc5

[pr-tracker-bot]

The pull request you sent on Mon, 7 Sep 2020 10:20:36 +0200:

> git://git.kernel.org/pub/scm/linux/kernel/git/livepatching/livepatching tags/livepatching-for-5.9-rc5

has been merged into torvalds/linux.git:
https://git.kernel.org/torvalds/c/612ab8ad64140f0f291d7baae45982ce7119839a

Thank you!

-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/prtracker.html

Re: [GIT PULL] livepatching for 5.9-rc5

[Josh Poimboeuf]

On Tue, Sep 08, 2020 at 11:13:58AM -0700, Linus Torvalds wrote:
> Josh,
> 
> On Mon, Sep 7, 2020 at 1:20 AM Petr Mladek <pmladek@suse.com> wrote:
> >
> > - Workaround "unreachable instruction" objtool warnings that happen
> >   with some compiler versions.
> 
> I know I said this fixes things for me, but I just realized it doesn't entirely.
> 
> I wonder how I missed the remaining one:
> 
>    arch/x86/kvm/vmx/vmx.o: warning: objtool:
> vmx_handle_exit_irqoff()+0x142: unreachable instruction
> 
> so apparently gcc and objtool can still disagree even without that
> '-flive-patching'.
> 
> The unreachable code in question is after the call to
> handle_external_interrupt_irqoff(), and while that function is a bit
> odd, in this case I think it's objtool that is wrong.
> 
> I think that what happens is that the function doesn't have a 'ret'
> instruction, and instead returns by doing a tail-call to
> __sanitizer_cov_trace_pc with my config. And maybe that is what
> confuses objtool.
> 
> This is current tip-of-git of my tree, with a allmodconfig build (but
> the actual config will then depend on things like the gcc plugins
> being there too, so you may not get exactly the same thing as I do)
> 
> Josh? Am I missing something, and the objtool warning is valid? But
> yes, that code is doing some very very special stuff with that thunk
> call asm, so it's hard to read the asm.

Hm, I don't think I've seen that one.  We saw a similar warning in that
function before, but it was caused by the combination of
CONFIG_UBSAN_ALIGNMENT and CONFIG_UBSAN_TRAP, which I think Kees fixed.

Can you share the .o file?  At least I can't recreate with GCC 9.3.1,
which is all I have at the moment.

-- 
Josh

Re: [GIT PULL] livepatching for 5.9-rc5

[Linus Torvalds]

On Tue, Sep 8, 2020 at 11:32 AM Josh Poimboeuf <jpoimboe@redhat.com> wrote:
>
> Can you share the .o file?  At least I can't recreate with GCC 9.3.1,
> which is all I have at the moment.

Done off-list in private, because I don't think anybody else wants
object files flying around on the mailing lists..

               Linus

Re: [GIT PULL] livepatching for 5.9-rc5

[Josh Poimboeuf]

On Tue, Sep 08, 2020 at 11:42:00AM -0700, Linus Torvalds wrote:
> On Tue, Sep 8, 2020 at 11:32 AM Josh Poimboeuf <jpoimboe@redhat.com> wrote:
> >
> > Can you share the .o file?  At least I can't recreate with GCC 9.3.1,
> > which is all I have at the moment.
> 
> Done off-list in private, because I don't think anybody else wants
> object files flying around on the mailing lists..

The problem is that objtool ignores handle_external_interrupt_irqoff()
(because it has the STACK_FRAME_NON_STANDARD annotation), and the
'ignore' logic is a bit crude.

Because that function is ignored, the tail call isn't detected (as you
pointed out).  Which confuses the static noreturn detection logic.

The proper fix would be to move that thunk call code to proper asm,
where we can add some unwind hints, and then get rid of the
STACK_FRAME_NON_STANDARD.

But, in the interest of being lazy, here's the easiest fix for now.
I'll need to run some builds to make sure it doesn't break anything.

diff --git a/tools/objtool/check.c b/tools/objtool/check.c
index e034a8f24f46..90a66891441a 100644
--- a/tools/objtool/check.c
+++ b/tools/objtool/check.c
@@ -619,7 +619,7 @@ static int add_jump_destinations(struct objtool_file *file)
 		if (!is_static_jump(insn))
 			continue;
 
-		if (insn->ignore || insn->offset == FAKE_JUMP_OFFSET)
+		if (insn->offset == FAKE_JUMP_OFFSET)
 			continue;
 
 		reloc = find_reloc_by_dest_range(file->elf, insn->sec,