|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] [Xen-devel] [PATCH] x86/debug: Make debugger_trap_entry() safe during early boot
debugger_trap_entry() is reachable during early boot where its unconditional
use of current is unsafe. Add a warning to the function to this effect.
Perform the vector check first, as this allows the compiler to elide the other
content from most of its callsites. Check guest_mode(regs) before using
current, which makes the path safe on early boot.
While editing this area, drop DEBUGGER_trap_{entry,fatal}, as hiding a return
statement in a function-like macro is very antisocial programming; show the
real control flow at each of the callsites. Finally, switch
debugger_trap_{entry,fatal} to having boolean return types, to match their
semantics.
No behavioural change.
Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
---
CC: Jan Beulich <JBeulich@xxxxxxxx>
---
xen/arch/x86/traps.c | 46 ++++++++++++++++++++++++++++++------------
xen/include/asm-x86/debugger.h | 30 ++++++++++++++-------------
2 files changed, 49 insertions(+), 27 deletions(-)
diff --git a/xen/arch/x86/traps.c b/xen/arch/x86/traps.c
index 767d0b0..b042976 100644
--- a/xen/arch/x86/traps.c
+++ b/xen/arch/x86/traps.c
@@ -736,7 +736,9 @@ void do_reserved_trap(struct cpu_user_regs *regs)
{
unsigned int trapnr = regs->entry_vector;
- DEBUGGER_trap_fatal(trapnr, regs);
+ if ( debugger_trap_fatal(trapnr, regs) )
+ return;
+
show_execution_state(regs);
panic("FATAL RESERVED TRAP %#x: %s", trapnr, trapstr(trapnr));
}
@@ -750,7 +752,8 @@ static void do_trap(struct cpu_user_regs *regs, int
use_error_code)
if ( regs->error_code & X86_XEC_EXT )
goto hardware_trap;
- DEBUGGER_trap_entry(trapnr, regs);
+ if ( debugger_trap_entry(trapnr, regs) )
+ return;
if ( guest_mode(regs) )
{
@@ -777,7 +780,8 @@ static void do_trap(struct cpu_user_regs *regs, int
use_error_code)
}
hardware_trap:
- DEBUGGER_trap_fatal(trapnr, regs);
+ if ( debugger_trap_fatal(trapnr, regs) )
+ return;
show_execution_state(regs);
panic("FATAL TRAP: vector = %d (%s)\n"
@@ -1307,7 +1311,8 @@ void do_invalid_op(struct cpu_user_regs *regs)
int id = -1, lineno;
const struct virtual_region *region;
- DEBUGGER_trap_entry(TRAP_invalid_op, regs);
+ if ( debugger_trap_entry(TRAP_invalid_op, regs) )
+ return;
if ( likely(guest_mode(regs)) )
{
@@ -1377,7 +1382,10 @@ void do_invalid_op(struct cpu_user_regs *regs)
case BUGFRAME_bug:
printk("Xen BUG at %s%s:%d\n", prefix, filename, lineno);
- DEBUGGER_trap_fatal(TRAP_invalid_op, regs);
+
+ if ( debugger_trap_fatal(TRAP_invalid_op, regs) )
+ return;
+
show_execution_state(regs);
panic("Xen BUG at %s%s:%d", prefix, filename, lineno);
@@ -1389,7 +1397,10 @@ void do_invalid_op(struct cpu_user_regs *regs)
printk("Assertion '%s' failed at %s%s:%d\n",
predicate, prefix, filename, lineno);
- DEBUGGER_trap_fatal(TRAP_invalid_op, regs);
+
+ if ( debugger_trap_fatal(TRAP_invalid_op, regs) )
+ return;
+
show_execution_state(regs);
panic("Assertion '%s' failed at %s%s:%d",
predicate, prefix, filename, lineno);
@@ -1402,14 +1413,18 @@ void do_invalid_op(struct cpu_user_regs *regs)
regs->eip = fixup;
return;
}
- DEBUGGER_trap_fatal(TRAP_invalid_op, regs);
+
+ if ( debugger_trap_fatal(TRAP_invalid_op, regs) )
+ return;
+
show_execution_state(regs);
panic("FATAL TRAP: vector = %d (invalid opcode)", TRAP_invalid_op);
}
void do_int3(struct cpu_user_regs *regs)
{
- DEBUGGER_trap_entry(TRAP_int3, regs);
+ if ( debugger_trap_entry(TRAP_int3, regs) )
+ return;
if ( !guest_mode(regs) )
{
@@ -1744,7 +1759,8 @@ void do_page_fault(struct cpu_user_regs *regs)
/* fixup_page_fault() might change regs->error_code, so cache it here. */
error_code = regs->error_code;
- DEBUGGER_trap_entry(TRAP_page_fault, regs);
+ if ( debugger_trap_entry(TRAP_page_fault, regs) )
+ return;
perfc_incr(page_faults);
@@ -1774,7 +1790,8 @@ void do_page_fault(struct cpu_user_regs *regs)
return;
}
- DEBUGGER_trap_fatal(TRAP_page_fault, regs);
+ if ( debugger_trap_fatal(TRAP_page_fault, regs) )
+ return;
show_execution_state(regs);
show_page_walk(addr);
@@ -3471,7 +3488,8 @@ void do_general_protection(struct cpu_user_regs *regs)
struct vcpu *v = current;
unsigned long fixup;
- DEBUGGER_trap_entry(TRAP_gp_fault, regs);
+ if ( debugger_trap_entry(TRAP_gp_fault, regs) )
+ return;
if ( regs->error_code & X86_XEC_EXT )
goto hardware_gp;
@@ -3543,7 +3561,8 @@ void do_general_protection(struct cpu_user_regs *regs)
}
hardware_gp:
- DEBUGGER_trap_fatal(TRAP_gp_fault, regs);
+ if ( debugger_trap_fatal(TRAP_gp_fault, regs) )
+ return;
show_execution_state(regs);
panic("GENERAL PROTECTION FAULT\n[error_code=%04x]", regs->error_code);
@@ -3778,7 +3797,8 @@ void do_debug(struct cpu_user_regs *regs)
{
struct vcpu *v = current;
- DEBUGGER_trap_entry(TRAP_debug, regs);
+ if ( debugger_trap_entry(TRAP_debug, regs) )
+ return;
if ( !guest_mode(regs) )
{
diff --git a/xen/include/asm-x86/debugger.h b/xen/include/asm-x86/debugger.h
index 271cbaa..82e3dc9 100644
--- a/xen/include/asm-x86/debugger.h
+++ b/xen/include/asm-x86/debugger.h
@@ -33,17 +33,11 @@
#include <asm/regs.h>
#include <asm/processor.h>
-/* The main trap handlers use these helper macros which include early bail. */
-#define DEBUGGER_trap_entry(_v, _r) \
- if ( debugger_trap_entry(_v, _r) ) return;
-#define DEBUGGER_trap_fatal(_v, _r) \
- if ( debugger_trap_fatal(_v, _r) ) return;
-
#ifdef CONFIG_CRASH_DEBUG
#include <xen/gdbstub.h>
-static inline int debugger_trap_fatal(
+static inline bool debugger_trap_fatal(
unsigned int vector, struct cpu_user_regs *regs)
{
int rc = __trap_to_gdb(regs, vector);
@@ -55,31 +49,39 @@ static inline int debugger_trap_fatal(
#else
-static inline int debugger_trap_fatal(
+static inline bool debugger_trap_fatal(
unsigned int vector, struct cpu_user_regs *regs)
{
- return 0;
+ return false;
}
#define debugger_trap_immediate() ((void)0)
#endif
-static inline int debugger_trap_entry(
+static inline bool debugger_trap_entry(
unsigned int vector, struct cpu_user_regs *regs)
{
+ /*
+ * This function is called before any checks are made. Amongst other
+ * things, be aware that during early boot, current is not a safe pointer
+ * to follow.
+ */
struct vcpu *v = current;
- if ( guest_kernel_mode(v, regs) && v->domain->debugger_attached &&
- ((vector == TRAP_int3) || (vector == TRAP_debug)) )
+ if ( vector != TRAP_int3 && vector != TRAP_debug )
+ return false;
+
+ if ( guest_mode(regs) && guest_kernel_mode(v, regs) &&
+ v->domain->debugger_attached )
{
if ( vector != TRAP_debug ) /* domain pause is good enough */
current->arch.gdbsx_vcpu_event = vector;
domain_pause_for_debugger();
- return 1;
+ return true;
}
- return 0;
+ return false;
}
unsigned int dbg_rw_mem(void * __user addr, void * __user buf,
--
2.1.4
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |