[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[xen master] x86: Fix XEN_DOMCTL_gdbsx_guestmemio crash



commit 71dd0f6e3b83d9f9fcf310904080752610da7033
Author:     Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
AuthorDate: Wed Apr 20 16:26:33 2022 +0100
Commit:     Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
CommitDate: Tue Nov 21 13:28:51 2023 +0000

    x86: Fix XEN_DOMCTL_gdbsx_guestmemio crash
    
    Prior to commit 9cd7e31b3f58 ("xen/iommu: cleanup iommu related domctl
    handling"), when CONFIG_GDBSX was compiled out, iommu_do_domctl() fell over 
a
    NULL pointer.  One of several bugs here is known-but-compiled-out subops
    falling into the default chain and hitting unrelated logic.
    
    Remove the CONFIG_GDBSX ifdefary in arch_do_domctl() by implementing
    gdbsx_domctl() and moving the logic across.
    
    As minor cleanup,
     * gdbsx_guest_mem_io() can become static
     * Remove opencoding of domain_vcpu() and %pd
    
    Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
    Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>
---
 xen/arch/x86/domctl.c            | 61 +---------------------------------
 xen/arch/x86/gdbsx.c             | 70 +++++++++++++++++++++++++++++++++++++++-
 xen/arch/x86/include/asm/gdbsx.h | 16 +++++++--
 3 files changed, 83 insertions(+), 64 deletions(-)

diff --git a/xen/arch/x86/domctl.c b/xen/arch/x86/domctl.c
index 1a8b4cff48..3c0ea85541 100644
--- a/xen/arch/x86/domctl.c
+++ b/xen/arch/x86/domctl.c
@@ -814,71 +814,12 @@ long arch_do_domctl(
     }
 #endif
 
-#ifdef CONFIG_GDBSX
     case XEN_DOMCTL_gdbsx_guestmemio:
-        ret = gdbsx_guest_mem_io(d, &domctl->u.gdbsx_guest_memio);
-        if ( !ret )
-           copyback = true;
-        break;
-
     case XEN_DOMCTL_gdbsx_pausevcpu:
-    {
-        struct vcpu *v;
-
-        ret = -EBUSY;
-        if ( !d->controller_pause_count )
-            break;
-        ret = -EINVAL;
-        if ( domctl->u.gdbsx_pauseunp_vcpu.vcpu >= d->max_vcpus ||
-             (v = d->vcpu[domctl->u.gdbsx_pauseunp_vcpu.vcpu]) == NULL )
-            break;
-        ret = vcpu_pause_by_systemcontroller(v);
-        break;
-    }
-
     case XEN_DOMCTL_gdbsx_unpausevcpu:
-    {
-        struct vcpu *v;
-
-        ret = -EBUSY;
-        if ( !d->controller_pause_count )
-            break;
-        ret = -EINVAL;
-        if ( domctl->u.gdbsx_pauseunp_vcpu.vcpu >= d->max_vcpus ||
-             (v = d->vcpu[domctl->u.gdbsx_pauseunp_vcpu.vcpu]) == NULL )
-            break;
-        ret = vcpu_unpause_by_systemcontroller(v);
-        if ( ret == -EINVAL )
-            printk(XENLOG_G_WARNING
-                   "WARN: d%d attempting to unpause %pv which is not paused\n",
-                   currd->domain_id, v);
-        break;
-    }
-
     case XEN_DOMCTL_gdbsx_domstatus:
-    {
-        struct vcpu *v;
-
-        domctl->u.gdbsx_domstatus.vcpu_id = -1;
-        domctl->u.gdbsx_domstatus.paused = d->controller_pause_count > 0;
-        if ( domctl->u.gdbsx_domstatus.paused )
-        {
-            for_each_vcpu ( d, v )
-            {
-                if ( v->arch.gdbsx_vcpu_event )
-                {
-                    domctl->u.gdbsx_domstatus.vcpu_id = v->vcpu_id;
-                    domctl->u.gdbsx_domstatus.vcpu_ev =
-                        v->arch.gdbsx_vcpu_event;
-                    v->arch.gdbsx_vcpu_event = 0;
-                    break;
-                }
-            }
-        }
-        copyback = true;
+        ret = gdbsx_domctl(d, domctl, &copyback);
         break;
-    }
-#endif
 
     case XEN_DOMCTL_setvcpuextstate:
     case XEN_DOMCTL_getvcpuextstate:
diff --git a/xen/arch/x86/gdbsx.c b/xen/arch/x86/gdbsx.c
index 6ef46e8ea7..21442f5dff 100644
--- a/xen/arch/x86/gdbsx.c
+++ b/xen/arch/x86/gdbsx.c
@@ -152,7 +152,8 @@ static unsigned int dbg_rw_guest_mem(struct domain *dp, 
unsigned long addr,
     return len;
 }
 
-int gdbsx_guest_mem_io(struct domain *d, struct xen_domctl_gdbsx_memio *iop)
+static int gdbsx_guest_mem_io(
+    struct domain *d, struct xen_domctl_gdbsx_memio *iop)
 {
     if ( d && !d->is_dying )
     {
@@ -178,6 +179,73 @@ void domain_pause_for_debugger(void)
         send_global_virq(VIRQ_DEBUGGER);
 }
 
+int gdbsx_domctl(struct domain *d, struct xen_domctl *domctl, bool *copyback)
+{
+    struct vcpu *v;
+    int ret;
+
+    switch ( domctl->cmd )
+    {
+    case XEN_DOMCTL_gdbsx_guestmemio:
+        ret = gdbsx_guest_mem_io(d, &domctl->u.gdbsx_guest_memio);
+        if ( !ret )
+            *copyback = true;
+        break;
+
+    case XEN_DOMCTL_gdbsx_pausevcpu:
+        ret = -EBUSY;
+        if ( !d->controller_pause_count )
+            break;
+        ret = -EINVAL;
+        if ( (v = domain_vcpu(d, domctl->u.gdbsx_pauseunp_vcpu.vcpu)) == NULL )
+            break;
+        ret = vcpu_pause_by_systemcontroller(v);
+        break;
+
+    case XEN_DOMCTL_gdbsx_unpausevcpu:
+        ret = -EBUSY;
+        if ( !d->controller_pause_count )
+            break;
+        ret = -EINVAL;
+        if ( (v = domain_vcpu(d, domctl->u.gdbsx_pauseunp_vcpu.vcpu)) == NULL )
+            break;
+        ret = vcpu_unpause_by_systemcontroller(v);
+        if ( ret == -EINVAL )
+            printk(XENLOG_G_WARNING
+                   "WARN: %pd attempting to unpause %pv which is not paused\n",
+                   current->domain, v);
+        break;
+
+    case XEN_DOMCTL_gdbsx_domstatus:
+        ret = 0;
+        domctl->u.gdbsx_domstatus.vcpu_id = -1;
+        domctl->u.gdbsx_domstatus.paused = d->controller_pause_count > 0;
+        if ( domctl->u.gdbsx_domstatus.paused )
+        {
+            for_each_vcpu ( d, v )
+            {
+                if ( v->arch.gdbsx_vcpu_event )
+                {
+                    domctl->u.gdbsx_domstatus.vcpu_id = v->vcpu_id;
+                    domctl->u.gdbsx_domstatus.vcpu_ev =
+                        v->arch.gdbsx_vcpu_event;
+                    v->arch.gdbsx_vcpu_event = 0;
+                    break;
+                }
+            }
+        }
+        *copyback = true;
+        break;
+
+    default:
+        ASSERT_UNREACHABLE();
+        ret = -ENOSYS;
+        break;
+    }
+
+    return ret;
+}
+
 /*
  * Local variables:
  * mode: C
diff --git a/xen/arch/x86/include/asm/gdbsx.h b/xen/arch/x86/include/asm/gdbsx.h
index 938eb74e2e..e906be9ea3 100644
--- a/xen/arch/x86/include/asm/gdbsx.h
+++ b/xen/arch/x86/include/asm/gdbsx.h
@@ -2,18 +2,28 @@
 #ifndef __X86_GDBX_H__
 #define __X86_GDBX_H__
 
-#ifdef CONFIG_GDBSX
+#include <xen/stdbool.h>
 
 struct domain;
-struct xen_domctl_gdbsx_memio;
+struct xen_domctl;
 
-int gdbsx_guest_mem_io(struct domain *d, struct xen_domctl_gdbsx_memio *iop);
+#ifdef CONFIG_GDBSX
 
 void domain_pause_for_debugger(void);
 
+int gdbsx_domctl(struct domain *d, struct xen_domctl *domctl, bool *copyback);
+
 #else
 
+#include <xen/errno.h>
+
 static inline void domain_pause_for_debugger(void) {}
 
+static inline int gdbsx_domctl(
+    struct domain *d, struct xen_domctl *domctl, bool *copyback)
+{
+    return -ENOSYS;
+}
+
 #endif /* CONFIG_GDBSX */
 #endif /* __X86_GDBX_H__ */
--
generated by git-patchbot for /home/xen/git/xen.git#master



 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.