These don't inherently cause crashes, but they're bugs and, if
there's one triggerable from userspace, it can be used to probe
KASLR.
Signed-off-by: Andy Lutomirski <luto@xxxxxxxxxxxxxx>
---
I don't know what the kprobes code is doing, so I made the conservative
change. This can probably be improved.
Kprobes people: what does this code do?
arch/x86/include/asm/uaccess.h | 2 +-
arch/x86/kernel/kprobes/core.c | 4 +++-
arch/x86/kernel/traps.c | 10 +++++++---
arch/x86/mm/extable.c | 7 +++++--
arch/x86/mm/fault.c | 17 ++++++++++-------
5 files changed, 26 insertions(+), 14 deletions(-)
diff --git a/arch/x86/include/asm/uaccess.h b/arch/x86/include/asm/uaccess.h
index ed2d77a..a2a92d8 100644
--- a/arch/x86/include/asm/uaccess.h
+++ b/arch/x86/include/asm/uaccess.h
@@ -101,7 +101,7 @@ struct exception_table_entry {
#define ARCH_HAS_SORT_EXTABLE
#define ARCH_HAS_SEARCH_EXTABLE
-extern int fixup_exception(struct pt_regs *regs);
+extern int fixup_exception(struct pt_regs *regs, bool uaccess_ok);
extern int early_fixup_exception(unsigned long *ip);
/*
diff --git a/arch/x86/kernel/kprobes/core.c b/arch/x86/kernel/kprobes/core.c
index 7bfe318..64932d2 100644
--- a/arch/x86/kernel/kprobes/core.c
+++ b/arch/x86/kernel/kprobes/core.c
@@ -927,8 +927,10 @@ int __kprobes kprobe_fault_handler(struct pt_regs *regs, int trapnr)
/*
* In case the user-specified fault handler returned
* zero, try to fix up.
+ *
+ * XXX: This could be much more conservative.
*/
- if (fixup_exception(regs))
+ if (fixup_exception(regs, true))
return 1;
/*
diff --git a/arch/x86/kernel/traps.c b/arch/x86/kernel/traps.c
index 8647670..a9453b0 100644
--- a/arch/x86/kernel/traps.c
+++ b/arch/x86/kernel/traps.c
@@ -124,7 +124,7 @@ do_trap_no_signal(struct task_struct *tsk, int trapnr, char *str,
}
#endif
if (!user_mode(regs)) {
- if (!fixup_exception(regs)) {
+ if (!fixup_exception(regs, false)) {
tsk->thread.error_code = error_code;
tsk->thread.trap_nr = trapnr;
die(str, regs, error_code);
@@ -277,7 +277,11 @@ do_general_protection(struct pt_regs *regs, long error_code)
if (!user_mode(regs)) {
fixup_pnpbios_exception(regs); /* Might not return */
- if (fixup_exception(regs))
+ /*
+ * This could be a non-canonical address in uaccess. If so,
+ * this is bad.
+ */
+ if (fixup_exception(regs, false))
goto exit;
tsk->thread.error_code = error_code;
@@ -491,7 +495,7 @@ void math_error(struct pt_regs *regs, int error_code, int trapnr)
if (!user_mode_vm(regs))
{
- if (!fixup_exception(regs)) {
+ if (!fixup_exception(regs, false)) {
task->thread.error_code = error_code;
task->thread.trap_nr = trapnr;
die(str, regs, error_code);
diff --git a/arch/x86/mm/extable.c b/arch/x86/mm/extable.c
index 85f45d4..05e078a 100644
--- a/arch/x86/mm/extable.c
+++ b/arch/x86/mm/extable.c
@@ -22,7 +22,7 @@ ex_fixup_addr(const struct exception_table_entry *x)
return (unsigned long)&x->fixup + offset;
}
-int fixup_exception(struct pt_regs *regs)
+int fixup_exception(struct pt_regs *regs, bool uaccess_ok)
{
const struct exception_table_entry *fixup;
unsigned long new_ip;
@@ -33,6 +33,9 @@ int fixup_exception(struct pt_regs *regs)
class = ex_class(fixup);
new_ip = ex_fixup_addr(fixup);
+ if (class != _EXTABLE_CLASS_ANY && !uaccess_ok)
+ return 0;
+
if (class == _EXTABLE_CLASS_EX) {
/* Special hack for uaccess_err */
current_thread_info()->uaccess_err = 1;
@@ -51,7 +54,7 @@ int __init early_fixup_exception(unsigned long *ip)
fixup = search_exception_tables(*ip);
if (fixup) {
- if (ex_class(fixup) == _EXTABLE_CLASS_EX) {
+ if (ex_class(fixup) != _EXTABLE_CLASS_ANY) {
/* uaccess handling not supported during early boot */
return 0;
}
diff --git a/arch/x86/mm/fault.c b/arch/x86/mm/fault.c
index 58afb50..c9fdf7d 100644
--- a/arch/x86/mm/fault.c
+++ b/arch/x86/mm/fault.c
@@ -644,6 +644,11 @@ pgtable_bad(struct pt_regs *regs, unsigned long error_code,
oops_end(flags, regs, sig);
}
+static int fault_in_kernel_space(unsigned long address)
+{
+ return address >= TASK_SIZE_MAX;
+}
+
static noinline void
no_context(struct pt_regs *regs, unsigned long error_code,
unsigned long address, int signal, int si_code)
@@ -655,8 +660,11 @@ no_context(struct pt_regs *regs, unsigned long error_code,
fixup_pnpbios_exception(regs); /* Might not return */
- /* Are we prepared to handle this kernel fault? */
- if (fixup_exception(regs)) {
+ /*
+ * Are we prepared to handle this kernel fault? If this is a
+ * uaccess fault, then the faulting address must be in user space.
+ */
+ if (fixup_exception(regs, !fault_in_kernel_space(address))) {
if (current_thread_info()->sig_on_uaccess_error && signal) {
tsk->thread.trap_nr = X86_TRAP_PF;
tsk->thread.error_code = error_code | PF_USER;
@@ -1001,11 +1009,6 @@ access_error(unsigned long error_code, struct vm_area_struct *vma)
return 0;
}
-static int fault_in_kernel_space(unsigned long address)
-{
- return address >= TASK_SIZE_MAX;
-}
-
static inline bool smap_violation(int error_code, struct pt_regs *regs)
{
if (error_code & PF_USER)
--
1.8.1.4
--
To unsubscribe from this list: send the line "unsubscribe trinity" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
