linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Ben Hutchings <ben@decadent.org.uk>
To: linux-kernel@vger.kernel.org, stable@vger.kernel.org
Cc: akpm@linux-foundation.org,
	"Konrad Rzeszutek Wilk" <konrad.wilk@oracle.com>,
	"David Woodhouse" <dwmw2@infradead.org>,
	"Peter Zijlstra" <peterz@infradead.org>,
	"Juergen Gross" <jgross@suse.com>,
	"Boris Ostrovsky" <boris.ostrovsky@oracle.com>,
	"Nadav Amit" <namit@vmware.com>,
	"Thomas Gleixner" <tglx@linutronix.de>
Subject: [PATCH 3.16 45/63] x86/paravirt: Fix spectre-v2 mitigations for paravirt guests
Date: Sat, 22 Sep 2018 01:15:42 +0100	[thread overview]
Message-ID: <lsq.1537575342.718020505@decadent.org.uk> (raw)
In-Reply-To: <lsq.1537575341.194909669@decadent.org.uk>

3.16.58-rc1 review patch.  If anyone has any objections, please let me know.

------------------

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>
Signed-off-by: Ben Hutchings <ben@decadent.org.uk>
---
 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
@@ -97,10 +97,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;
@@ -115,8 +117,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;


  parent reply	other threads:[~2018-09-22  0:23 UTC|newest]

Thread overview: 71+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-09-22  0:15 [PATCH 3.16 00/63] 3.16.58-rc1 review Ben Hutchings
2018-09-22  0:15 ` [PATCH 3.16 51/63] xfs: catch inode allocation state mismatch corruption Ben Hutchings
2018-09-22  5:25   ` Dave Chinner
2018-09-22 20:57     ` Ben Hutchings
2018-09-22  0:15 ` [PATCH 3.16 07/63] usbip: usbip_host: refine probe and disconnect debug msgs to be useful Ben Hutchings
2018-09-22  0:15 ` [PATCH 3.16 41/63] USB: yurex: fix out-of-bounds uaccess in read handler Ben Hutchings
2018-09-22  0:15 ` [PATCH 3.16 54/63] seccomp: create internal mode-setting function Ben Hutchings
2018-09-22  0:15 ` Ben Hutchings [this message]
2018-09-22  0:15 ` [PATCH 3.16 36/63] jbd2: don't mark block as modified if the handle is out of credits Ben Hutchings
2018-09-22  0:15 ` [PATCH 3.16 18/63] sr: pass down correctly sized SCSI sense buffer Ben Hutchings
2018-09-22  0:15 ` [PATCH 3.16 06/63] usbip: usbip_host: fix to hold parent lock for device_attach() calls Ben Hutchings
2018-09-22  0:15 ` [PATCH 3.16 29/63] ext4: make sure bitmaps and the inode table don't overlap with bg descriptors Ben Hutchings
2018-09-22  0:15 ` [PATCH 3.16 63/63] mm: get rid of vmacache_flush_all() entirely Ben Hutchings
2018-09-22  0:15 ` [PATCH 3.16 26/63] ext4: verify the depth of extent tree in ext4_find_extent() Ben Hutchings
2018-09-22  0:15 ` [PATCH 3.16 38/63] Fix up non-directory creation in SGID directories Ben Hutchings
2018-09-22  0:15 ` [PATCH 3.16 56/63] seccomp: split mode setting routines Ben Hutchings
2018-09-22  0:15 ` [PATCH 3.16 13/63] futex: Remove unnecessary warning from get_futex_key Ben Hutchings
2018-09-22  0:15 ` [PATCH 3.16 21/63] Bluetooth: hidp: buffer overflow in hidp_process_report Ben Hutchings
2018-09-22  0:15 ` [PATCH 3.16 32/63] ext4: always verify the magic number in xattr blocks Ben Hutchings
2018-09-22  0:15 ` [PATCH 3.16 23/63] xfs: set format back to extents if xfs_bmap_extents_to_btree Ben Hutchings
2018-09-22  0:15 ` [PATCH 3.16 08/63] usbip: usbip_host: delete device from busid_table after rebind Ben Hutchings
2018-09-22  0:15 ` [PATCH 3.16 60/63] x86/cpu/AMD: Fix erratum 1076 (CPB bit) Ben Hutchings
2018-09-22  0:15 ` [PATCH 3.16 57/63] seccomp: add "seccomp" syscall Ben Hutchings
2018-09-22  0:15 ` [PATCH 3.16 20/63] scsi: sg: allocate with __GFP_ZERO in sg_build_indirect() Ben Hutchings
2018-09-22  0:19   ` syzbot
2018-09-22  0:15 ` [PATCH 3.16 04/63] net: Set sk_prot_creator when cloning sockets to the right proto Ben Hutchings
2018-09-22  0:15 ` [PATCH 3.16 33/63] ext4: never move the system.data xattr out of the inode body Ben Hutchings
2018-09-22  0:15 ` [PATCH 3.16 55/63] seccomp: extract check/assign mode helpers Ben Hutchings
2018-09-22  0:15 ` [PATCH 3.16 16/63] KVM: x86: pass kvm_vcpu to kvm_read_guest_virt and kvm_write_guest_virt_system Ben Hutchings
2018-09-22  0:15 ` [PATCH 3.16 35/63] ext4: add more inode number paranoia checks Ben Hutchings
2018-09-22  0:15 ` [PATCH 3.16 42/63] ALSA: rawmidi: Change resized buffers atomically Ben Hutchings
2018-09-22  0:15 ` [PATCH 3.16 27/63] ext4: always check block group bounds in ext4_init_block_bitmap() Ben Hutchings
2018-09-22  0:15 ` [PATCH 3.16 25/63] ext4: fix check to prevent initializing reserved inodes Ben Hutchings
2018-09-22  0:15 ` [PATCH 3.16 14/63] KVM: x86: Emulator ignores LDTR/TR extended base on LLDT/LTR Ben Hutchings
2018-09-22  0:15 ` [PATCH 3.16 15/63] KVM: x86: introduce linear_{read,write}_system Ben Hutchings
2018-09-22  0:15 ` [PATCH 3.16 61/63] x86/cpu/intel: Add Knights Mill to Intel family Ben Hutchings
2018-09-22  0:15 ` [PATCH 3.16 40/63] infiniband: fix a possible use-after-free bug Ben Hutchings
2018-09-22  0:15 ` [PATCH 3.16 09/63] usbip: usbip_host: run rebind from exit when module is removed Ben Hutchings
2018-09-22  0:15 ` [PATCH 3.16 11/63] usbip: usbip_host: fix bad unlock balance during stub_probe() Ben Hutchings
2018-09-22  0:15 ` [PATCH 3.16 12/63] futex: Remove requirement for lock_page() in get_futex_key() Ben Hutchings
2018-09-22  0:15 ` [PATCH 3.16 30/63] ext4: fix false negatives *and* false positives in ext4_check_descriptors() Ben Hutchings
2018-09-22  0:15 ` [PATCH 3.16 37/63] ext4: avoid running out of journal credits when appending to an inline file Ben Hutchings
2018-09-22  0:15 ` [PATCH 3.16 47/63] uas: replace WARN_ON_ONCE() with lockdep_assert_held() Ben Hutchings
2018-09-22  0:15 ` [PATCH 3.16 44/63] x86/speculation: Protect against userspace-userspace spectreRSB Ben Hutchings
2018-09-22  0:15 ` [PATCH 3.16 49/63] btrfs: relocation: Only remove reloc rb_trees if reloc control has been initialized Ben Hutchings
2018-09-22  0:15 ` [PATCH 3.16 24/63] ext4: only look at the bg_flags field if it is valid Ben Hutchings
2018-09-22  0:15 ` [PATCH 3.16 53/63] xfs: don't call xfs_da_shrink_inode with NULL bp Ben Hutchings
2018-09-22  0:15 ` [PATCH 3.16 17/63] kvm: x86: use correct privilege level for sgdt/sidt/fxsave/fxrstor access Ben Hutchings
2018-09-22  0:15 ` [PATCH 3.16 43/63] x86/speculation: Clean up various Spectre related details Ben Hutchings
2018-09-22  0:15 ` [PATCH 3.16 52/63] xfs: validate cached inodes are free when allocated Ben Hutchings
2018-09-22  5:26   ` Dave Chinner
2018-09-22 20:57     ` Ben Hutchings
2018-09-22  0:15 ` [PATCH 3.16 48/63] video: uvesafb: Fix integer overflow in allocation Ben Hutchings
2018-09-22  0:15 ` [PATCH 3.16 19/63] jfs: Fix inconsistency between memory allocation and ea_buf->max_size Ben Hutchings
2018-09-22  0:15 ` [PATCH 3.16 22/63] scsi: libsas: defer ata device eh commands to libata Ben Hutchings
2018-09-22  0:15 ` [PATCH 3.16 05/63] usbip: fix error handling in stub_probe() Ben Hutchings
2018-09-22  0:15 ` [PATCH 3.16 10/63] usbip: usbip_host: fix NULL-ptr deref and use-after-free errors Ben Hutchings
2018-09-22  0:15 ` [PATCH 3.16 46/63] cdrom: Fix info leak/OOB read in cdrom_ioctl_drive_status Ben Hutchings
2018-09-22  0:15 ` [PATCH 3.16 28/63] ext4: don't allow r/w mounts if metadata blocks overlap the superblock Ben Hutchings
2018-09-22  0:15 ` [PATCH 3.16 03/63] Revert "vti4: Don't override MTU passed on link creation via IFLA_MTU" Ben Hutchings
2018-09-22  0:15 ` [PATCH 3.16 34/63] ext4: clear i_data in ext4_inode_info when removing inline data Ben Hutchings
2018-09-22  0:15 ` [PATCH 3.16 01/63] x86/fpu: Fix the 'nofxsr' boot parameter to also clear X86_FEATURE_FXSR_OPT Ben Hutchings
2018-09-22  0:15 ` [PATCH 3.16 02/63] x86/fpu: Default eagerfpu if FPU and FXSR are enabled Ben Hutchings
2018-09-22  0:15 ` [PATCH 3.16 58/63] x86/process: Optimize TIF checks in __switch_to_xtra() Ben Hutchings
2018-09-22  0:15 ` [PATCH 3.16 31/63] ext4: add corruption check in ext4_xattr_set_entry() Ben Hutchings
2018-09-22  0:15 ` [PATCH 3.16 39/63] x86/entry/64: Remove %ebx handling from error_entry/exit Ben Hutchings
2018-09-22  0:15 ` [PATCH 3.16 59/63] x86/process: Correct and optimize TIF_BLOCKSTEP switch Ben Hutchings
2018-09-22  0:15 ` [PATCH 3.16 50/63] hfsplus: fix NULL dereference in hfsplus_lookup() Ben Hutchings
2018-09-22  0:15 ` [PATCH 3.16 62/63] KVM: x86: introduce num_emulated_msrs Ben Hutchings
2018-09-22 12:28 ` [PATCH 3.16 00/63] 3.16.58-rc1 review Guenter Roeck
2018-09-22 21:03   ` Ben Hutchings

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=lsq.1537575342.718020505@decadent.org.uk \
    --to=ben@decadent.org.uk \
    --cc=akpm@linux-foundation.org \
    --cc=boris.ostrovsky@oracle.com \
    --cc=dwmw2@infradead.org \
    --cc=jgross@suse.com \
    --cc=konrad.wilk@oracle.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=namit@vmware.com \
    --cc=peterz@infradead.org \
    --cc=stable@vger.kernel.org \
    --cc=tglx@linutronix.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).