[Xen-devel] [PATCH] x86: correct create_bounce_frame

["Jan Beulich" <JBeulich@xxxxxxxx>]

Commit d9b7ef209a7 ("x86: drop failsafe callback invocation from
assembly") didn't go quite far enough with the cleanup it did: The
changed maximum frame size should also have been reflected in the early
address range check (which has now been pointed out to have been wrong
anyway, using 60 instead of 0x60), and it should have updated the
comment ahead of the function.

Also adjust the lower bound - all is fine (for our purposes) if the
initial guest kernel stack pointer points right at the hypervisor base
address, as only memory _below_ that address is going to be written.

Additionally limit the number of times %rsi is being adjusted to what
is really needed.

Finally move exception fixup code into the designated .fixup section.

Reported-by: Jann Horn <jannh@xxxxxxxxxx>
Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
---
This corrects the code which did result in XSA-215 on Xen 4.6 and
older. For that reason I at least want to explore whether this is a
change we want to take for 4.9.

--- a/xen/arch/x86/x86_64/entry.S
+++ b/xen/arch/x86/x86_64/entry.S
@@ -258,7 +258,7 @@ int80_slow_path:
         jmp   handle_exception_saved
 
 /* CREATE A BASIC EXCEPTION FRAME ON GUEST OS STACK:                     */
-/*   { RCX, R11, [DS-GS,] [CR2,] [ERRCODE,] RIP, CS, RFLAGS, RSP, SS }   */
+/*   { RCX, R11, [ERRCODE,] RIP, CS, RFLAGS, RSP, SS }                   */
 /* %rdx: trap_bounce, %rbx: struct vcpu                                  */
 /* On return only %rbx and %rdx are guaranteed non-clobbered.            */
 create_bounce_frame:
@@ -276,9 +276,9 @@ create_bounce_frame:
         movq  UREGS_rsp+8(%rsp),%rsi
         andb  $0xfc,UREGS_cs+8(%rsp)    # Indicate kernel context to guest.
 2:      andq  $~0xf,%rsi                # Stack frames are 16-byte aligned.
-        movq  $HYPERVISOR_VIRT_START,%rax
+        movq  $HYPERVISOR_VIRT_START+1,%rax
         cmpq  %rax,%rsi
-        movq  $HYPERVISOR_VIRT_END+60,%rax
+        movq  $HYPERVISOR_VIRT_END+8*8,%rax
         sbb   %ecx,%ecx                 # In +ve address space? Then okay.
         cmpq  %rax,%rsi
         adc   %ecx,%ecx                 # Above Xen private area? Then okay.
@@ -286,13 +286,13 @@ UNLIKELY_START(g, create_bounce_frame_ba
         lea   UNLIKELY_DISPATCH_LABEL(create_bounce_frame_bad_sp)(%rip), %rdi
         jmp   asm_domain_crash_synchronous  /* Does not return */
 __UNLIKELY_END(create_bounce_frame_bad_sp)
-        subq  $40,%rsi
+        subq  $7*8,%rsi
         movq  UREGS_ss+8(%rsp),%rax
         ASM_STAC
         movq  VCPU_domain(%rbx),%rdi
-.Lft2:  movq  %rax,32(%rsi)             # SS
+.Lft2:  movq  %rax,6*8(%rsi)            # SS
         movq  UREGS_rsp+8(%rsp),%rax
-.Lft3:  movq  %rax,24(%rsi)             # RSP
+.Lft3:  movq  %rax,5*8(%rsi)            # RSP
         movq  VCPU_vcpu_info(%rbx),%rax
         pushq VCPUINFO_upcall_mask(%rax)
         testb $TBF_INTERRUPT,TRAPBOUNCE_flags(%rdx)
@@ -301,7 +301,7 @@ __UNLIKELY_END(create_bounce_frame_bad_s
         popq  %rax
         shlq  $32,%rax                  # Bits 32-39: saved_upcall_mask
         movw  UREGS_cs+8(%rsp),%ax      # Bits  0-15: CS
-.Lft4:  movq  %rax,8(%rsi)              # CS / saved_upcall_mask
+.Lft4:  movq  %rax,3*8(%rsi)            # CS / saved_upcall_mask
         shrq  $32,%rax
         testb $0xFF,%al                 # Bits 0-7: saved_upcall_mask
         setz  %ch                       # %ch == !saved_upcall_mask
@@ -313,16 +313,15 @@ __UNLIKELY_END(create_bounce_frame_bad_s
         testb $1 << VMASST_TYPE_architectural_iopl,DOMAIN_vm_assist(%rdi)
         cmovnzl VCPU_iopl(%rbx),%ecx    # Bits 13:12 (EFLAGS.IOPL)
         orl   %ecx,%eax                 # Fold EFLAGS.IOPL into %eax
-.Lft5:  movq  %rax,16(%rsi)             # RFLAGS
+.Lft5:  movq  %rax,4*8(%rsi)            # RFLAGS
         movq  UREGS_rip+8(%rsp),%rax
-.Lft6:  movq  %rax,(%rsi)               # RIP
+.Lft6:  movq  %rax,2*8(%rsi)            # RIP
         testb $TBF_EXCEPTION_ERRCODE,TRAPBOUNCE_flags(%rdx)
         jz    1f
         subq  $8,%rsi
         movl  TRAPBOUNCE_error_code(%rdx),%eax
-.Lft7:  movq  %rax,(%rsi)               # ERROR CODE
+.Lft7:  movq  %rax,2*8(%rsi)            # ERROR CODE
 1:
-        subq  $16,%rsi
         movq  UREGS_r11+8(%rsp),%rax
 .Lft12: movq  %rax,8(%rsi)              # R11
         movq  UREGS_rcx+8(%rsp),%rax
@@ -345,15 +344,20 @@ UNLIKELY_START(z, create_bounce_frame_ba
 __UNLIKELY_END(create_bounce_frame_bad_bounce_ip)
         movq  %rax,UREGS_rip+8(%rsp)
         ret
-        _ASM_EXTABLE(.Lft2,  domain_crash_page_fault_32)
-        _ASM_EXTABLE(.Lft3,  domain_crash_page_fault_24)
-        _ASM_EXTABLE(.Lft4,  domain_crash_page_fault_8)
-        _ASM_EXTABLE(.Lft5,  domain_crash_page_fault_16)
-        _ASM_EXTABLE(.Lft6,  domain_crash_page_fault)
-        _ASM_EXTABLE(.Lft7,  domain_crash_page_fault)
+        _ASM_EXTABLE(.Lft2,  domain_crash_page_fault_48)
+        _ASM_EXTABLE(.Lft3,  domain_crash_page_fault_40)
+        _ASM_EXTABLE(.Lft4,  domain_crash_page_fault_24)
+        _ASM_EXTABLE(.Lft5,  domain_crash_page_fault_32)
+        _ASM_EXTABLE(.Lft6,  domain_crash_page_fault_16)
+        _ASM_EXTABLE(.Lft7,  domain_crash_page_fault_16)
         _ASM_EXTABLE(.Lft12, domain_crash_page_fault_8)
         _ASM_EXTABLE(.Lft13, domain_crash_page_fault)
 
+        .pushsection .fixup, "ax", @progbits
+domain_crash_page_fault_48:
+        addq  $8,%rsi
+domain_crash_page_fault_40:
+        addq  $8,%rsi
 domain_crash_page_fault_32:
         addq  $8,%rsi
 domain_crash_page_fault_24:
@@ -380,6 +384,7 @@ ENTRY(dom_crash_sync_extable)
         orb   %al,UREGS_cs(%rsp)
         xorl  %edi,%edi
         jmp   asm_domain_crash_synchronous /* Does not return */
+        .popsection
 
 ENTRY(common_interrupt)
         SAVE_ALL CLAC

Attachment: x86-bounce-frame-size.patch
Description: Text document

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel