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:   Wed, 9 Aug 2017 15:15:06 -0500
From:   Josh Poimboeuf <jpoimboe@...hat.com>
To:     Juergen Gross <jgross@...e.com>
Cc:     Peter Zijlstra <peterz@...radead.org>,
        Andy Lutomirski <luto@...nel.org>,
        Linus Torvalds <torvalds@...ux-foundation.org>,
        "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>,
        Jiri Slaby <jslaby@...e.cz>, Ingo Molnar <mingo@...nel.org>,
        "H. Peter Anvin" <hpa@...or.com>, Mike Galbraith <efault@....de>
Subject: Re: [PATCH v4 1/2] x86/unwind: add ORC unwinder

On Wed, Aug 09, 2017 at 11:55:35AM +0200, Juergen Gross wrote:
> On 09/08/17 11:35, Peter Zijlstra wrote:
> > On Wed, Aug 09, 2017 at 11:24:07AM +0200, Juergen Gross wrote:
> >> On 09/08/17 11:16, Peter Zijlstra wrote:
> >>> On Wed, Aug 09, 2017 at 10:49:43AM +0200, Juergen Gross wrote:
> >>>>> ALTERNATIVE "pushfq; popq %rax", "callq *pv_irq_ops.save_fl",
> >>>>> X86_FEATURE_GODDAMN_PV_IRQ_OPS
> >>>>
> >>>> You are aware that at least some of the Xen irq pvops functionality is
> >>>> patched inline? Your modification would slow down pv guests quite a
> >>>> bit, I guess.
> >>>
> >>> Where does that live? I know of the inline patching for native, but
> >>> didn't know the guests did any of that too.
> >>
> >> See arch/x86/xen/enlighten_pv.c xen_patch().
> > 
> > 'obvious' name that :-) I see that the actual code that's patched in
> > lives in xen-asm.S which unlike the native case doesn't appear to have
> > its own section. So that might make things even more difficult.
> 
> I don't see why this couldn't be changed.

I'm wondering why xen_patch() even exists.  The main difference between
xen_patch() and native_patch() seems to be that xen_patch() does some
relocs when doing an inline patch after calling paravirt_patch_insns().

But I can't see how that code path would ever run, because the
replacement functions are all larger than the size of the call
instruction to be replaced (7 bytes).  So they would never fit, and
instead the paravirt_patch_default() case would always run.  Or am I
missing something?

If we could get rid of the hypervisor-specific patching functions
(pv_init_ops) -- including possibly removing the lguest and vsmp code,
if nobody cares about them anymore -- that might make it easier to
consolidate all the patching things into a single place.

For example, maybe the paravirt patching could just use alternatives
under the hood somehow (insert hand waving).

-- 
Josh