* [PATCH] ioreq: don't (deliberately) crash Dom0
@ 2021-02-01 15:22 Jan Beulich
2021-02-01 15:37 ` Andrew Cooper
0 siblings, 1 reply; 3+ messages in thread
From: Jan Beulich @ 2021-02-01 15:22 UTC (permalink / raw)
To: xen-devel; +Cc: Paul Durrant, Andrew Cooper, Ian Jackson
We consider this error path of hvm_alloc_ioreq_mfn() to not be possible
to be taken, or otherwise to indicate abuse or a bug somewhere. If there
is abuse of some kind, crashing Dom0 here would mean a system-wide DoS.
Only crash the emulator domain if it's not the (global) control domain;
crash only the guest being serviced otherwise.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
--- a/xen/common/ioreq.c
+++ b/xen/common/ioreq.c
@@ -274,7 +274,7 @@ static int hvm_alloc_ioreq_mfn(struct hv
* The domain can't possibly know about this page yet, so failure
* here is a clear indication of something fishy going on.
*/
- domain_crash(s->emulator);
+ domain_crash(is_control_domain(s->emulator) ? s->target : s->emulator);
return -ENODATA;
}
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] ioreq: don't (deliberately) crash Dom0
2021-02-01 15:22 [PATCH] ioreq: don't (deliberately) crash Dom0 Jan Beulich
@ 2021-02-01 15:37 ` Andrew Cooper
2021-02-01 16:03 ` Jan Beulich
0 siblings, 1 reply; 3+ messages in thread
From: Andrew Cooper @ 2021-02-01 15:37 UTC (permalink / raw)
To: Jan Beulich, xen-devel; +Cc: Paul Durrant, Ian Jackson
On 01/02/2021 15:22, Jan Beulich wrote:
> We consider this error path of hvm_alloc_ioreq_mfn() to not be possible
> to be taken, or otherwise to indicate abuse or a bug somewhere. If there
> is abuse of some kind, crashing Dom0 here would mean a system-wide DoS.
> Only crash the emulator domain if it's not the (global) control domain;
> crash only the guest being serviced otherwise.
>
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
Honestly, I'm -1 towards this.
Asymmetrically shooting things which aren't dom0 only complicates
investigations, and doesn't remove the fact that this is an XSA.
I do not subscribe to the opinion that keeping dom0 running at all
possible costs is the best thing thing for the system.
In this particular case, the theoretical cases where it can go wrong
might not be the fault of either domain.
~Andrew
>
> --- a/xen/common/ioreq.c
> +++ b/xen/common/ioreq.c
> @@ -274,7 +274,7 @@ static int hvm_alloc_ioreq_mfn(struct hv
> * The domain can't possibly know about this page yet, so failure
> * here is a clear indication of something fishy going on.
> */
> - domain_crash(s->emulator);
> + domain_crash(is_control_domain(s->emulator) ? s->target : s->emulator);
> return -ENODATA;
> }
>
>
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] ioreq: don't (deliberately) crash Dom0
2021-02-01 15:37 ` Andrew Cooper
@ 2021-02-01 16:03 ` Jan Beulich
0 siblings, 0 replies; 3+ messages in thread
From: Jan Beulich @ 2021-02-01 16:03 UTC (permalink / raw)
To: Andrew Cooper; +Cc: Paul Durrant, Ian Jackson, xen-devel
On 01.02.2021 16:37, Andrew Cooper wrote:
> On 01/02/2021 15:22, Jan Beulich wrote:
>> We consider this error path of hvm_alloc_ioreq_mfn() to not be possible
>> to be taken, or otherwise to indicate abuse or a bug somewhere. If there
>> is abuse of some kind, crashing Dom0 here would mean a system-wide DoS.
>> Only crash the emulator domain if it's not the (global) control domain;
>> crash only the guest being serviced otherwise.
>>
>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
>
> Honestly, I'm -1 towards this.
>
> Asymmetrically shooting things which aren't dom0 only complicates
> investigations, and doesn't remove the fact that this is an XSA.
>
> I do not subscribe to the opinion that keeping dom0 running at all
> possible costs is the best thing thing for the system.
>
> In this particular case, the theoretical cases where it can go wrong
> might not be the fault of either domain.
I agree with "might", but I don't think we should consider buggy
Xen as the first option for what errors mean. As said on another
related thread, failure here could come from the page having got
freed very quickly (by guessing its MFN). If could be guest,
emulator, or yet some other domain's misbehavior. In none of
these cases I consider it appropriate to kill Dom0. The guest
not starting (or crashing, if this is dynamic insertion of an
ioreq server) is a clear enough sign that there's an issue that
needs looking into. No need to also penalize all other domains
running on that host.
Jan
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2021-02-01 16:03 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-02-01 15:22 [PATCH] ioreq: don't (deliberately) crash Dom0 Jan Beulich
2021-02-01 15:37 ` Andrew Cooper
2021-02-01 16:03 ` 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.