added 1 changeset to branch 'refs/remotes/xyzzy-github/x86_64' old head: b77772725ccf08904ed1ed8e0e28500868093ef4 new head: d93ed095640345495ace3b653ea87b66815c7c81 ---------------------------------------------------------------------------- d93ed09: Improved safety for user memory accesses. * Changed IS_USER_ADDRESS to check an address using USER_BASE and USER_SIZE, rather than just !IS_KERNEL_ADDRESS. The old check would allow user buffers to point into the physical memory map area. * Added an unmapped hole at the end of the bottom half of the address space which catches buffers that cross into the uncanonical address region. This also removes the need to check for uncanonical return addresses in the syscall handler, it is no longer possible for the return address to be uncanonical under normal circumstances. All cases in which the return address might be changed by the kernel are still handled via the IRET path. [ Alex Smith <alex@xxxxxxxxxxxxxxxx> ] ---------------------------------------------------------------------------- Commit: d93ed095640345495ace3b653ea87b66815c7c81 Author: Alex Smith <alex@xxxxxxxxxxxxxxxx> Date: Thu Aug 2 08:32:33 2012 UTC ---------------------------------------------------------------------------- 7 files changed, 25 insertions(+), 28 deletions(-) headers/private/kernel/arch/arm/arch_kernel.h | 4 ++-- headers/private/kernel/arch/m68k/arch_kernel.h | 4 ++-- headers/private/kernel/arch/mipsel/arch_kernel.h | 4 ++-- headers/private/kernel/arch/ppc/arch_kernel.h | 4 ++-- headers/private/kernel/arch/x86/arch_kernel.h | 15 +++++++++------ headers/private/kernel/kernel.h | 9 ++++++++- src/system/kernel/arch/x86/64/interrupts.S | 13 ------------- ---------------------------------------------------------------------------- diff --git a/headers/private/kernel/arch/arm/arch_kernel.h b/headers/private/kernel/arch/arm/arch_kernel.h index af0a9e2..44a05e7 100644 --- a/headers/private/kernel/arch/arm/arch_kernel.h +++ b/headers/private/kernel/arch/arm/arch_kernel.h @@ -23,10 +23,10 @@ #define USER_BASE 0x100000 #define USER_BASE_ANY USER_BASE #define USER_SIZE (0x80000000 - (0x10000 + 0x100000)) -#define USER_TOP (USER_BASE + USER_SIZE) +#define USER_TOP (USER_BASE + (USER_SIZE - 1)) #define KERNEL_USER_DATA_BASE 0x6fff0000 #define USER_STACK_REGION 0x70000000 -#define USER_STACK_REGION_SIZE (USER_TOP - USER_STACK_REGION) +#define USER_STACK_REGION_SIZE ((USER_TOP - USER_STACK_REGION) + 1) #endif /* _KERNEL_ARCH_ARM_KERNEL_H */ diff --git a/headers/private/kernel/arch/m68k/arch_kernel.h b/headers/private/kernel/arch/m68k/arch_kernel.h index 85e9168..7f98069 100644 --- a/headers/private/kernel/arch/m68k/arch_kernel.h +++ b/headers/private/kernel/arch/m68k/arch_kernel.h @@ -23,10 +23,10 @@ #define USER_BASE 0x100000 #define USER_BASE_ANY USER_BASE #define USER_SIZE (0x80000000 - (0x10000 + 0x100000)) -#define USER_TOP (USER_BASE + USER_SIZE) +#define USER_TOP (USER_BASE + (USER_SIZE - 1)) #define KERNEL_USER_DATA_BASE 0x6fff0000 #define USER_STACK_REGION 0x70000000 -#define USER_STACK_REGION_SIZE (USER_TOP - USER_STACK_REGION) +#define USER_STACK_REGION_SIZE ((USER_TOP - USER_STACK_REGION) + 1) #endif /* _KERNEL_ARCH_M68K_KERNEL_H */ diff --git a/headers/private/kernel/arch/mipsel/arch_kernel.h b/headers/private/kernel/arch/mipsel/arch_kernel.h index 6eea165..42f3c8f 100644 --- a/headers/private/kernel/arch/mipsel/arch_kernel.h +++ b/headers/private/kernel/arch/mipsel/arch_kernel.h @@ -26,11 +26,11 @@ #define USER_BASE 0x100000 #define USER_BASE_ANY USER_BASE #define USER_SIZE (0x80000000 - (0x10000 + 0x100000)) -#define USER_TOP (USER_BASE + USER_SIZE) +#define USER_TOP (USER_BASE + (USER_SIZE - 1)) #define KERNEL_USER_DATA_BASE 0x6fff0000 #define USER_STACK_REGION 0x70000000 -#define USER_STACK_REGION_SIZE (USER_TOP - USER_STACK_REGION) +#define USER_STACK_REGION_SIZE ((USER_TOP - USER_STACK_REGION) + 1) #endif /* _KERNEL_ARCH_MIPSEL_KERNEL_H */ diff --git a/headers/private/kernel/arch/ppc/arch_kernel.h b/headers/private/kernel/arch/ppc/arch_kernel.h index 3886034..c7d4480 100644 --- a/headers/private/kernel/arch/ppc/arch_kernel.h +++ b/headers/private/kernel/arch/ppc/arch_kernel.h @@ -23,10 +23,10 @@ #define USER_BASE 0x100000 #define USER_BASE_ANY USER_BASE #define USER_SIZE (0x80000000 - (0x10000 + 0x100000)) -#define USER_TOP (USER_BASE + USER_SIZE) +#define USER_TOP (USER_BASE + (USER_SIZE - 1)) #define KERNEL_USER_DATA_BASE 0x6fff0000 #define USER_STACK_REGION 0x70000000 -#define USER_STACK_REGION_SIZE (USER_TOP - USER_STACK_REGION) +#define USER_STACK_REGION_SIZE ((USER_TOP - USER_STACK_REGION) + 1) #endif /* _KERNEL_ARCH_PPC_KERNEL_H */ diff --git a/headers/private/kernel/arch/x86/arch_kernel.h b/headers/private/kernel/arch/x86/arch_kernel.h index 87e676b..3668525 100644 --- a/headers/private/kernel/arch/x86/arch_kernel.h +++ b/headers/private/kernel/arch/x86/arch_kernel.h @@ -40,14 +40,17 @@ #define KERNEL_PMAP_SIZE 0x8000000000 // Userspace address space layout. +// There is a 2MB hole just before the end of the bottom half of the address +// space. This means that if userland passes in a buffer that crosses into the +// uncanonical address region, it will be caught through a page fault. #define USER_BASE 0x0 #define USER_BASE_ANY 0x100000 -#define USER_SIZE 0x800000000000 -#define USER_TOP (USER_BASE + USER_SIZE) +#define USER_SIZE (0x800000000000 - 0x200000) +#define USER_TOP (USER_BASE + (USER_SIZE - 1)) #define KERNEL_USER_DATA_BASE 0x7fffefff0000 #define USER_STACK_REGION 0x7ffff0000000 -#define USER_STACK_REGION_SIZE (USER_TOP - USER_STACK_REGION) +#define USER_STACK_REGION_SIZE ((USER_TOP - USER_STACK_REGION) + 1) #else // __x86_64__ @@ -68,14 +71,14 @@ * TODO: introduce the 1Mb lower barrier again - it's only used for vm86 mode, * and this should be moved into the kernel (and address space) completely. */ -#define USER_BASE 0x00 +#define USER_BASE 0x0 #define USER_BASE_ANY 0x100000 #define USER_SIZE (KERNEL_BASE - 0x10000) -#define USER_TOP (USER_BASE + USER_SIZE) +#define USER_TOP (USER_BASE + (USER_SIZE - 1)) #define KERNEL_USER_DATA_BASE 0x6fff0000 #define USER_STACK_REGION 0x70000000 -#define USER_STACK_REGION_SIZE (USER_TOP - USER_STACK_REGION) +#define USER_STACK_REGION_SIZE ((USER_TOP - USER_STACK_REGION) + 1) #endif // __x86_64__ diff --git a/headers/private/kernel/kernel.h b/headers/private/kernel/kernel.h index ec5a166..a5c01e3 100644 --- a/headers/private/kernel/kernel.h +++ b/headers/private/kernel/kernel.h @@ -31,7 +31,14 @@ #endif // Buffers passed in from user-space shouldn't point into the kernel. -#define IS_USER_ADDRESS(x) (!IS_KERNEL_ADDRESS(x)) +#if USER_BASE == 0 +# define IS_USER_ADDRESS(x) ((addr_t)(x) <= USER_TOP) +#elif USER_TOP == __HAIKU_ADDR_MAX +# define IS_USER_ADDRESS(x) ((addr_t)(x) >= USER_BASE) +#else +# define IS_USER_ADDRESS(x) \ + ((addr_t)(x) >= USER_BASE && (addr_t)(x) <= USER_TOP) +#endif #define DEBUG_KERNEL_STACKS // Note, debugging kernel stacks doesn't really work yet. Since the diff --git a/src/system/kernel/arch/x86/64/interrupts.S b/src/system/kernel/arch/x86/64/interrupts.S index acc82a0..a74b7b4 100644 --- a/src/system/kernel/arch/x86/64/interrupts.S +++ b/src/system/kernel/arch/x86/64/interrupts.S @@ -393,19 +393,6 @@ FUNCTION(x86_64_syscall_entry): cmpq $SYSCALL_RESTORE_SIGNAL_FRAME, %r14 je .Liret - // If the return address is not canonical, return using the IRET path. - // On Intel's implementation of SYSRET, the canonical address check for the - // return address is performed before the switch to user mode, so a fault - // for a non-canonical address will be triggered in kernel mode. However, by - // then we will have switched onto the user stack meaning the kernel could - // be tricked into executing with the user RSP. Using IRET will handle - // non-canonical addresses properly. - movq IFRAME_user_sp(%rsp), %rax - sarq $47, %rax - incl %eax - cmpl $1, %eax - ja .Liret - // Restore the iframe and RCX/R11 for SYSRET. RESTORE_IFRAME() pop %rcx