linux-kernel - [PATCH v3] usb: dwc3: gadget: trb

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-next>] [day] [month] [year] [list]

Message-Id: <1563497183-7114-1-git-send-email-fei.yang@intel.com>
Date:   Thu, 18 Jul 2019 17:46:23 -0700
From:   fei.yang@...el.com
To:     felipe.balbi@...ux.intel.com, john.stultz@...aro.org,
        andrzej.p@...labora.com, linux-usb@...r.kernel.org,
        linux-kernel@...r.kernel.org, gregkh@...uxfoundation.org,
        stable@...r.kernel.org
Subject: [PATCH v3] usb: dwc3: gadget: trb_dequeue is not updated properly

From: Fei Yang <fei.yang@...el.com>

If scatter-gather operation is allowed, a large USB request is split into
multiple TRBs. These TRBs are chained up by setting DWC3_TRB_CTRL_CHN bit
except the last one which has DWC3_TRB_CTRL_IOC bit set instead.
Since only the last TRB has IOC set for the whole USB request, the
dwc3_gadget_ep_reclaim_trb_sg() gets called only once after the last TRB
completes and all the TRBs allocated for this request are supposed to be
reclaimed. However that is not what the current code does.

dwc3_gadget_ep_reclaim_trb_sg() is trying to reclaim all the TRBs in the
following for-loop,
	for_each_sg(sg, s, pending, i) {
		trb = &dep->trb_pool[dep->trb_dequeue];

                if (trb->ctrl & DWC3_TRB_CTRL_HWO)
                        break;

                req->sg = sg_next(s);
                req->num_pending_sgs--;

                ret = dwc3_gadget_ep_reclaim_completed_trb(dep, req,
                                trb, event, status, chain);
                if (ret)
                        break;
        }
but since the interrupt comes only after the last TRB completes, the
event->status has DEPEVT_STATUS_IOC bit set, so that the for-loop ends for
the first TRB due to dwc3_gadget_ep_reclaim_completed_trb() returns 1.
	if (event->status & DEPEVT_STATUS_IOC)
		return 1;

This patch addresses the issue by checking each TRB in function
dwc3_gadget_ep_reclaim_trb_sg() and maing sure the chained ones are properly
reclaimed. dwc3_gadget_ep_reclaim_completed_trb() will return 1 Only for the
last TRB.

Signed-off-by: Fei Yang <fei.yang@...el.com>
Cc: stable <stable@...r.kernel.org>
---
v2: Better solution is to reclaim chained TRBs in dwc3_gadget_ep_reclaim_trb_sg()
    and leave the last TRB to the dwc3_gadget_ep_reclaim_completed_trb().
v3: Checking DWC3_TRB_CTRL_CHN bit for each TRB instead, and making sure that
    dwc3_gadget_ep_reclaim_completed_trb() returns 1 only for the last TRB.
---
 drivers/usb/dwc3/gadget.c | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
index 173f532..88eed49 100644
--- a/drivers/usb/dwc3/gadget.c
+++ b/drivers/usb/dwc3/gadget.c
@@ -2394,7 +2394,7 @@ static int dwc3_gadget_ep_reclaim_completed_trb(struct dwc3_ep *dep,
 	if (event->status & DEPEVT_STATUS_SHORT && !chain)
 		return 1;
 
-	if (event->status & DEPEVT_STATUS_IOC)
+	if (event->status & DEPEVT_STATUS_IOC && !chain)
 		return 1;
 
 	return 0;
@@ -2404,11 +2404,12 @@ static int dwc3_gadget_ep_reclaim_trb_sg(struct dwc3_ep *dep,
 		struct dwc3_request *req, const struct dwc3_event_depevt *event,
 		int status)
 {
-	struct dwc3_trb *trb = &dep->trb_pool[dep->trb_dequeue];
+	struct dwc3_trb *trb;
 	struct scatterlist *sg = req->sg;
 	struct scatterlist *s;
 	unsigned int pending = req->num_pending_sgs;
 	unsigned int i;
+	int chain = false;
 	int ret = 0;
 
 	for_each_sg(sg, s, pending, i) {
@@ -2419,9 +2420,13 @@ static int dwc3_gadget_ep_reclaim_trb_sg(struct dwc3_ep *dep,
 
 		req->sg = sg_next(s);
 		req->num_pending_sgs--;
+		if (trb->ctrl & DWC3_TRB_CTRL_CHN)
+			chain = true;
+		else
+			chain = false;
 
 		ret = dwc3_gadget_ep_reclaim_completed_trb(dep, req,
-				trb, event, status, true);
+				trb, event, status, chain);
 		if (ret)
 			break;
 	}
-- 
2.7.4