linux-kernel - Re: [PATCH v4 1/2] x86/unwind: add ORC unwinder

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Tue, 8 Aug 2017 14:13:44 -0500
From:   Josh Poimboeuf <jpoimboe@...hat.com>
To:     Linus Torvalds <torvalds@...ux-foundation.org>
Cc:     "Levin, Alexander (Sasha Levin)" <alexander.levin@...izon.com>,
        "x86@...nel.org" <x86@...nel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "live-patching@...r.kernel.org" <live-patching@...r.kernel.org>,
        Andy Lutomirski <luto@...nel.org>, Jiri Slaby <jslaby@...e.cz>,
        Ingo Molnar <mingo@...nel.org>,
        "H. Peter Anvin" <hpa@...or.com>,
        Peter Zijlstra <peterz@...radead.org>,
        Mike Galbraith <efault@....de>
Subject: Re: [PATCH v4 1/2] x86/unwind: add ORC unwinder

On Tue, Aug 08, 2017 at 12:03:51PM -0700, Linus Torvalds wrote:
> On Tue, Aug 8, 2017 at 11:58 AM, Josh Poimboeuf <jpoimboe@...hat.com> wrote:
> >
> > Take for example the lock_is_held_type() function.  In vmlinux, it has
> > the following instruction:
> >
> >   callq *0xffffffff85a94880 (pv_irq_ops.save_fl)
> >
> > At runtime, that instruction is patched and replaced with a fast inline
> > version of arch_local_save_flags() which eliminates the call:
> >
> >   pushfq
> >   pop %rax
> >
> > The problem is when an interrupt hits after the push:
> >
> >   pushfq
> >   --- irq ---
> >   pop %rax
> 
> That should actually be something easily fixable, for an odd reason:
> the instruction boundaries are different.
> 
> > I'm not sure what the solution should be.  It will probably need to be
> > one of the following:
> >
> >   a) either don't allow runtime "alternative" patches to mess with the
> >      stack pointer (objtool could enforce this); or
> >
> >   b) come up with some way to register such patches with the ORC
> >      unwinder at runtime.
> 
> c) just add ORC data for the alternative statically and _unconditionally_.
> 
> No runtime registration. Just an unconditional entry for the
> particular IP that comes after the "pushfq". It cannot match the
> "callq" instruction, since it would be in the middle of that
> instruction.
> 
> Basically, just do a "union" of the ORC data for all the alternatives.
> 
> Now, objtool should still verify that the instruction pointers for
> alternatives are unique - or that they share the same ORC unwinder
> information if they are not.
> 
> But in cases like this, when the instruction boundaires are different,
> things should "just work", with no need for any special cases.
> 
> Hmm?

Yeah, that might work.  Objtool already knows about alternatives, so it
might not be too hard.  I'll try it.

And it can spit out a warning if we get two different ORC states for the
same address after doing the "union".  Then I guess we'd have to
rearrange things or sprinkle some nops to work around it.

-- 
Josh