All of lore.kernel.org
 help / color / mirror / Atom feed
From: Andrew Cooper <andrew.cooper3@citrix.com>
To: Xen-devel <xen-devel@lists.xenproject.org>
Cc: "Andrew Cooper" <andrew.cooper3@citrix.com>,
	"George Dunlap" <George.Dunlap@eu.citrix.com>,
	"Jan Beulich" <JBeulich@suse.com>,
	"Stefano Stabellini" <sstabellini@kernel.org>,
	"Wei Liu" <wl@xen.org>, "Julien Grall" <julien@xen.org>,
	"Juergen Gross" <jgross@suse.com>,
	"Roger Pau Monné" <roger.pau@citrix.com>,
	"Volodymyr Babchuk" <Volodymyr_Babchuk@epam.com>,
	"Bertrand Marquis" <bertrand.marquis@arm.com>
Subject: [PATCH RFC] x86: Fix XEN_DOMCTL_gdbsx_guestmemio crash
Date: Wed, 20 Apr 2022 16:56:57 +0100	[thread overview]
Message-ID: <20220420155657.32506-1-andrew.cooper3@citrix.com> (raw)

When CONFIG_GDBSX is compiled out, iommu_do_domctl() falls over a NULL
pointer.  It isn't really correct for processing of XEN_DOMCTL_gdbsx_* to fall
into the default case when compiled out.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: George Dunlap <George.Dunlap@eu.citrix.com>
CC: Jan Beulich <JBeulich@suse.com>
CC: Stefano Stabellini <sstabellini@kernel.org>
CC: Wei Liu <wl@xen.org>
CC: Julien Grall <julien@xen.org>
CC: Juergen Gross <jgross@suse.com>
CC: Roger Pau Monné <roger.pau@citrix.com>
CC: Volodymyr Babchuk <Volodymyr_Babchuk@epam.com>
CC: Bertrand Marquis <bertrand.marquis@arm.com>

RFC, because this has implications across the codebase.  The tl;dr is that
case FOO:'s shouldn't be compiled out; we still know what the subops are, even
when the functionality is compiled out.

There are several ways to express this.  Alternatives would be:

    case XEN_DOMCTL_gdbsx_guestmemio:
        if ( !IS_ENABLED(CONFIG_GDBSX) )
        {
            rc = -EOPNOTSUPP;
            break;
        }
        ...;

but given my debugger series creating gdbsx.c, I was also considering:

    case XEN_DOMCTL_gdbsx_guestmemio:
    case XEN_DOMCTL_gdbsx_pausevcpu:
    case XEN_DOMCTL_gdbsx_unpausevcpu:
    case XEN_DOMCTL_gdbsx_domstatus:
        rc = gdbsx_do_domctl(d, iop);
        break;

when I can rework the callers of domain_pause_for_debugger() slightly, at
which point we can conditionally compile the gdbsx variables out struct
domain/vcpu, which wouldn't be compatible with the first suggestion.

Thoughts?
---
 xen/arch/x86/domctl.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/xen/arch/x86/domctl.c b/xen/arch/x86/domctl.c
index a6aae500a30b..1faa5a49ff3c 100644
--- a/xen/arch/x86/domctl.c
+++ b/xen/arch/x86/domctl.c
@@ -890,7 +890,14 @@ long arch_do_domctl(
         copyback = true;
         break;
     }
-#endif
+#else /* CONFIG_GDBSX */
+    case XEN_DOMCTL_gdbsx_guestmemio:
+    case XEN_DOMCTL_gdbsx_pausevcpu:
+    case XEN_DOMCTL_gdbsx_unpausevcpu:
+    case XEN_DOMCTL_gdbsx_domstatus:
+        rc = -EOPNOTSUPP;
+        break;
+#endif /* CONFIG_GDBSX */
 
     case XEN_DOMCTL_setvcpuextstate:
     case XEN_DOMCTL_getvcpuextstate:
-- 
2.11.0



             reply	other threads:[~2022-04-20 15:57 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-04-20 15:56 Andrew Cooper [this message]
2022-04-20 16:03 ` [PATCH RFC] x86: Fix XEN_DOMCTL_gdbsx_guestmemio crash Juergen Gross
2022-04-21  9:26   ` Jan Beulich
2022-04-21  9:46     ` Andrew Cooper

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20220420155657.32506-1-andrew.cooper3@citrix.com \
    --to=andrew.cooper3@citrix.com \
    --cc=George.Dunlap@eu.citrix.com \
    --cc=JBeulich@suse.com \
    --cc=Volodymyr_Babchuk@epam.com \
    --cc=bertrand.marquis@arm.com \
    --cc=jgross@suse.com \
    --cc=julien@xen.org \
    --cc=roger.pau@citrix.com \
    --cc=sstabellini@kernel.org \
    --cc=wl@xen.org \
    --cc=xen-devel@lists.xenproject.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.