xen: make debugger support configurable
diff mbox series

Message ID 20191113162229.1140-1-jgross@suse.com
State New
Headers show
Series
  • xen: make debugger support configurable
Related show

Commit Message

Jürgen Groß Nov. 13, 2019, 4:22 p.m. UTC
Debugger support in the hypervisor is rarely used and it is opening a
way for dom0 to modify the running hypervisor by very easy means.

Add a Kconfig option to control support of gdbsx. Default is off.

Signed-off-by: Juergen Gross <jgross@suse.com>
---
 xen/Kconfig.debug              |  4 ++++
 xen/arch/x86/Kconfig           |  1 -
 xen/arch/x86/domctl.c          |  4 ++++
 xen/common/Kconfig             |  3 ---
 xen/common/domain.c            |  2 +-
 xen/include/asm-x86/debugger.h | 30 ++++++++++++++++++------------
 xen/include/xen/sched.h        |  4 ++++
 7 files changed, 31 insertions(+), 17 deletions(-)

Comments

Jan Beulich Nov. 13, 2019, 4:59 p.m. UTC | #1
On 13.11.2019 17:22, Juergen Gross wrote:
> Debugger support in the hypervisor is rarely used and it is opening a
> way for dom0 to modify the running hypervisor by very easy means.
> 
> Add a Kconfig option to control support of gdbsx. Default is off.
> 
> Signed-off-by: Juergen Gross <jgross@suse.com>

Acked-by: Jan Beulich <jbeulich@suse.com>
Andrew Cooper Nov. 13, 2019, 5:26 p.m. UTC | #2
On 13/11/2019 16:22, Juergen Gross wrote:
> Debugger support in the hypervisor is rarely used and it is opening a
> way for dom0 to modify the running hypervisor by very easy means.
>
> Add a Kconfig option to control support of gdbsx. Default is off.
>
> Signed-off-by: Juergen Gross <jgross@suse.com>
> ---
>  xen/Kconfig.debug              |  4 ++++
>  xen/arch/x86/Kconfig           |  1 -
>  xen/arch/x86/domctl.c          |  4 ++++
>  xen/common/Kconfig             |  3 ---
>  xen/common/domain.c            |  2 +-
>  xen/include/asm-x86/debugger.h | 30 ++++++++++++++++++------------
>  xen/include/xen/sched.h        |  4 ++++
>  7 files changed, 31 insertions(+), 17 deletions(-)
>
> diff --git a/xen/Kconfig.debug b/xen/Kconfig.debug
> index 22573e74db..84a6e1b8eb 100644
> --- a/xen/Kconfig.debug
> +++ b/xen/Kconfig.debug
> @@ -13,9 +13,13 @@ config DEBUG
>  
>  if DEBUG || EXPERT = "y"
>  
> +config GDBSX
> +	bool
> +
>  config CRASH_DEBUG
>  	bool "Crash Debugging Support"
>  	depends on X86
> +	select GDBSX
>  	---help---
>  	  If you want to attach gdb to Xen to debug Xen if it crashes
>  	  then say Y.

CRASH_DEBUG and GDBSX are unrelated.

The former is gdbstub over serial for Xen itself (which I've never seen
used, and therefore doubt functions), while the latter is a set of dom0
hypercalls used by the gdbsx utility.

I'm happy to make CONFIG_GDBSX more useful than it currently is, but I
don't think the two options want conflating.

~Andrew
Jürgen Groß Nov. 14, 2019, 7:52 a.m. UTC | #3
On 13.11.19 18:26, Andrew Cooper wrote:
> On 13/11/2019 16:22, Juergen Gross wrote:
>> Debugger support in the hypervisor is rarely used and it is opening a
>> way for dom0 to modify the running hypervisor by very easy means.
>>
>> Add a Kconfig option to control support of gdbsx. Default is off.
>>
>> Signed-off-by: Juergen Gross <jgross@suse.com>
>> ---
>>   xen/Kconfig.debug              |  4 ++++
>>   xen/arch/x86/Kconfig           |  1 -
>>   xen/arch/x86/domctl.c          |  4 ++++
>>   xen/common/Kconfig             |  3 ---
>>   xen/common/domain.c            |  2 +-
>>   xen/include/asm-x86/debugger.h | 30 ++++++++++++++++++------------
>>   xen/include/xen/sched.h        |  4 ++++
>>   7 files changed, 31 insertions(+), 17 deletions(-)
>>
>> diff --git a/xen/Kconfig.debug b/xen/Kconfig.debug
>> index 22573e74db..84a6e1b8eb 100644
>> --- a/xen/Kconfig.debug
>> +++ b/xen/Kconfig.debug
>> @@ -13,9 +13,13 @@ config DEBUG
>>   
>>   if DEBUG || EXPERT = "y"
>>   
>> +config GDBSX
>> +	bool
>> +
>>   config CRASH_DEBUG
>>   	bool "Crash Debugging Support"
>>   	depends on X86
>> +	select GDBSX
>>   	---help---
>>   	  If you want to attach gdb to Xen to debug Xen if it crashes
>>   	  then say Y.
> 
> CRASH_DEBUG and GDBSX are unrelated.
> 
> The former is gdbstub over serial for Xen itself (which I've never seen
> used, and therefore doubt functions), while the latter is a set of dom0
> hypercalls used by the gdbsx utility.
> 
> I'm happy to make CONFIG_GDBSX more useful than it currently is, but I
> don't think the two options want conflating.

Ah, okay.

Will send V2 with 2 patches: one for putting more code under
CONFIG_CRASH_DEBUG and one for introducing CONFIG_GDBSX.


Juergen

Patch
diff mbox series

diff --git a/xen/Kconfig.debug b/xen/Kconfig.debug
index 22573e74db..84a6e1b8eb 100644
--- a/xen/Kconfig.debug
+++ b/xen/Kconfig.debug
@@ -13,9 +13,13 @@  config DEBUG
 
 if DEBUG || EXPERT = "y"
 
+config GDBSX
+	bool
+
 config CRASH_DEBUG
 	bool "Crash Debugging Support"
 	depends on X86
+	select GDBSX
 	---help---
 	  If you want to attach gdb to Xen to debug Xen if it crashes
 	  then say Y.
diff --git a/xen/arch/x86/Kconfig b/xen/arch/x86/Kconfig
index 28b3b4692a..c72da8964a 100644
--- a/xen/arch/x86/Kconfig
+++ b/xen/arch/x86/Kconfig
@@ -13,7 +13,6 @@  config X86
 	select HAS_EHCI
 	select HAS_EX_TABLE
 	select HAS_FAST_MULTIPLY
-	select HAS_GDBSX
 	select HAS_IOPORTS
 	select HAS_KEXEC
 	select MEM_ACCESS_ALWAYS_ON
diff --git a/xen/arch/x86/domctl.c b/xen/arch/x86/domctl.c
index 43e368d63b..90e36b9ad8 100644
--- a/xen/arch/x86/domctl.c
+++ b/xen/arch/x86/domctl.c
@@ -36,6 +36,7 @@ 
 #include <asm/psr.h>
 #include <asm/cpuid.h>
 
+#ifdef CONFIG_GDBSX
 static int gdbsx_guest_mem_io(domid_t domid, struct xen_domctl_gdbsx_memio *iop)
 {
     void * __user gva = (void *)iop->gva, * __user uva = (void *)iop->uva;
@@ -45,6 +46,7 @@  static int gdbsx_guest_mem_io(domid_t domid, struct xen_domctl_gdbsx_memio *iop)
 
     return iop->remain ? -EFAULT : 0;
 }
+#endif
 
 static void domain_cpu_policy_changed(struct domain *d)
 {
@@ -912,6 +914,7 @@  long arch_do_domctl(
     }
 #endif
 
+#ifdef CONFIG_GDBSX
     case XEN_DOMCTL_gdbsx_guestmemio:
         domctl->u.gdbsx_guest_memio.remain = domctl->u.gdbsx_guest_memio.len;
         ret = gdbsx_guest_mem_io(domctl->domain, &domctl->u.gdbsx_guest_memio);
@@ -976,6 +979,7 @@  long arch_do_domctl(
         copyback = true;
         break;
     }
+#endif
 
     case XEN_DOMCTL_setvcpuextstate:
     case XEN_DOMCTL_getvcpuextstate:
diff --git a/xen/common/Kconfig b/xen/common/Kconfig
index f754741972..7bde6aff02 100644
--- a/xen/common/Kconfig
+++ b/xen/common/Kconfig
@@ -57,9 +57,6 @@  config HAS_UBSAN
 config HAS_KEXEC
 	bool
 
-config HAS_GDBSX
-	bool
-
 config HAS_IOPORTS
 	bool
 
diff --git a/xen/common/domain.c b/xen/common/domain.c
index 611116c7fc..b4e041ffd0 100644
--- a/xen/common/domain.c
+++ b/xen/common/domain.c
@@ -915,7 +915,7 @@  void vcpu_end_shutdown_deferral(struct vcpu *v)
         vcpu_check_shutdown(v);
 }
 
-#ifdef CONFIG_HAS_GDBSX
+#ifdef CONFIG_GDBSX
 void domain_pause_for_debugger(void)
 {
     struct vcpu *curr = current;
diff --git a/xen/include/asm-x86/debugger.h b/xen/include/asm-x86/debugger.h
index b1b627f1fa..4a8f226b86 100644
--- a/xen/include/asm-x86/debugger.h
+++ b/xen/include/asm-x86/debugger.h
@@ -47,18 +47,6 @@  static inline bool debugger_trap_fatal(
 /* Int3 is a trivial way to gather cpu_user_regs context. */
 #define debugger_trap_immediate() __asm__ __volatile__ ( "int3" );
 
-#else
-
-static inline bool debugger_trap_fatal(
-    unsigned int vector, struct cpu_user_regs *regs)
-{
-    return false;
-}
-
-#define debugger_trap_immediate() ((void)0)
-
-#endif
-
 static inline bool debugger_trap_entry(
     unsigned int vector, struct cpu_user_regs *regs)
 {
@@ -88,4 +76,22 @@  unsigned int dbg_rw_mem(void * __user addr, void * __user buf,
                         unsigned int len, domid_t domid, bool toaddr,
                         uint64_t pgd3);
 
+#else
+
+static inline bool debugger_trap_fatal(
+    unsigned int vector, struct cpu_user_regs *regs)
+{
+    return false;
+}
+
+#define debugger_trap_immediate() ((void)0)
+
+static inline bool debugger_trap_entry(
+    unsigned int vector, struct cpu_user_regs *regs)
+{
+    return false;
+}
+
+#endif
+
 #endif /* __X86_DEBUGGER_H__ */
diff --git a/xen/include/xen/sched.h b/xen/include/xen/sched.h
index 9f7bc69293..094003f562 100644
--- a/xen/include/xen/sched.h
+++ b/xen/include/xen/sched.h
@@ -652,7 +652,11 @@  void domain_destroy(struct domain *d);
 int domain_kill(struct domain *d);
 int domain_shutdown(struct domain *d, u8 reason);
 void domain_resume(struct domain *d);
+#ifdef CONFIG_GDBSX
 void domain_pause_for_debugger(void);
+#else
+static inline void domain_pause_for_debugger(void) { }
+#endif
 
 int domain_soft_reset(struct domain *d);