From patchwork Tue May 11 07:03:56 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Wesley Cheng X-Patchwork-Id: 436098 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-16.8 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID, HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_CR_TRAILER, INCLUDES_PATCH, MAILING_LIST_MULTI, SPF_HELO_NONE, SPF_PASS, URIBL_BLOCKED, USER_AGENT_GIT autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 217A5C433ED for ; Tue, 11 May 2021 07:04:02 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id EB3B661920 for ; Tue, 11 May 2021 07:04:01 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S230295AbhEKHFH (ORCPT ); Tue, 11 May 2021 03:05:07 -0400 Received: from m43-7.mailgun.net ([69.72.43.7]:64869 "EHLO m43-7.mailgun.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229931AbhEKHFG (ORCPT ); Tue, 11 May 2021 03:05:06 -0400 DKIM-Signature: a=rsa-sha256; v=1; c=relaxed/relaxed; d=mg.codeaurora.org; q=dns/txt; s=smtp; t=1620716640; h=Message-Id: Date: Subject: Cc: To: From: Sender; bh=/3St8vspwlaMJv2fv4wvq0SBdS7FrQGyxt0HzhIqFZQ=; b=evJz1WBAmaE65sicfxv55Go3XIlGsfoEF6fzbmNuQzJR9Q8n/7FrYDumVDnTSPgC82zk2doq 7HvDVcrxnM6gnO/4gJoADmv8/bMV3ogHocdAUMIl3fnBv6C5J7B3Tyf2C/9NwPjZLai/XxZN TEzvPJX5H2uQ5wW+1axNTK1o4t0= X-Mailgun-Sending-Ip: 69.72.43.7 X-Mailgun-Sid: WyIxZTE2YSIsICJsaW51eC11c2JAdmdlci5rZXJuZWwub3JnIiwgImJlOWU0YSJd Received: from smtp.codeaurora.org (ec2-35-166-182-171.us-west-2.compute.amazonaws.com [35.166.182.171]) by smtp-out-n04.prod.us-west-2.postgun.com with SMTP id 609a2c60da4b8b1332fca688 (version=TLS1.2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256); Tue, 11 May 2021 07:04:00 GMT Sender: wcheng=codeaurora.org@mg.codeaurora.org Received: by smtp.codeaurora.org (Postfix, from userid 1001) id 1E8C8C433D3; Tue, 11 May 2021 07:04:00 +0000 (UTC) Received: from wcheng-linux.qualcomm.com (i-global254.qualcomm.com [199.106.103.254]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-SHA256 (128/128 bits)) (No client certificate requested) (Authenticated sender: wcheng) by smtp.codeaurora.org (Postfix) with ESMTPSA id B40BAC43145; Tue, 11 May 2021 07:03:58 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 smtp.codeaurora.org B40BAC43145 Authentication-Results: aws-us-west-2-caf-mail-1.web.codeaurora.org; dmarc=none (p=none dis=none) header.from=codeaurora.org Authentication-Results: aws-us-west-2-caf-mail-1.web.codeaurora.org; spf=fail smtp.mailfrom=wcheng@codeaurora.org From: Wesley Cheng To: balbi@kernel.org, gregkh@linuxfoundation.org, peter.chen@kernel.org Cc: linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org, jackp@codeaurora.org, Wesley Cheng Subject: [PATCH v2] usb: dwc3: gadget: Replace list_for_each_entry_safe() if using giveback Date: Tue, 11 May 2021 00:03:56 -0700 Message-Id: <1620716636-12422-1-git-send-email-wcheng@codeaurora.org> X-Mailer: git-send-email 2.7.4 Precedence: bulk List-ID: X-Mailing-List: linux-usb@vger.kernel.org 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 Fixes: d4f1afe5e896 ("usb: dwc3: gadget: move requests to cancelled_list") Signed-off-by: Wesley Cheng Reviewed-by: Peter Chen --- Changes in v2: - Updated commit message with context call stack of an example scenario seen on device. drivers/usb/dwc3/gadget.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c index dd80e5c..efa939b 100644 --- a/drivers/usb/dwc3/gadget.c +++ b/drivers/usb/dwc3/gadget.c @@ -1737,10 +1737,10 @@ static void dwc3_gadget_ep_skip_trbs(struct dwc3_ep *dep, struct dwc3_request *r static void dwc3_gadget_ep_cleanup_cancelled_requests(struct dwc3_ep *dep) { struct dwc3_request *req; - struct dwc3_request *tmp; struct dwc3 *dwc = dep->dwc; - list_for_each_entry_safe(req, tmp, &dep->cancelled_list, list) { + while (!list_empty(&dep->cancelled_list)) { + req = next_request(&dep->cancelled_list); dwc3_gadget_ep_skip_trbs(dep, req); switch (req->status) { case DWC3_REQUEST_STATUS_DISCONNECTED: @@ -2935,11 +2935,11 @@ static void dwc3_gadget_ep_cleanup_completed_requests(struct dwc3_ep *dep, const struct dwc3_event_depevt *event, int status) { struct dwc3_request *req; - struct dwc3_request *tmp; - list_for_each_entry_safe(req, tmp, &dep->started_list, list) { + while (!list_empty(&dep->started_list)) { int ret; + req = next_request(&dep->started_list); ret = dwc3_gadget_ep_cleanup_completed_request(dep, event, req, status); if (ret)