* [PATCH for-4.5] x86/hvm: do not create ioreq server if guest domain is dying
@ 2014-09-26 12:22 Vitaly Kuznetsov
2014-09-26 12:29 ` Paul Durrant
0 siblings, 1 reply; 3+ messages in thread
From: Vitaly Kuznetsov @ 2014-09-26 12:22 UTC (permalink / raw)
To: xen-devel; +Cc: Andrew Jones, Paul Durrant, Ian Campbell, Jan Beulich
If HVM_PARAM_IOREQ_PFN, HVM_PARAM_BUFIOREQ_PFN, or HVM_PARAM_BUFIOREQ_EVTCHN
parameters are read when guest domain is dying it leads to the following
ASSERT:
(XEN) Assertion '_raw_spin_is_locked(lock)' failed at ...workspace/KERNEL/xen/xen/include/asm/spinlock.h:18
(XEN) ----[ Xen-4.5-unstable x86_64 debug=y Not tainted ]----
...
(XEN) Xen call trace:
(XEN) [<ffff82d08012b07f>] _spin_unlock+0x27/0x30
(XEN) [<ffff82d0801b6103>] hvm_create_ioreq_server+0x3df/0x49a
(XEN) [<ffff82d0801bcceb>] do_hvm_op+0x12bf/0x27a0
(XEN) [<ffff82d08022b9bb>] syscall_enter+0xeb/0x145
It doesn't make sense (and is unsafe) to create ioreq server if we're dying. Make
hvm_create_ioreq_server() fail with -EFAULT in this case.
Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
---
xen/arch/x86/hvm/hvm.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
index 0a20cbe..2cc6de7 100644
--- a/xen/arch/x86/hvm/hvm.c
+++ b/xen/arch/x86/hvm/hvm.c
@@ -1038,6 +1038,9 @@ static int hvm_create_ioreq_server(struct domain *d, domid_t domid,
struct hvm_ioreq_server *s;
int rc;
+ if ( d->is_dying )
+ return -EFAULT;
+
rc = -ENOMEM;
s = xzalloc(struct hvm_ioreq_server);
if ( !s )
--
1.9.3
^ permalink raw reply related [flat|nested] 3+ messages in thread* Re: [PATCH for-4.5] x86/hvm: do not create ioreq server if guest domain is dying
2014-09-26 12:22 [PATCH for-4.5] x86/hvm: do not create ioreq server if guest domain is dying Vitaly Kuznetsov
@ 2014-09-26 12:29 ` Paul Durrant
2014-09-26 13:08 ` Vitaly Kuznetsov
0 siblings, 1 reply; 3+ messages in thread
From: Paul Durrant @ 2014-09-26 12:29 UTC (permalink / raw)
To: Vitaly Kuznetsov, xen-devel@lists.xenproject.org
Cc: Andrew Jones, Ian Campbell, Jan Beulich
> -----Original Message-----
> From: Vitaly Kuznetsov [mailto:vkuznets@redhat.com]
> Sent: 26 September 2014 13:23
> To: xen-devel@lists.xenproject.org
> Cc: Paul Durrant; Ian Campbell; Jan Beulich; Andrew Jones
> Subject: [PATCH for-4.5] x86/hvm: do not create ioreq server if guest domain
> is dying
>
> If HVM_PARAM_IOREQ_PFN, HVM_PARAM_BUFIOREQ_PFN, or
> HVM_PARAM_BUFIOREQ_EVTCHN
> parameters are read when guest domain is dying it leads to the following
> ASSERT:
>
> (XEN) Assertion '_raw_spin_is_locked(lock)' failed at
> ...workspace/KERNEL/xen/xen/include/asm/spinlock.h:18
> (XEN) ----[ Xen-4.5-unstable x86_64 debug=y Not tainted ]----
> ...
> (XEN) Xen call trace:
> (XEN) [<ffff82d08012b07f>] _spin_unlock+0x27/0x30
> (XEN) [<ffff82d0801b6103>] hvm_create_ioreq_server+0x3df/0x49a
> (XEN) [<ffff82d0801bcceb>] do_hvm_op+0x12bf/0x27a0
> (XEN) [<ffff82d08022b9bb>] syscall_enter+0xeb/0x145
>
> It doesn't make sense (and is unsafe) to create ioreq server if we're dying.
> Make
> hvm_create_ioreq_server() fail with -EFAULT in this case.
>
> Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
> ---
> xen/arch/x86/hvm/hvm.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
> index 0a20cbe..2cc6de7 100644
> --- a/xen/arch/x86/hvm/hvm.c
> +++ b/xen/arch/x86/hvm/hvm.c
> @@ -1038,6 +1038,9 @@ static int hvm_create_ioreq_server(struct domain
> *d, domid_t domid,
> struct hvm_ioreq_server *s;
> int rc;
>
> + if ( d->is_dying )
> + return -EFAULT;
> +
Whilst I agree that it makes no sense to be creating in ioreq server in this case, this patch is not an actual fix for the bug.
The bug AFAICT is actually a stray spin_unlock() in the fail1 error case in hvm_ioreq_server_init().
Paul
> rc = -ENOMEM;
> s = xzalloc(struct hvm_ioreq_server);
> if ( !s )
> --
> 1.9.3
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH for-4.5] x86/hvm: do not create ioreq server if guest domain is dying
2014-09-26 12:29 ` Paul Durrant
@ 2014-09-26 13:08 ` Vitaly Kuznetsov
0 siblings, 0 replies; 3+ messages in thread
From: Vitaly Kuznetsov @ 2014-09-26 13:08 UTC (permalink / raw)
To: Paul Durrant
Cc: xen-devel@lists.xenproject.org, Andrew Jones, Ian Campbell,
Jan Beulich
Paul Durrant <Paul.Durrant@citrix.com> writes:
>> -----Original Message-----
>> From: Vitaly Kuznetsov [mailto:vkuznets@redhat.com]
>> Sent: 26 September 2014 13:23
>> To: xen-devel@lists.xenproject.org
>> Cc: Paul Durrant; Ian Campbell; Jan Beulich; Andrew Jones
>> Subject: [PATCH for-4.5] x86/hvm: do not create ioreq server if guest domain
>> is dying
>>
>> If HVM_PARAM_IOREQ_PFN, HVM_PARAM_BUFIOREQ_PFN, or
>> HVM_PARAM_BUFIOREQ_EVTCHN
>> parameters are read when guest domain is dying it leads to the following
>> ASSERT:
>>
>> (XEN) Assertion '_raw_spin_is_locked(lock)' failed at
>> ...workspace/KERNEL/xen/xen/include/asm/spinlock.h:18
>> (XEN) ----[ Xen-4.5-unstable x86_64 debug=y Not tainted ]----
>> ...
>> (XEN) Xen call trace:
>> (XEN) [<ffff82d08012b07f>] _spin_unlock+0x27/0x30
>> (XEN) [<ffff82d0801b6103>] hvm_create_ioreq_server+0x3df/0x49a
>> (XEN) [<ffff82d0801bcceb>] do_hvm_op+0x12bf/0x27a0
>> (XEN) [<ffff82d08022b9bb>] syscall_enter+0xeb/0x145
>>
>> It doesn't make sense (and is unsafe) to create ioreq server if we're dying.
>> Make
>> hvm_create_ioreq_server() fail with -EFAULT in this case.
>>
>> Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
>> ---
>> xen/arch/x86/hvm/hvm.c | 3 +++
>> 1 file changed, 3 insertions(+)
>>
>> diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
>> index 0a20cbe..2cc6de7 100644
>> --- a/xen/arch/x86/hvm/hvm.c
>> +++ b/xen/arch/x86/hvm/hvm.c
>> @@ -1038,6 +1038,9 @@ static int hvm_create_ioreq_server(struct domain
>> *d, domid_t domid,
>> struct hvm_ioreq_server *s;
>> int rc;
>>
>> + if ( d->is_dying )
>> + return -EFAULT;
>> +
>
> Whilst I agree that it makes no sense to be creating in ioreq server in this case, this patch is not an actual fix for the bug.
>
> The bug AFAICT is actually a stray spin_unlock() in the fail1 error case in hvm_ioreq_server_init().
>
Ah, yea, you're right, hvm_ioreq_server_init() is the root cause. I'll
test and repost.
Thanks!
> Paul
>
>> rc = -ENOMEM;
>> s = xzalloc(struct hvm_ioreq_server);
>> if ( !s )
>> --
>> 1.9.3
--
Vitaly
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2014-09-26 13:08 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-09-26 12:22 [PATCH for-4.5] x86/hvm: do not create ioreq server if guest domain is dying Vitaly Kuznetsov
2014-09-26 12:29 ` Paul Durrant
2014-09-26 13:08 ` Vitaly Kuznetsov
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).