• Wesley Cheng's avatar
    usb: dwc3: gadget: Use list_replace_init() before traversing lists · d25d8506
    Wesley Cheng authored
    The list_for_each_entry_safe() macro saves the current item (n) and
    the item after (n+1), so that n can be safely removed without
    corrupting the list.  However, when traversing the list and removing
    items using gadget giveback, the DWC3 lock is briefly released,
    allowing other routines to execute.  There is a situation where, while
    items are being removed from the cancelled_list using
    dwc3_gadget_ep_cleanup_cancelled_requests(), the pullup disable
    routine is running in parallel (due to UDC unbind).  As the cleanup
    routine removes n, and the pullup disable removes n+1, once the
    cleanup retakes the DWC3 lock, it references a request who was already
    removed/handled.  With list debug enabled, this leads to a panic.
    Ensure all instances of the macro are replaced where gadget giveback
    is used.
    
    Example call stack:
    
    Thread#1:
    __dwc3_gadget_ep_set_halt() - CLEAR HALT
      -> dwc3_gadget_ep_cleanup_cancelled_requests()
        ->list_for_each_entry_safe()
        ->dwc3_gadget_giveback(n)
          ->dwc3_gadget_del_and_unmap_request()- n deleted[cancelled_list]
          ->spin_unlock
          ->Thread#2 executes
          ...
        ->dwc3_gadget_giveback(n+1)
          ->Already removed!
    
    Thread#2:
    dwc3_gadget_pullup()
      ->waiting for dwc3 spin_lock
      ...
      ->Thread#1 released lock
      ->dwc3_stop_active_transfers()
        ->dwc3_remove_requests()
          ->fetches n+1 item from cancelled_list (n removed by Thread#1)
          ->dwc3_gadget_giveback()
            ->dwc3_gadget_del_and_unmap_request()- n+1
    deleted[cancelled_list]
            ->spin_unlock
    
    Fix this condition by utilizing list_replace_init(), and traversing
    through a local copy of the current elements in the endpoint lists.
    This will also set the parent list as empty, so if another thread is
    also looping through the list, it will be empty on the next iteration.
    
    Fixes: d4f1afe5 ("usb: dwc3: gadget: move requests to cancelled_list")
    Cc: stable <stable@vger.kernel.org>
    Acked-by: default avatarFelipe Balbi <balbi@kernel.org>
    Signed-off-by: default avatarWesley Cheng <wcheng@codeaurora.org>
    Link: https://lore.kernel.org/r/1627543994-20327-1-git-send-email-wcheng@codeaurora.orgSigned-off-by: default avatarGreg Kroah-Hartman <gregkh@linuxfoundation.org>
    d25d8506
gadget.c 107 KB