sysenter: handle bad return addresses

Author: xvanc

src/aero_kernel/src/arch/x86_64/interrupts/exceptions.rs

@@ -69,12 +69,15 @@ pub fn invalid_opcode(stack: &mut InterruptErrorStack) {
     // The RIP on the stack for #UD points to the instruction which generated the exception.
     // The return RIP and RSP need to be changed to the user-provided values in RCX and R11.
     const SYSENTER_OPCODE: [u8; 2] = [0x0f, 0x34];
-    
+
     let opcode = unsafe { *(stack.stack.iret.rip as *const [u8; 2]) };
     if opcode == SYSENTER_OPCODE {
+        log::debug!("handling SYSENTER via #UD");
+
         stack.stack.iret.rip = stack.stack.scratch.rcx;
         stack.stack.iret.rsp = stack.stack.scratch.r11;
 
+        super::super::syscall::x86_64_check_sysenter(stack);
         super::super::syscall::x86_64_do_syscall(stack);
         return;
     }

src/aero_kernel/src/arch/x86_64/syscall.rs

@@ -54,6 +54,24 @@ fn arch_prctl(command: usize, address: usize) -> Result<usize, AeroSyscallError>
     }
 }
 
+/// Check the user-provided return addresses for system calls via SYSENTER
+///
+/// We cannot execute `sysexit` on return with non-canonical return addresses, or we
+/// will take a fault in the kernel with the user's GS base already swapped back.
+#[no_mangle]
+pub(super) extern "sysv64" fn x86_64_check_sysenter(stack: &mut InterruptErrorStack) {
+    let rip = stack.stack.iret.rip;
+    let rsp = stack.stack.iret.rsp;
+    let max_user_addr = super::task::userland_last_address().as_u64();
+
+    if rip > max_user_addr || rsp > max_user_addr {
+        log::error!("bad sysenter: rip={:#018x},rsp={:#018x}", rip, rsp);
+
+        // Forcibly kill the process, we have nowhere to return to.
+        scheduler::get_scheduler().exit(-1);
+    }
+}
+
 #[no_mangle]
 pub(super) extern "C" fn x86_64_do_syscall(stack: &mut InterruptErrorStack) {
     let stack = &mut stack.stack;

src/aero_kernel/src/arch/x86_64/syscall_handler.asm

@@ -20,6 +20,7 @@ bits 64
 %include "registers.inc"
 
 extern x86_64_do_syscall
+extern x86_64_check_sysenter
 global x86_64_syscall_handler
 
 %define TSS_TEMP_USTACK_OFF 0x1c
@@ -123,7 +124,7 @@ x86_64_sysenter_handler:
     push    rcx
 
     ; Mask the same flags as for SYSCALL.
-    ; Note that up to this pont the code can be single-stepped if the user sets TF.
+    ; Note that up to this point the code can be single-stepped if the user sets TF.
     pushfq
     and     dword [rsp], 0x300
     popfq
@@ -133,11 +134,14 @@ x86_64_sysenter_handler:
     push_preserved
     push    0
 
-    ; Sore the stack pointer (interrupt frame pointer) in RBP for save keeping,
+    ; Store the stack pointer (interrupt frame pointer) in RBP for safe keeping,
     ; and align the stack as specified by the SysV calling convention.
     mov     rbp, rsp
     and     rsp, ~0xf
 
+    mov     rdi, rbp
+    call    x86_64_check_sysenter
+
     mov     rdi, rbp
     call    x86_64_do_syscall
 
@@ -146,14 +150,15 @@ x86_64_sysenter_handler:
     pop_preserved
     pop_scratch
 
-    ; Restore RFLAGS
-    add     rsp, 16
-    popfq
-
-    ; Move the return RIP and RSP into the registers expected by SYSEXIT.
-    mov     rdx, rcx
-    mov     rcx, r11
+    ; Pop the IRET frame into the registers expected by SYSEXIT.
+    pop     rdx     ; return RIP in RDX
+    add     rsp, 8
+    popfq           ; restore saved RFLAGS
+    pop     rcx     ; return RSP in RCX
 
+    ; SAFETY: The above call to `x86_64_check_sysenter` is guarantees that we
+    ; execute `sysexit` with canonical addresses in RCX and RDX. Otherwise we would
+    ; fault in the kernel having already swapped back to the user's GS.
     swapgs
     o64 sysexit
 .end:

userland/apps/utest/src/main.rs

@@ -317,6 +317,23 @@ fn rpc_test() -> Result<(), AeroSyscallError> {
 
 #[utest_proc::test]
 fn sysenter_test() -> Result<(), AeroSyscallError> {
+    let pid = sys_fork()?;
+
+    if pid == 0 {
+        unsafe {
+            core::arch::asm! {
+                "sysenter",
+                in("rcx") 0xf0f0usize << 48,
+                in("r11") 0x0f0fusize << 48,
+            }
+        }
+
+        core::unreachable!();
+    } else {
+        let mut status = 0;
+        sys_waitpid(pid, &mut status, 0)?;
+    }
+
     let msg = "sysenter works!\n";
     let ptr = msg.as_ptr();
     let len = msg.len();