Re: [PATCH] hw/usb/hcd-xhci: Fix endless loop in case the DMA access fails (CVE-2020-14394)

[Peter Maydell]

On Thu, 4 Aug 2022 at 09:00, Thomas Huth <thuth@redhat.com> wrote:
>
> On 02/08/2022 16.09, Peter Maydell wrote:
> > On Tue, 2 Aug 2022 at 14:53, Thomas Huth <thuth@redhat.com> wrote:
> >>
> >> The XHCI code could enter an endless loop in case the guest points
> >> QEMU to fetch TRBs from invalid memory areas. Fix it by properly
> >> checking the return value of dma_memory_read().
> >
> > It certainly makes sense to check the return value from
> > dma_memory_read(), but how can we end up in an infinite loop
> > if it fails? Either:
> >
> > (1) there is some combination of data values which allow an
> > infinite loop, so we can hit those if the DMA access fails and
> > we use bogus uninitialized data. But then the guest could
> > maliciously pass us those bad values and create an infinite
> > loop without a failing DMA access, ie commit 05f43d44e4b
> > missed a case. If so we need to fix that!
>
> No, as far as I can see, that's not the case.
>
> > Or (2) there isn't actually an infinite loop possible here,
> > and we're just tidying up a case which is C undefined
> > behaviour but in practice unlikely to have effects any
> > worse than the guest could provoke anyway (ie running up
> > to the TRB_LINK_LIMIT and stopping). In this case the
> > commit message here is a bit misleading.
>
> So from what I understand, this is happening: The guest somehow manages to
> trick QEMU in a state where it reads through a set of TRBs where none is
> marked in a way that could cause the function to return. QEMU then reads all
> memory 'till the end of RAM and then continues to loop through no-mans-land
> since the return value of dma_memory_read() is not checked.

But the point of TRB_LINK_LIMIT is that regardless of what the
contents of the TRBs are, the loop is not supposed to
be able to continue for more than TRB_LINK_LIMIT iterations,
ie 32 times. In this example case, do we stop after 32 TRBs
(case 2) or not (case 1)?

thanks
-- PMM