* [PATCH v3 0/3] xen: fix and cleanup domctl handling
@ 2022-04-19 13:52 Juergen Gross
2022-04-19 13:52 ` [PATCH v3 1/3] xen: fix XEN_DOMCTL_gdbsx_guestmemio crash Juergen Gross
` (2 more replies)
0 siblings, 3 replies; 13+ messages in thread
From: Juergen Gross @ 2022-04-19 13:52 UTC (permalink / raw)
To: xen-devel
Cc: Juergen Gross, Andrew Cooper, George Dunlap, Jan Beulich,
Julien Grall, Stefano Stabellini, Wei Liu, Elena Ufimtseva,
Roger Pau Monné,
Bertrand Marquis, Volodymyr Babchuk
A fix of a hypervisor crash in domctl handling and some related cleanup.
Juergen Gross (3):
xen: fix XEN_DOMCTL_gdbsx_guestmemio crash
xen: cleanup gdbsx_guest_mem_io() call
xen/iommu: cleanup iommu related domctl handling
xen/arch/arm/domctl.c | 11 +----------
xen/arch/x86/debug.c | 12 +++---------
xen/arch/x86/domctl.c | 8 ++++----
xen/arch/x86/include/asm/debugger.h | 2 +-
xen/common/domctl.c | 8 +++++++-
5 files changed, 16 insertions(+), 25 deletions(-)
--
2.34.1
^ permalink raw reply [flat|nested] 13+ messages in thread* [PATCH v3 1/3] xen: fix XEN_DOMCTL_gdbsx_guestmemio crash 2022-04-19 13:52 [PATCH v3 0/3] xen: fix and cleanup domctl handling Juergen Gross @ 2022-04-19 13:52 ` Juergen Gross 2022-04-19 14:32 ` Jan Beulich 2022-04-19 14:37 ` Andrew Cooper 2022-04-19 13:52 ` [PATCH v3 2/3] xen: cleanup gdbsx_guest_mem_io() call Juergen Gross 2022-04-19 13:52 ` [PATCH v3 3/3] xen/iommu: cleanup iommu related domctl handling Juergen Gross 2 siblings, 2 replies; 13+ messages in thread From: Juergen Gross @ 2022-04-19 13:52 UTC (permalink / raw) To: xen-devel Cc: Juergen Gross, Andrew Cooper, George Dunlap, Jan Beulich, Julien Grall, Stefano Stabellini, Wei Liu, Cheyenne Wills A hypervisor built without CONFIG_GDBSX will crash in case the XEN_DOMCTL_gdbsx_guestmemio domctl is being called, as the call will end up in iommu_do_domctl() with d == NULL: (XEN) CPU: 6 (XEN) RIP: e008:[<ffff82d040269984>] iommu_do_domctl+0x4/0x30 (XEN) RFLAGS: 0000000000010202 CONTEXT: hypervisor (d0v0) (XEN) rax: 00000000000003e8 rbx: ffff830856277ef8 rcx: ffff830856277fff ... (XEN) Xen call trace: (XEN) [<ffff82d040269984>] R iommu_do_domctl+0x4/0x30 (XEN) [<ffff82d04035cd5f>] S arch_do_domctl+0x7f/0x2330 (XEN) [<ffff82d040239e46>] S do_domctl+0xe56/0x1930 (XEN) [<ffff82d040238ff0>] S do_domctl+0/0x1930 (XEN) [<ffff82d0402f8c59>] S pv_hypercall+0x99/0x110 (XEN) [<ffff82d0402f5161>] S arch/x86/pv/domain.c#_toggle_guest_pt+0x11/0x90 (XEN) [<ffff82d040366288>] S lstar_enter+0x128/0x130 (XEN) (XEN) Pagetable walk from 0000000000000144: (XEN) L4[0x000] = 0000000000000000 ffffffffffffffff (XEN) (XEN) **************************************** (XEN) Panic on CPU 6: (XEN) FATAL PAGE FAULT (XEN) [error_code=0000] (XEN) Faulting linear address: 0000000000000144 Fix this issue by making sure the domain pointer has a sane value. Reported-by: Cheyenne Wills <cheyenne.wills@gmail.com> Fixes: e726a82ca0dc ("xen: make gdbsx support configurable") Signed-off-by: Juergen Gross <jgross@suse.com> --- V2: - use gdbsx_guest_mem_io() interface modification (Jan Beulich) V3: - avoid d being NULL (Andrew Cooper) --- xen/common/domctl.c | 1 - 1 file changed, 1 deletion(-) diff --git a/xen/common/domctl.c b/xen/common/domctl.c index 57135d4478..5879117580 100644 --- a/xen/common/domctl.c +++ b/xen/common/domctl.c @@ -308,7 +308,6 @@ long cf_check do_domctl(XEN_GUEST_HANDLE_PARAM(xen_domctl_t) u_domctl) if ( op->domain == DOMID_INVALID ) { case XEN_DOMCTL_createdomain: - case XEN_DOMCTL_gdbsx_guestmemio: d = NULL; break; } -- 2.34.1 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH v3 1/3] xen: fix XEN_DOMCTL_gdbsx_guestmemio crash 2022-04-19 13:52 ` [PATCH v3 1/3] xen: fix XEN_DOMCTL_gdbsx_guestmemio crash Juergen Gross @ 2022-04-19 14:32 ` Jan Beulich 2022-04-19 14:37 ` Andrew Cooper 1 sibling, 0 replies; 13+ messages in thread From: Jan Beulich @ 2022-04-19 14:32 UTC (permalink / raw) To: Juergen Gross Cc: Andrew Cooper, George Dunlap, Julien Grall, Stefano Stabellini, Wei Liu, Cheyenne Wills, xen-devel On 19.04.2022 15:52, Juergen Gross wrote: > A hypervisor built without CONFIG_GDBSX will crash in case the > XEN_DOMCTL_gdbsx_guestmemio domctl is being called, as the call will > end up in iommu_do_domctl() with d == NULL: > > (XEN) CPU: 6 > (XEN) RIP: e008:[<ffff82d040269984>] iommu_do_domctl+0x4/0x30 > (XEN) RFLAGS: 0000000000010202 CONTEXT: hypervisor (d0v0) > (XEN) rax: 00000000000003e8 rbx: ffff830856277ef8 rcx: ffff830856277fff > ... > (XEN) Xen call trace: > (XEN) [<ffff82d040269984>] R iommu_do_domctl+0x4/0x30 > (XEN) [<ffff82d04035cd5f>] S arch_do_domctl+0x7f/0x2330 > (XEN) [<ffff82d040239e46>] S do_domctl+0xe56/0x1930 > (XEN) [<ffff82d040238ff0>] S do_domctl+0/0x1930 > (XEN) [<ffff82d0402f8c59>] S pv_hypercall+0x99/0x110 > (XEN) [<ffff82d0402f5161>] S arch/x86/pv/domain.c#_toggle_guest_pt+0x11/0x90 > (XEN) [<ffff82d040366288>] S lstar_enter+0x128/0x130 > (XEN) > (XEN) Pagetable walk from 0000000000000144: > (XEN) L4[0x000] = 0000000000000000 ffffffffffffffff > (XEN) > (XEN) **************************************** > (XEN) Panic on CPU 6: > (XEN) FATAL PAGE FAULT > (XEN) [error_code=0000] > (XEN) Faulting linear address: 0000000000000144 > > Fix this issue by making sure the domain pointer has a sane value. > > Reported-by: Cheyenne Wills <cheyenne.wills@gmail.com> > Fixes: e726a82ca0dc ("xen: make gdbsx support configurable") > Signed-off-by: Juergen Gross <jgross@suse.com> Reviewed-by: Jan Beulich <jbeulich@suse.com> ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v3 1/3] xen: fix XEN_DOMCTL_gdbsx_guestmemio crash 2022-04-19 13:52 ` [PATCH v3 1/3] xen: fix XEN_DOMCTL_gdbsx_guestmemio crash Juergen Gross 2022-04-19 14:32 ` Jan Beulich @ 2022-04-19 14:37 ` Andrew Cooper 2022-04-19 14:50 ` Juergen Gross 1 sibling, 1 reply; 13+ messages in thread From: Andrew Cooper @ 2022-04-19 14:37 UTC (permalink / raw) To: Juergen Gross, xen-devel Cc: George Dunlap, Jan Beulich, Julien Grall, Stefano Stabellini, Wei Liu, Cheyenne Wills On 19/04/2022 14:52, Juergen Gross wrote: > A hypervisor built without CONFIG_GDBSX will crash in case the > XEN_DOMCTL_gdbsx_guestmemio domctl is being called, as the call will > end up in iommu_do_domctl() with d == NULL: > > (XEN) CPU: 6 > (XEN) RIP: e008:[<ffff82d040269984>] iommu_do_domctl+0x4/0x30 > (XEN) RFLAGS: 0000000000010202 CONTEXT: hypervisor (d0v0) > (XEN) rax: 00000000000003e8 rbx: ffff830856277ef8 rcx: ffff830856277fff > ... > (XEN) Xen call trace: > (XEN) [<ffff82d040269984>] R iommu_do_domctl+0x4/0x30 > (XEN) [<ffff82d04035cd5f>] S arch_do_domctl+0x7f/0x2330 > (XEN) [<ffff82d040239e46>] S do_domctl+0xe56/0x1930 > (XEN) [<ffff82d040238ff0>] S do_domctl+0/0x1930 > (XEN) [<ffff82d0402f8c59>] S pv_hypercall+0x99/0x110 > (XEN) [<ffff82d0402f5161>] S arch/x86/pv/domain.c#_toggle_guest_pt+0x11/0x90 > (XEN) [<ffff82d040366288>] S lstar_enter+0x128/0x130 > (XEN) > (XEN) Pagetable walk from 0000000000000144: > (XEN) L4[0x000] = 0000000000000000 ffffffffffffffff > (XEN) > (XEN) **************************************** > (XEN) Panic on CPU 6: > (XEN) FATAL PAGE FAULT > (XEN) [error_code=0000] > (XEN) Faulting linear address: 0000000000000144 > > Fix this issue by making sure the domain pointer has a sane value. > > Reported-by: Cheyenne Wills <cheyenne.wills@gmail.com> > Fixes: e726a82ca0dc ("xen: make gdbsx support configurable") > Signed-off-by: Juergen Gross <jgross@suse.com> Thanks, but I was hoping for a bit of discussion on the use of DOMID_IDLE. It used to be permitted to pass DOMID_IDLE to dbg_rw_mem() to access Xen memory, which is why the XEN_DOMCTL_gdbsx_guestmemio special case existed. It turns out that it was also e726a82ca0dc which dropped the ability to use DOMID_IDLE, meaning that this fix is a missing hunk from the original change too. This is relevant backport information, and would have created complexities if they hadn't been the same changeset. So, now about: "It used to be permitted to pass DOMID_IDLE to dbg_rw_mem(), which is why the special case excluding domid checks exists. Now that it is only permitted to pass proper domids, remove the special case, thus making 'd' always valid." ? Can be fixed on commit, so Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com> for everything else. ~Andrew ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v3 1/3] xen: fix XEN_DOMCTL_gdbsx_guestmemio crash 2022-04-19 14:37 ` Andrew Cooper @ 2022-04-19 14:50 ` Juergen Gross 0 siblings, 0 replies; 13+ messages in thread From: Juergen Gross @ 2022-04-19 14:50 UTC (permalink / raw) To: Andrew Cooper, xen-devel Cc: George Dunlap, Jan Beulich, Julien Grall, Stefano Stabellini, Wei Liu, Cheyenne Wills [-- Attachment #1.1.1: Type: text/plain, Size: 2522 bytes --] On 19.04.22 16:37, Andrew Cooper wrote: > On 19/04/2022 14:52, Juergen Gross wrote: >> A hypervisor built without CONFIG_GDBSX will crash in case the >> XEN_DOMCTL_gdbsx_guestmemio domctl is being called, as the call will >> end up in iommu_do_domctl() with d == NULL: >> >> (XEN) CPU: 6 >> (XEN) RIP: e008:[<ffff82d040269984>] iommu_do_domctl+0x4/0x30 >> (XEN) RFLAGS: 0000000000010202 CONTEXT: hypervisor (d0v0) >> (XEN) rax: 00000000000003e8 rbx: ffff830856277ef8 rcx: ffff830856277fff >> ... >> (XEN) Xen call trace: >> (XEN) [<ffff82d040269984>] R iommu_do_domctl+0x4/0x30 >> (XEN) [<ffff82d04035cd5f>] S arch_do_domctl+0x7f/0x2330 >> (XEN) [<ffff82d040239e46>] S do_domctl+0xe56/0x1930 >> (XEN) [<ffff82d040238ff0>] S do_domctl+0/0x1930 >> (XEN) [<ffff82d0402f8c59>] S pv_hypercall+0x99/0x110 >> (XEN) [<ffff82d0402f5161>] S arch/x86/pv/domain.c#_toggle_guest_pt+0x11/0x90 >> (XEN) [<ffff82d040366288>] S lstar_enter+0x128/0x130 >> (XEN) >> (XEN) Pagetable walk from 0000000000000144: >> (XEN) L4[0x000] = 0000000000000000 ffffffffffffffff >> (XEN) >> (XEN) **************************************** >> (XEN) Panic on CPU 6: >> (XEN) FATAL PAGE FAULT >> (XEN) [error_code=0000] >> (XEN) Faulting linear address: 0000000000000144 >> >> Fix this issue by making sure the domain pointer has a sane value. >> >> Reported-by: Cheyenne Wills <cheyenne.wills@gmail.com> >> Fixes: e726a82ca0dc ("xen: make gdbsx support configurable") >> Signed-off-by: Juergen Gross <jgross@suse.com> > > Thanks, but I was hoping for a bit of discussion on the use of DOMID_IDLE. > > It used to be permitted to pass DOMID_IDLE to dbg_rw_mem() to access Xen > memory, which is why the XEN_DOMCTL_gdbsx_guestmemio special case existed. > > It turns out that it was also e726a82ca0dc which dropped the ability to > use DOMID_IDLE, meaning that this fix is a missing hunk from the > original change too. > > This is relevant backport information, and would have created > complexities if they hadn't been the same changeset. > > So, now about: > > "It used to be permitted to pass DOMID_IDLE to dbg_rw_mem(), which is > why the special case excluding domid checks exists. Now that it is only > permitted to pass proper domids, remove the special case, thus making > 'd' always valid." > > ? Fine with me. > > Can be fixed on commit, so Reviewed-by: Andrew Cooper > <andrew.cooper3@citrix.com> for everything else. Thanks, Juergen [-- Attachment #1.1.2: OpenPGP public key --] [-- Type: application/pgp-keys, Size: 3149 bytes --] [-- Attachment #2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 495 bytes --] ^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH v3 2/3] xen: cleanup gdbsx_guest_mem_io() call 2022-04-19 13:52 [PATCH v3 0/3] xen: fix and cleanup domctl handling Juergen Gross 2022-04-19 13:52 ` [PATCH v3 1/3] xen: fix XEN_DOMCTL_gdbsx_guestmemio crash Juergen Gross @ 2022-04-19 13:52 ` Juergen Gross 2022-04-19 14:32 ` Jan Beulich 2022-04-19 14:41 ` Andrew Cooper 2022-04-19 13:52 ` [PATCH v3 3/3] xen/iommu: cleanup iommu related domctl handling Juergen Gross 2 siblings, 2 replies; 13+ messages in thread From: Juergen Gross @ 2022-04-19 13:52 UTC (permalink / raw) To: xen-devel Cc: Juergen Gross, Elena Ufimtseva, Jan Beulich, Andrew Cooper, Roger Pau Monné, Wei Liu Modify the gdbsx_guest_mem_io() interface to take the already known domain pointer as parameter instead of the domid. This enables to remove some more code further down the call tree. Suggested-by: Jan Beulich <jbeulich@suse.com> Signed-off-by: Juergen Gross <jgross@suse.com> --- V2: - split off from previous patch (Andrew Cooper) --- xen/arch/x86/debug.c | 12 +++--------- xen/arch/x86/domctl.c | 6 +++--- xen/arch/x86/include/asm/debugger.h | 2 +- 3 files changed, 7 insertions(+), 13 deletions(-) diff --git a/xen/arch/x86/debug.c b/xen/arch/x86/debug.c index d90dc93056..62fbabb084 100644 --- a/xen/arch/x86/debug.c +++ b/xen/arch/x86/debug.c @@ -159,17 +159,11 @@ static unsigned int dbg_rw_guest_mem(struct domain *dp, unsigned long addr, * Returns: number of bytes remaining to be copied. */ unsigned int dbg_rw_mem(unsigned long gva, XEN_GUEST_HANDLE_PARAM(void) buf, - unsigned int len, domid_t domid, bool toaddr, + unsigned int len, struct domain *d, bool toaddr, uint64_t pgd3) { - struct domain *d = rcu_lock_domain_by_id(domid); - - if ( d ) - { - if ( !d->is_dying ) - len = dbg_rw_guest_mem(d, gva, buf, len, toaddr, pgd3); - rcu_unlock_domain(d); - } + if ( d && !d->is_dying ) + len = dbg_rw_guest_mem(d, gva, buf, len, toaddr, pgd3); return len; } diff --git a/xen/arch/x86/domctl.c b/xen/arch/x86/domctl.c index e49f9e91b9..a6aae500a3 100644 --- a/xen/arch/x86/domctl.c +++ b/xen/arch/x86/domctl.c @@ -38,10 +38,10 @@ #include <asm/cpuid.h> #ifdef CONFIG_GDBSX -static int gdbsx_guest_mem_io(domid_t domid, struct xen_domctl_gdbsx_memio *iop) +static int gdbsx_guest_mem_io(struct domain *d, struct xen_domctl_gdbsx_memio *iop) { iop->remain = dbg_rw_mem(iop->gva, guest_handle_from_ptr(iop->uva, void), - iop->len, domid, iop->gwr, iop->pgd3val); + iop->len, d, iop->gwr, iop->pgd3val); return iop->remain ? -EFAULT : 0; } @@ -828,7 +828,7 @@ long arch_do_domctl( #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); + ret = gdbsx_guest_mem_io(d, &domctl->u.gdbsx_guest_memio); if ( !ret ) copyback = true; break; diff --git a/xen/arch/x86/include/asm/debugger.h b/xen/arch/x86/include/asm/debugger.h index 99803bfd0c..221bcde137 100644 --- a/xen/arch/x86/include/asm/debugger.h +++ b/xen/arch/x86/include/asm/debugger.h @@ -94,7 +94,7 @@ static inline bool debugger_trap_entry( #ifdef CONFIG_GDBSX unsigned int dbg_rw_mem(unsigned long gva, XEN_GUEST_HANDLE_PARAM(void) buf, - unsigned int len, domid_t domid, bool toaddr, + unsigned int len, struct domain *d, bool toaddr, uint64_t pgd3); #endif -- 2.34.1 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH v3 2/3] xen: cleanup gdbsx_guest_mem_io() call 2022-04-19 13:52 ` [PATCH v3 2/3] xen: cleanup gdbsx_guest_mem_io() call Juergen Gross @ 2022-04-19 14:32 ` Jan Beulich 2022-04-19 14:41 ` Andrew Cooper 1 sibling, 0 replies; 13+ messages in thread From: Jan Beulich @ 2022-04-19 14:32 UTC (permalink / raw) To: Juergen Gross Cc: Elena Ufimtseva, Andrew Cooper, Roger Pau Monné, Wei Liu, xen-devel On 19.04.2022 15:52, Juergen Gross wrote: > Modify the gdbsx_guest_mem_io() interface to take the already known > domain pointer as parameter instead of the domid. This enables to > remove some more code further down the call tree. > > Suggested-by: Jan Beulich <jbeulich@suse.com> > Signed-off-by: Juergen Gross <jgross@suse.com> Reviewed-by: Jan Beulich <jbeulich@suse.com> ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v3 2/3] xen: cleanup gdbsx_guest_mem_io() call 2022-04-19 13:52 ` [PATCH v3 2/3] xen: cleanup gdbsx_guest_mem_io() call Juergen Gross 2022-04-19 14:32 ` Jan Beulich @ 2022-04-19 14:41 ` Andrew Cooper 1 sibling, 0 replies; 13+ messages in thread From: Andrew Cooper @ 2022-04-19 14:41 UTC (permalink / raw) To: Juergen Gross, xen-devel Cc: Elena Ufimtseva, Jan Beulich, Roger Pau Monne, Wei Liu On 19/04/2022 14:52, Juergen Gross wrote: > Modify the gdbsx_guest_mem_io() interface to take the already known > domain pointer as parameter instead of the domid. This enables to > remove some more code further down the call tree. > > Suggested-by: Jan Beulich <jbeulich@suse.com> > Signed-off-by: Juergen Gross <jgross@suse.com> Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com> I'll rebase the other debugger cleanup over this. ^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH v3 3/3] xen/iommu: cleanup iommu related domctl handling 2022-04-19 13:52 [PATCH v3 0/3] xen: fix and cleanup domctl handling Juergen Gross 2022-04-19 13:52 ` [PATCH v3 1/3] xen: fix XEN_DOMCTL_gdbsx_guestmemio crash Juergen Gross 2022-04-19 13:52 ` [PATCH v3 2/3] xen: cleanup gdbsx_guest_mem_io() call Juergen Gross @ 2022-04-19 13:52 ` Juergen Gross 2022-04-19 14:41 ` Jan Beulich 2022-04-19 14:51 ` Andrew Cooper 2 siblings, 2 replies; 13+ messages in thread From: Juergen Gross @ 2022-04-19 13:52 UTC (permalink / raw) To: xen-devel Cc: Juergen Gross, Stefano Stabellini, Julien Grall, Bertrand Marquis, Volodymyr Babchuk, Andrew Cooper, George Dunlap, Jan Beulich, Wei Liu, Roger Pau Monné Today iommu_do_domctl() is being called from arch_do_domctl() in the "default:" case of a switch statement. This has led already to crashes due to unvalidated parameters. Fix that by moving the call of iommu_do_domctl() to the main switch statement of do_domctl(). Signed-off-by: Juergen Gross <jgross@suse.com> --- Another possibility would even be to merge iommu_do_domctl() completely into do_domctl(), but I wanted to start with a less intrusive variant. V3: - new patch --- xen/arch/arm/domctl.c | 11 +---------- xen/arch/x86/domctl.c | 2 +- xen/common/domctl.c | 7 +++++++ 3 files changed, 9 insertions(+), 11 deletions(-) diff --git a/xen/arch/arm/domctl.c b/xen/arch/arm/domctl.c index 6245af6d0b..1baf25c3d9 100644 --- a/xen/arch/arm/domctl.c +++ b/xen/arch/arm/domctl.c @@ -176,16 +176,7 @@ long arch_do_domctl(struct xen_domctl *domctl, struct domain *d, return rc; } default: - { - int rc; - - rc = subarch_do_domctl(domctl, d, u_domctl); - - if ( rc == -ENOSYS ) - rc = iommu_do_domctl(domctl, d, u_domctl); - - return rc; - } + return subarch_do_domctl(domctl, d, u_domctl); } } diff --git a/xen/arch/x86/domctl.c b/xen/arch/x86/domctl.c index a6aae500a3..c9699bb868 100644 --- a/xen/arch/x86/domctl.c +++ b/xen/arch/x86/domctl.c @@ -1380,7 +1380,7 @@ long arch_do_domctl( break; default: - ret = iommu_do_domctl(domctl, d, u_domctl); + ret = -ENOSYS; break; } diff --git a/xen/common/domctl.c b/xen/common/domctl.c index 5879117580..0a866e3132 100644 --- a/xen/common/domctl.c +++ b/xen/common/domctl.c @@ -871,6 +871,13 @@ long cf_check do_domctl(XEN_GUEST_HANDLE_PARAM(xen_domctl_t) u_domctl) copyback = 1; break; + case XEN_DOMCTL_assign_device: + case XEN_DOMCTL_test_assign_device: + case XEN_DOMCTL_deassign_device: + case XEN_DOMCTL_get_device_group: + ret = iommu_do_domctl(op, d, u_domctl); + break; + default: ret = arch_do_domctl(op, d, u_domctl); break; -- 2.34.1 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH v3 3/3] xen/iommu: cleanup iommu related domctl handling 2022-04-19 13:52 ` [PATCH v3 3/3] xen/iommu: cleanup iommu related domctl handling Juergen Gross @ 2022-04-19 14:41 ` Jan Beulich 2022-04-19 14:51 ` Andrew Cooper 1 sibling, 0 replies; 13+ messages in thread From: Jan Beulich @ 2022-04-19 14:41 UTC (permalink / raw) To: Juergen Gross Cc: Stefano Stabellini, Julien Grall, Bertrand Marquis, Volodymyr Babchuk, Andrew Cooper, George Dunlap, Wei Liu, Roger Pau Monné, xen-devel On 19.04.2022 15:52, Juergen Gross wrote: > --- a/xen/common/domctl.c > +++ b/xen/common/domctl.c > @@ -871,6 +871,13 @@ long cf_check do_domctl(XEN_GUEST_HANDLE_PARAM(xen_domctl_t) u_domctl) > copyback = 1; > break; > > + case XEN_DOMCTL_assign_device: > + case XEN_DOMCTL_test_assign_device: > + case XEN_DOMCTL_deassign_device: > + case XEN_DOMCTL_get_device_group: As said in reply to Andrew, I'm not convinced having these enumerated here is a good idea. But in any event the whole addition wants framing by #ifdef CONFIG_HAS_PASSTHROUGH now. For x86 I wonder whether the adjustment wouldn't allow to drop domctl.c's including of xen/iommu.h (but perhaps it's included indirectly anyway). Jan > + ret = iommu_do_domctl(op, d, u_domctl); > + break; > + > default: > ret = arch_do_domctl(op, d, u_domctl); > break; ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v3 3/3] xen/iommu: cleanup iommu related domctl handling 2022-04-19 13:52 ` [PATCH v3 3/3] xen/iommu: cleanup iommu related domctl handling Juergen Gross 2022-04-19 14:41 ` Jan Beulich @ 2022-04-19 14:51 ` Andrew Cooper 2022-04-19 14:56 ` Juergen Gross 1 sibling, 1 reply; 13+ messages in thread From: Andrew Cooper @ 2022-04-19 14:51 UTC (permalink / raw) To: Juergen Gross, xen-devel Cc: Stefano Stabellini, Julien Grall, Bertrand Marquis, Volodymyr Babchuk, George Dunlap, Jan Beulich, Wei Liu, Roger Pau Monne On 19/04/2022 14:52, Juergen Gross wrote: > Today iommu_do_domctl() is being called from arch_do_domctl() in the > "default:" case of a switch statement. This has led already to crashes > due to unvalidated parameters. > > Fix that by moving the call of iommu_do_domctl() to the main switch > statement of do_domctl(). > > Signed-off-by: Juergen Gross <jgross@suse.com> > --- > Another possibility would even be to merge iommu_do_domctl() completely > into do_domctl(), but I wanted to start with a less intrusive variant. > V3: > - new patch I definitely prefer this approach, thanks. In addition to being clearer, it's also faster because there isn't a long line of "do you understand this subop?" calls when we know exactly what it is. However, I think we need stub for the !HAS_PASSTHROUGH case now that it is being called from common code. I'd forgotten that it was used on ARM now, and yes - it absolutely should be called from somewhere common, not from the arch hooks. ~Andrew ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v3 3/3] xen/iommu: cleanup iommu related domctl handling 2022-04-19 14:51 ` Andrew Cooper @ 2022-04-19 14:56 ` Juergen Gross 2022-04-19 16:11 ` Jan Beulich 0 siblings, 1 reply; 13+ messages in thread From: Juergen Gross @ 2022-04-19 14:56 UTC (permalink / raw) To: Andrew Cooper, xen-devel Cc: Stefano Stabellini, Julien Grall, Bertrand Marquis, Volodymyr Babchuk, George Dunlap, Jan Beulich, Wei Liu, Roger Pau Monne [-- Attachment #1.1.1: Type: text/plain, Size: 1006 bytes --] On 19.04.22 16:51, Andrew Cooper wrote: > On 19/04/2022 14:52, Juergen Gross wrote: >> Today iommu_do_domctl() is being called from arch_do_domctl() in the >> "default:" case of a switch statement. This has led already to crashes >> due to unvalidated parameters. >> >> Fix that by moving the call of iommu_do_domctl() to the main switch >> statement of do_domctl(). >> >> Signed-off-by: Juergen Gross <jgross@suse.com> >> --- >> Another possibility would even be to merge iommu_do_domctl() completely >> into do_domctl(), but I wanted to start with a less intrusive variant. >> V3: >> - new patch > > I definitely prefer this approach, thanks. In addition to being > clearer, it's also faster because there isn't a long line of "do you > understand this subop?" calls when we know exactly what it is. > > However, I think we need stub for the !HAS_PASSTHROUGH case now that it > is being called from common code. Okay, I'll add it. Jan, are you fine with a stub? Juergen [-- Attachment #1.1.2: OpenPGP public key --] [-- Type: application/pgp-keys, Size: 3149 bytes --] [-- Attachment #2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 495 bytes --] ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v3 3/3] xen/iommu: cleanup iommu related domctl handling 2022-04-19 14:56 ` Juergen Gross @ 2022-04-19 16:11 ` Jan Beulich 0 siblings, 0 replies; 13+ messages in thread From: Jan Beulich @ 2022-04-19 16:11 UTC (permalink / raw) To: Juergen Gross Cc: Stefano Stabellini, Julien Grall, Bertrand Marquis, Volodymyr Babchuk, George Dunlap, Wei Liu, Andrew Cooper, xen-devel, Roger Pau Monne On 19.04.2022 16:56, Juergen Gross wrote: > On 19.04.22 16:51, Andrew Cooper wrote: >> On 19/04/2022 14:52, Juergen Gross wrote: >>> Today iommu_do_domctl() is being called from arch_do_domctl() in the >>> "default:" case of a switch statement. This has led already to crashes >>> due to unvalidated parameters. >>> >>> Fix that by moving the call of iommu_do_domctl() to the main switch >>> statement of do_domctl(). >>> >>> Signed-off-by: Juergen Gross <jgross@suse.com> >>> --- >>> Another possibility would even be to merge iommu_do_domctl() completely >>> into do_domctl(), but I wanted to start with a less intrusive variant. >>> V3: >>> - new patch >> >> I definitely prefer this approach, thanks. In addition to being >> clearer, it's also faster because there isn't a long line of "do you >> understand this subop?" calls when we know exactly what it is. >> >> However, I think we need stub for the !HAS_PASSTHROUGH case now that it >> is being called from common code. > > Okay, I'll add it. Jan, are you fine with a stub? Sure. Jan ^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2022-04-19 16:12 UTC | newest] Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2022-04-19 13:52 [PATCH v3 0/3] xen: fix and cleanup domctl handling Juergen Gross 2022-04-19 13:52 ` [PATCH v3 1/3] xen: fix XEN_DOMCTL_gdbsx_guestmemio crash Juergen Gross 2022-04-19 14:32 ` Jan Beulich 2022-04-19 14:37 ` Andrew Cooper 2022-04-19 14:50 ` Juergen Gross 2022-04-19 13:52 ` [PATCH v3 2/3] xen: cleanup gdbsx_guest_mem_io() call Juergen Gross 2022-04-19 14:32 ` Jan Beulich 2022-04-19 14:41 ` Andrew Cooper 2022-04-19 13:52 ` [PATCH v3 3/3] xen/iommu: cleanup iommu related domctl handling Juergen Gross 2022-04-19 14:41 ` Jan Beulich 2022-04-19 14:51 ` Andrew Cooper 2022-04-19 14:56 ` Juergen Gross 2022-04-19 16:11 ` Jan Beulich
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.