linux-kernel - Re: [PATCH] USB: Fix xhci ERDP update issue

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Wed, 16 Mar 2022 14:45:27 +0200
From:   Mathias Nyman <mathias.nyman@...ux.intel.com>
To:     Peter Chen <hzpeterchen@...il.com>,
        Greg KH <gregkh@...uxfoundation.org>
Cc:     "WeitaoWang-oc@...oxin.com" <WeitaoWang-oc@...oxin.com>,
        mathias.nyman@...el.com, Alan Stern <stern@...land.harvard.edu>,
        USB list <linux-usb@...r.kernel.org>,
        lkml <linux-kernel@...r.kernel.org>, CobeChen@...oxin.com,
        TimGuo@...oxin.com, tonywwang@...oxin.com, weitaowang@...oxin.com
Subject: Re: [PATCH] USB: Fix xhci ERDP update issue

On 16.3.2022 13.57, Peter Chen wrote:
> On Mon, Mar 14, 2022 at 10:34 PM Greg KH <gregkh@...uxfoundation.org> wrote:
>>
>> On Mon, Mar 14, 2022 at 03:25:23PM +0800, WeitaoWang-oc@...oxin.com wrote:
>>> On some situations, software handles TRB events slower than adding TRBs,
>>> xhci_irq will not exit until all events are handled. If xhci_irq just
>>> handles 256 TRBs and exit, the temp variable(event_ring_deq) driver records
>>> in xhci irq is equal to driver current dequeue pointer. It will cause driver
>>> not update ERDP and software dequeue pointer lost sync with ERDP. On the
>>> next xhci_irq, the event ring is full but driver will not update ERDP as
>>> software dequeue pointer is equal to ERDP.
> 
> At the current driver, the ERDP is updated at most 128 TRBs, how is
> the above condition
> triggered?
> 
> Peter
> 

Before, and during _one_ interrupt handling xHC hardware writes exactly 256 events
to event ring. ring buffer size is 256 so buffer position 0 and 256 point
to the same place.

Interrupt handler stores software dequeue in a local variable "event_ring_deq".
Handler start handling events, it updates software dequeue, but not local variable.
After 128 events handler updates hardware ERDP.

So at event 128 we got:
Hardware ERDP = 128
software dequeue = 128
event_ring_deq = 0

Handler continue handling events, at event 256 try to update HW ERDP again, but fail due
to this condition in update_erst_dequeue():
      if (event_ring_deq != xhci->event_ring->dequeue)

This fails because event_ring_deq is still 0, and software deq is 256,
pointing to the same place in the event ring.

So at the end of the interrupt handler we have:
HW ERDP = 128
software dequeue = 256 (same as 0)

So in this specific case we fail to update ERDP correctly

-Mathias