x86: correct create_bounce_frame 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 Signed-off-by: Jan Beulich --- 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. --- v2: Change domain_crash_page_fault_* tags and add comment ahead of the labels. Convert 8(%rsi) to 1*8(%rsi) and (%rsi) to 0*8(%rsi). --- 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,20 +313,19 @@ __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 +.Lft12: movq %rax,1*8(%rsi) # R11 movq UREGS_rcx+8(%rsp),%rax -.Lft13: movq %rax,(%rsi) # RCX +.Lft13: movq %rax,0*8(%rsi) # RCX ASM_CLAC /* Rewrite our stack frame and return to guest-OS mode. */ /* IA32 Ref. Vol. 3: TF, VM, RF and NT flags are cleared on trap. */ @@ -345,24 +344,30 @@ 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(.Lft12, domain_crash_page_fault_8) - _ASM_EXTABLE(.Lft13, domain_crash_page_fault) + _ASM_EXTABLE(.Lft2, domain_crash_page_fault_6x8) + _ASM_EXTABLE(.Lft3, domain_crash_page_fault_5x8) + _ASM_EXTABLE(.Lft4, domain_crash_page_fault_4x8) + _ASM_EXTABLE(.Lft5, domain_crash_page_fault_3x8) + _ASM_EXTABLE(.Lft6, domain_crash_page_fault_2x8) + _ASM_EXTABLE(.Lft7, domain_crash_page_fault_2x8) + _ASM_EXTABLE(.Lft12, domain_crash_page_fault_1x8) + _ASM_EXTABLE(.Lft13, domain_crash_page_fault_0x8) -domain_crash_page_fault_32: + .pushsection .fixup, "ax", @progbits + # Numeric tags below represent the intended overall %rsi adjustment. +domain_crash_page_fault_6x8: addq $8,%rsi -domain_crash_page_fault_24: +domain_crash_page_fault_5x8: addq $8,%rsi -domain_crash_page_fault_16: +domain_crash_page_fault_4x8: addq $8,%rsi -domain_crash_page_fault_8: +domain_crash_page_fault_3x8: addq $8,%rsi -domain_crash_page_fault: +domain_crash_page_fault_2x8: + addq $8,%rsi +domain_crash_page_fault_1x8: + addq $8,%rsi +domain_crash_page_fault_0x8: ASM_CLAC movq %rsi,%rdi call show_page_walk @@ -380,6 +385,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