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: <8138A1EE-359D-4CD2-8E96-5BF00313AB3B@vmware.com>
Date:   Thu, 10 Jan 2019 09:32:23 +0000
From:   Nadav Amit <namit@...are.com>
To:     Josh Poimboeuf <jpoimboe@...hat.com>
CC:     X86 ML <x86@...nel.org>, LKML <linux-kernel@...r.kernel.org>,
        Ard Biesheuvel <ard.biesheuvel@...aro.org>,
        Andy Lutomirski <luto@...nel.org>,
        Steven Rostedt <rostedt@...dmis.org>,
        Peter Zijlstra <peterz@...radead.org>,
        Ingo Molnar <mingo@...nel.org>,
        Thomas Gleixner <tglx@...utronix.de>,
        Linus Torvalds <torvalds@...ux-foundation.org>,
        Masami Hiramatsu <mhiramat@...nel.org>,
        Jason Baron <jbaron@...mai.com>, Jiri Kosina <jkosina@...e.cz>,
        David Laight <David.Laight@...LAB.COM>,
        Borislav Petkov <bp@...en8.de>,
        Julia Cartwright <julia@...com>, Jessica Yu <jeyu@...nel.org>,
        "H. Peter Anvin" <hpa@...or.com>,
        Rasmus Villemoes <linux@...musvillemoes.dk>,
        Edward Cree <ecree@...arflare.com>,
        Daniel Bristot de Oliveira <bristot@...hat.com>
Subject: Re: [PATCH v3 5/6] x86/alternative: Use a single access in
 text_poke() where possible

> On Jan 9, 2019, at 2:59 PM, Josh Poimboeuf <jpoimboe@...hat.com> wrote:
> 
> Static call inline patching will need to use single 32-bit writes.
> Change text_poke() to do so where possible.
> 
> Signed-off-by: Josh Poimboeuf <jpoimboe@...hat.com>
> ---
> arch/x86/kernel/alternative.c | 31 ++++++++++++++++++++++++++++---
> 1 file changed, 28 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/x86/kernel/alternative.c b/arch/x86/kernel/alternative.c
> index ebeac487a20c..607f48a90097 100644
> --- a/arch/x86/kernel/alternative.c
> +++ b/arch/x86/kernel/alternative.c
> @@ -692,7 +692,7 @@ void *__init_or_module text_poke_early(void *addr, const void *opcode,
> void *text_poke(void *addr, const void *opcode, size_t len)
> {
> 	unsigned long flags;
> -	char *vaddr;
> +	unsigned long vaddr;
> 	struct page *pages[2];
> 	int i;
> 
> @@ -714,14 +714,39 @@ void *text_poke(void *addr, const void *opcode, size_t len)
> 	}
> 	BUG_ON(!pages[0]);
> 	local_irq_save(flags);
> +
> 	set_fixmap(FIX_TEXT_POKE0, page_to_phys(pages[0]));
> 	if (pages[1])
> 		set_fixmap(FIX_TEXT_POKE1, page_to_phys(pages[1]));
> -	vaddr = (char *)fix_to_virt(FIX_TEXT_POKE0);
> -	memcpy(&vaddr[(unsigned long)addr & ~PAGE_MASK], opcode, len);
> +
> +	vaddr = fix_to_virt(FIX_TEXT_POKE0) + ((unsigned long)addr & ~PAGE_MASK);
> +
> +	/*
> +	 * Use a single access where possible.  Note that a single unaligned
> +	 * multi-byte write will not necessarily be atomic on x86-32, or if the
> +	 * address crosses a cache line boundary.
> +	 */
> +	switch (len) {
> +	case 1:
> +		WRITE_ONCE(*(u8 *)vaddr, *(u8 *)opcode);
> +		break;
> +	case 2:
> +		WRITE_ONCE(*(u16 *)vaddr, *(u16 *)opcode);
> +		break;
> +	case 4:
> +		WRITE_ONCE(*(u32 *)vaddr, *(u32 *)opcode);
> +		break;
> +	case 8:
> +		WRITE_ONCE(*(u64 *)vaddr, *(u64 *)opcode);
> +		break;
> +	default:
> +		memcpy((void *)vaddr, opcode, len);
> +	}
> +

Even if Intel and AMD CPUs are guaranteed to run instructions from L1
atomically, this may break instruction emulators, such as those that
hypervisors use. They might not read instructions atomically if on SMP VMs
when the VM's text_poke() races with the emulated instruction fetch.

While I can't find a reason for hypervisors to emulate this instruction,
smarter people might find ways to turn it into a security exploit.

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ