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

[Thomas Huth <thuth@redhat.com>]

On 04/08/2022 12.17, Peter Maydell wrote:
> On Thu, 4 Aug 2022 at 11:07, Thomas Huth <thuth@redhat.com> wrote:
>>
>> On 04/08/2022 10.56, Peter Maydell wrote:
>>> 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)?
>>
>> Oh, wait, I think we were maybe looking at different spots. The problem
>> likely does not occur in the xhci_ring_fetch() function
>> (which you were likely looking at), but only in the xhci_ring_chain_length()
>> function (which I was looking at)!
>> xhci_ring_chain_length() can certainly continue more than 32 times. In
>> xhci_ring_chain_length() the TRB_LINK_LIMIT only applies if "type ==
>> TR_LINK", but the TRBs we're talking about here are *not* of type TR_LINK.
> 
> That sounds like we do still have an unbounded-loop problem,
> then: there's no limit on the number of consecutive TRBs
> we try to read in that function. Maybe we're missing an
> error check of some kind (does the spec limit how many
> consecutive TRBs there can be somehow?) or else we need
> another artificial limit.

I'm not an XHCI expert at all, but while at least having a quick glance at 
the spec, I did not see any limit there. So I assume that we should enforce 
an artificial limit? What would be a good value for this? Gerd, do you maybe 
have any opinion?

  Thomas