Re: [Qemu-devel] (for 2.6?) [PATCH] usb:xhci: no DMA on HC reset

["Denis V. Lunev" <den@openvz.org>]

On 05/04/2016 08:53 PM, Roman Kagan wrote:
> This patch is a rough fix to a memory corruption we are observing when
> running VMs with xhci USB controller and OVMF firmware.
>
> Specifically, on the following call chain
>
> xhci_reset
>    xhci_disable_slot
>      xhci_disable_ep
>        xhci_set_ep_state
>
> QEMU overwrites guest memory using stale guest addresses.
>
> This doesn't happen when the guest (firmware) driver sets up xhci for
> the first time as there are no slots configured yet.  However when the
> firmware hands over the control to the OS some slots and endpoints are
> already set up with their context in the guest RAM.  Now the OS' driver
> resets the controller again and xhci_set_ep_state then reads and writes
> that memory which is now owned by the OS.
>
> As a quick fix, skip calling xhci_set_ep_state in xhci_disable_ep if the
> device context base address array pointer is zero (indicating we're in
> the HC reset and no DMA is possible).
>
> Signed-off-by: Roman Kagan <rkagan@virtuozzo.com>
> Cc: Gerd Hoffmann <kraxel@redhat.com>
> ---
>   hw/usb/hcd-xhci.c | 5 ++++-
>   1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/hw/usb/hcd-xhci.c b/hw/usb/hcd-xhci.c
> index bcde8a2..43ba615 100644
> --- a/hw/usb/hcd-xhci.c
> +++ b/hw/usb/hcd-xhci.c
> @@ -1531,7 +1531,10 @@ static TRBCCode xhci_disable_ep(XHCIState *xhci, unsigned int slotid,
>           usb_packet_cleanup(&epctx->transfers[i].packet);
>       }
>   
> -    xhci_set_ep_state(xhci, epctx, NULL, EP_DISABLED);
> +    /* only touch guest RAM if we're not resetting the HC */
> +    if (xhci->dcbaap_low || xhci->dcbaap_high) {
> +        xhci_set_ep_state(xhci, epctx, NULL, EP_DISABLED);
> +    }
>   
>       timer_free(epctx->kick_timer);
>       g_free(epctx);
the situation is quite reliable with win2k12 and EFI
guest. Kernel image gets corrupted and the guest
crashes on boot.

The problem is serious and really severe.

Den