Re: [PATCH] ioreq: cope with server disappearing while I/O is pending

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 05/10/2020 15:08, Paul Durrant wrote:
> From: Paul Durrant <pdurrant@xxxxxxxxxx>
>
> Currently, in the event of an ioreq server being destroyed while I/O is
> pending in the attached emulator, it is possible that hvm_wait_for_io() will
> dereference a pointer to a 'struct hvm_ioreq_vcpu' or the ioreq server's
> shared page after it has been freed.

It's not legitimate for the shared page to be freed while Xen is still
using it.

Furthermore, this patch only covers the most obvious place for the bug
to manifest.  It doesn't fix them all, as a ioreq server destroy can
race with an in-progress emulation and still suffer a UAF.


An extra ref needs holding on the shared page between acquire_resource
and domain destruction, to cover Xen's use of the page.

Similarly, I don't think it is legitimate for hvm_ioreq_vcpu to be freed
while potentially in use.  These need to stick around until domain
destruction as well, I think.

> This will only occur if the emulator (which is necessarily running in a
> service domain with some degree of privilege) does not complete pending I/O
> during tear-down and is not directly exploitable by a guest domain.
>
> This patch adds a call to get_pending_vcpu() into the condition of the
> wait_on_xen_event_channel() macro to verify the continued existence of the
> ioreq server. Should it disappear, the guest domain will be crashed.
>
> NOTE: take the opportunity to modify the text on one gdprintk() for
>       consistency with others.

I know this is tangential, but all these gdprintk()'s need to change to
gprintk()'s, so release builds provide at least some hint as to why the
domain has been crashed.

(And I also really need to finish off my domain_crash() patch to force
this everywhere.)

~Andrew