[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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


  • To: Paul Durrant <paul@xxxxxxx>, <xen-devel@xxxxxxxxxxxxxxxxxxxx>
  • From: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
  • Date: Mon, 5 Oct 2020 15:30:24 +0100
  • Authentication-results: esa2.hc3370-68.iphmx.com; dkim=none (message not signed) header.i=none
  • Cc: Paul Durrant <pdurrant@xxxxxxxxxx>, Julien Grall <julien@xxxxxxx>, "Jan Beulich" <jbeulich@xxxxxxxx>, Roger Pau Monné <roger.pau@xxxxxxxxxx>, Wei Liu <wl@xxxxxxx>
  • Delivery-date: Mon, 05 Oct 2020 14:30:54 +0000
  • Ironport-sdr: 5NfeUyMmJ7ZCSCc4Fjo7RwASt7DO584VaS/tUzGSR6l0q66wM8YRzbBMBc/L4dr2ZoGzHV9K46 gpsz0C26MsWRARo4jm13Etv27veFRYpEFRaNwvKMypOjcejxw25b4Xl4yLjTywhvio334XnXlL THNQSe581rWt1w/fQ8HNW7j1p1giOYe0VNQ2SkUuG585k7XPCTheLeSzYgor0iezSCp15HH8lg 9AkI+3GYGCTyAvJt8iyoMCxNSNm2fIjS2oAgOGiL6oIUJgO1xrPEr7AA2dR05Yibx6m53qvvcc wV4=
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>

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



 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.