| From 5800dc5c19f34e6e03b5adab1282535cb102fafd Mon Sep 17 00:00:00 2001 |
| From: Peter Zijlstra <peterz@infradead.org> |
| Date: Fri, 3 Aug 2018 16:41:39 +0200 |
| Subject: x86/paravirt: Fix spectre-v2 mitigations for paravirt guests |
| |
| From: Peter Zijlstra <peterz@infradead.org> |
| |
| commit 5800dc5c19f34e6e03b5adab1282535cb102fafd upstream. |
| |
| Nadav reported that on guests we're failing to rewrite the indirect |
| calls to CALLEE_SAVE paravirt functions. In particular the |
| pv_queued_spin_unlock() call is left unpatched and that is all over the |
| place. This obviously wrecks Spectre-v2 mitigation (for paravirt |
| guests) which relies on not actually having indirect calls around. |
| |
| The reason is an incorrect clobber test in paravirt_patch_call(); this |
| function rewrites an indirect call with a direct call to the _SAME_ |
| function, there is no possible way the clobbers can be different |
| because of this. |
| |
| Therefore remove this clobber check. Also put WARNs on the other patch |
| failure case (not enough room for the instruction) which I've not seen |
| trigger in my (limited) testing. |
| |
| Three live kernel image disassemblies for lock_sock_nested (as a small |
| function that illustrates the problem nicely). PRE is the current |
| situation for guests, POST is with this patch applied and NATIVE is with |
| or without the patch for !guests. |
| |
| PRE: |
| |
| (gdb) disassemble lock_sock_nested |
| Dump of assembler code for function lock_sock_nested: |
| 0xffffffff817be970 <+0>: push %rbp |
| 0xffffffff817be971 <+1>: mov %rdi,%rbp |
| 0xffffffff817be974 <+4>: push %rbx |
| 0xffffffff817be975 <+5>: lea 0x88(%rbp),%rbx |
| 0xffffffff817be97c <+12>: callq 0xffffffff819f7160 <_cond_resched> |
| 0xffffffff817be981 <+17>: mov %rbx,%rdi |
| 0xffffffff817be984 <+20>: callq 0xffffffff819fbb00 <_raw_spin_lock_bh> |
| 0xffffffff817be989 <+25>: mov 0x8c(%rbp),%eax |
| 0xffffffff817be98f <+31>: test %eax,%eax |
| 0xffffffff817be991 <+33>: jne 0xffffffff817be9ba <lock_sock_nested+74> |
| 0xffffffff817be993 <+35>: movl $0x1,0x8c(%rbp) |
| 0xffffffff817be99d <+45>: mov %rbx,%rdi |
| 0xffffffff817be9a0 <+48>: callq *0xffffffff822299e8 |
| 0xffffffff817be9a7 <+55>: pop %rbx |
| 0xffffffff817be9a8 <+56>: pop %rbp |
| 0xffffffff817be9a9 <+57>: mov $0x200,%esi |
| 0xffffffff817be9ae <+62>: mov $0xffffffff817be993,%rdi |
| 0xffffffff817be9b5 <+69>: jmpq 0xffffffff81063ae0 <__local_bh_enable_ip> |
| 0xffffffff817be9ba <+74>: mov %rbp,%rdi |
| 0xffffffff817be9bd <+77>: callq 0xffffffff817be8c0 <__lock_sock> |
| 0xffffffff817be9c2 <+82>: jmp 0xffffffff817be993 <lock_sock_nested+35> |
| End of assembler dump. |
| |
| POST: |
| |
| (gdb) disassemble lock_sock_nested |
| Dump of assembler code for function lock_sock_nested: |
| 0xffffffff817be970 <+0>: push %rbp |
| 0xffffffff817be971 <+1>: mov %rdi,%rbp |
| 0xffffffff817be974 <+4>: push %rbx |
| 0xffffffff817be975 <+5>: lea 0x88(%rbp),%rbx |
| 0xffffffff817be97c <+12>: callq 0xffffffff819f7160 <_cond_resched> |
| 0xffffffff817be981 <+17>: mov %rbx,%rdi |
| 0xffffffff817be984 <+20>: callq 0xffffffff819fbb00 <_raw_spin_lock_bh> |
| 0xffffffff817be989 <+25>: mov 0x8c(%rbp),%eax |
| 0xffffffff817be98f <+31>: test %eax,%eax |
| 0xffffffff817be991 <+33>: jne 0xffffffff817be9ba <lock_sock_nested+74> |
| 0xffffffff817be993 <+35>: movl $0x1,0x8c(%rbp) |
| 0xffffffff817be99d <+45>: mov %rbx,%rdi |
| 0xffffffff817be9a0 <+48>: callq 0xffffffff810a0c20 <__raw_callee_save___pv_queued_spin_unlock> |
| 0xffffffff817be9a5 <+53>: xchg %ax,%ax |
| 0xffffffff817be9a7 <+55>: pop %rbx |
| 0xffffffff817be9a8 <+56>: pop %rbp |
| 0xffffffff817be9a9 <+57>: mov $0x200,%esi |
| 0xffffffff817be9ae <+62>: mov $0xffffffff817be993,%rdi |
| 0xffffffff817be9b5 <+69>: jmpq 0xffffffff81063aa0 <__local_bh_enable_ip> |
| 0xffffffff817be9ba <+74>: mov %rbp,%rdi |
| 0xffffffff817be9bd <+77>: callq 0xffffffff817be8c0 <__lock_sock> |
| 0xffffffff817be9c2 <+82>: jmp 0xffffffff817be993 <lock_sock_nested+35> |
| End of assembler dump. |
| |
| NATIVE: |
| |
| (gdb) disassemble lock_sock_nested |
| Dump of assembler code for function lock_sock_nested: |
| 0xffffffff817be970 <+0>: push %rbp |
| 0xffffffff817be971 <+1>: mov %rdi,%rbp |
| 0xffffffff817be974 <+4>: push %rbx |
| 0xffffffff817be975 <+5>: lea 0x88(%rbp),%rbx |
| 0xffffffff817be97c <+12>: callq 0xffffffff819f7160 <_cond_resched> |
| 0xffffffff817be981 <+17>: mov %rbx,%rdi |
| 0xffffffff817be984 <+20>: callq 0xffffffff819fbb00 <_raw_spin_lock_bh> |
| 0xffffffff817be989 <+25>: mov 0x8c(%rbp),%eax |
| 0xffffffff817be98f <+31>: test %eax,%eax |
| 0xffffffff817be991 <+33>: jne 0xffffffff817be9ba <lock_sock_nested+74> |
| 0xffffffff817be993 <+35>: movl $0x1,0x8c(%rbp) |
| 0xffffffff817be99d <+45>: mov %rbx,%rdi |
| 0xffffffff817be9a0 <+48>: movb $0x0,(%rdi) |
| 0xffffffff817be9a3 <+51>: nopl 0x0(%rax) |
| 0xffffffff817be9a7 <+55>: pop %rbx |
| 0xffffffff817be9a8 <+56>: pop %rbp |
| 0xffffffff817be9a9 <+57>: mov $0x200,%esi |
| 0xffffffff817be9ae <+62>: mov $0xffffffff817be993,%rdi |
| 0xffffffff817be9b5 <+69>: jmpq 0xffffffff81063ae0 <__local_bh_enable_ip> |
| 0xffffffff817be9ba <+74>: mov %rbp,%rdi |
| 0xffffffff817be9bd <+77>: callq 0xffffffff817be8c0 <__lock_sock> |
| 0xffffffff817be9c2 <+82>: jmp 0xffffffff817be993 <lock_sock_nested+35> |
| End of assembler dump. |
| |
| |
| Fixes: 63f70270ccd9 ("[PATCH] i386: PARAVIRT: add common patching machinery") |
| Fixes: 3010a0663fd9 ("x86/paravirt, objtool: Annotate indirect calls") |
| Reported-by: Nadav Amit <namit@vmware.com> |
| Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org> |
| Signed-off-by: Thomas Gleixner <tglx@linutronix.de> |
| Reviewed-by: Juergen Gross <jgross@suse.com> |
| Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> |
| Cc: Boris Ostrovsky <boris.ostrovsky@oracle.com> |
| Cc: David Woodhouse <dwmw2@infradead.org> |
| Cc: stable@vger.kernel.org |
| Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> |
| |
| --- |
| arch/x86/kernel/paravirt.c | 14 ++++++++++---- |
| 1 file changed, 10 insertions(+), 4 deletions(-) |
| |
| --- a/arch/x86/kernel/paravirt.c |
| +++ b/arch/x86/kernel/paravirt.c |
| @@ -88,10 +88,12 @@ unsigned paravirt_patch_call(void *insnb |
| struct branch *b = insnbuf; |
| unsigned long delta = (unsigned long)target - (addr+5); |
| |
| - if (tgt_clobbers & ~site_clobbers) |
| - return len; /* target would clobber too much for this site */ |
| - if (len < 5) |
| + if (len < 5) { |
| +#ifdef CONFIG_RETPOLINE |
| + WARN_ONCE("Failing to patch indirect CALL in %ps\n", (void *)addr); |
| +#endif |
| return len; /* call too long for patch site */ |
| + } |
| |
| b->opcode = 0xe8; /* call */ |
| b->delta = delta; |
| @@ -106,8 +108,12 @@ unsigned paravirt_patch_jmp(void *insnbu |
| struct branch *b = insnbuf; |
| unsigned long delta = (unsigned long)target - (addr+5); |
| |
| - if (len < 5) |
| + if (len < 5) { |
| +#ifdef CONFIG_RETPOLINE |
| + WARN_ONCE("Failing to patch indirect JMP in %ps\n", (void *)addr); |
| +#endif |
| return len; /* call too long for patch site */ |
| + } |
| |
| b->opcode = 0xe9; /* jmp */ |
| b->delta = delta; |