linux-kernel - Re: [patch 3/3] x86/paravirt: Replace paravirt patch asm magic

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]

Message-ID: <20190425080810.GY11158@hirez.programming.kicks-ass.net>
Date:   Thu, 25 Apr 2019 10:08:10 +0200
From:   Peter Zijlstra <peterz@...radead.org>
To:     Ingo Molnar <mingo@...nel.org>
Cc:     Thomas Gleixner <tglx@...utronix.de>,
        LKML <linux-kernel@...r.kernel.org>, x86@...nel.org,
        Juergen Gross <jgross@...e.com>,
        Andi Kleen <ak@...ux.intel.com>
Subject: Re: [patch 3/3] x86/paravirt: Replace paravirt patch asm magic

On Thu, Apr 25, 2019 at 08:52:09AM +0200, Ingo Molnar wrote:

> > -# ifdef CONFIG_PARAVIRT_XXL
> > -DEF_NATIVE(irq, irq_disable, "cli");
> > -DEF_NATIVE(irq, irq_enable, "sti");
> > -DEF_NATIVE(irq, restore_fl, "push %eax; popf");
> > -DEF_NATIVE(irq, save_fl, "pushf; pop %eax");
> > -DEF_NATIVE(cpu, iret, "iret");
> > -DEF_NATIVE(mmu, read_cr2, "mov %cr2, %eax");
> > -DEF_NATIVE(mmu, write_cr3, "mov %eax, %cr3");
> > -DEF_NATIVE(mmu, read_cr3, "mov %cr3, %eax");
> 
> > +static const struct patch_xxl patch_data_xxl = {
> > +	.irq_irq_disable	= { 0xfa },		// cli
> > +	.irq_irq_enable		= { 0xfb },		// sti
> > +	.irq_save_fl		= { 0x9c, 0x58 },	// pushf; pop %[re]ax
> > +	.mmu_read_cr2		= { 0x0f, 0x20, 0xd0 },	// mov %cr2, %[re]ax
> > +	.mmu_read_cr3		= { 0x0f, 0x20, 0xd8 },	// mov %cr3, %[re]ax
> > +# ifdef CONFIG_X86_64
> > +	.irq_restore_fl		= { 0x57, 0x9d },	// push %rdi; popfq
> > +	.mmu_write_cr3		= { 0x0f, 0x22, 0xdf },	// mov %rdi, %cr3
> > +	.cpu_wbinvd		= { 0x0f, 0x09 },	// wbinvd
> > +	.cpu_usergs_sysret64	= { 0x0f, 0x01, 0xf8,
> > +				    0x48, 0x0f, 0x07 },	// swapgs; sysretq
> > +	.cpu_swapgs		= { 0x0f, 0x01, 0xf8 },	// swapgs
> > +	.mov64			= { 0x48, 0x89, 0xf8 },	// mov %rdi, %rax
> > +# else
> > +	.irq_restore_fl		= { 0x50, 0x9d },	// push %eax; popf
> > +	.mmu_write_cr3		= { 0x0f, 0x22, 0xd8 },	// mov %eax, %cr3
> > +	.cpu_iret		= { 0xcf },		// iret
> > +# endif
> 
> I think these open-coded hexa versions are somewhat fragile as well, how 
> about putting these into a .S file and controlling the sections in an LTO 
> safe manner there?
> 
> That will also allow us to write proper asm, and global labels can be 
> used to extract the patchlets and their length?

While I'm not fan either; I think that will be worse still, because it
splits the information over multiple files.

The advantage of this form is that it is clear how long the instructions
are, which is important for the patching. These immediates have to be
shorter than 5 bytes because they overwrite the CALL/JMP to the paravirt
function.

/me eyes .cpu_usergs_sysret64 and goes wtf..