From mboxrd@z Thu Jan 1 00:00:00 1970 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: base64 Subject: [4.9,086/148] usb: gadget: udc: net2280: Fix tmp reusage in net2280 driver From: Greg Kroah-Hartman Message-Id: <20171212124436.148277723@linuxfoundation.org> Date: Tue, 12 Dec 2017 13:44:56 +0100 To: linux-kernel@vger.kernel.org Cc: Greg Kroah-Hartman , stable@vger.kernel.org, Felipe Balbi , linux-usb@vger.kernel.org, Raz Manor , Felipe Balbi , Sasha Levin List-ID: NC45LXN0YWJsZSByZXZpZXcgcGF0Y2guICBJZiBhbnlvbmUgaGFzIGFueSBvYmplY3Rpb25zLCBw bGVhc2UgbGV0IG1lIGtub3cuCgotLS0tLS0tLS0tLS0tLS0tLS0KCkZyb206IFJheiBNYW5vciA8 UmF6Lk1hbm9yQHZhbGVucy5jb20+CgoKWyBVcHN0cmVhbSBjb21taXQgZWY1ZTJmYTlmNjViZWZh MTJmMTExM2M3MzQ2MDJkMmMxOTY0ZDJhNSBdCgpJbiB0aGUgZnVuY3Rpb24gc2Nhbl9kbWFfY29t cGxldGlvbnMoKSB0aGVyZSBpcyBhIHJldXNhZ2Ugb2YgdG1wCnZhcmlhYmxlLiBUaGF0IGNvdXNl ZCBhIHdyb25nIHZhbHVlIGJlaW5nIHVzZWQgaW4gc29tZSBjYXNlIHdoZW4KcmVhZGluZyBhIHNo b3J0IHBhY2tldCB0ZXJtaW5hdGVkIHRyYW5zYWN0aW9uIGZyb20gYW4gZW5kcG9pbnQsCmluIDIg Y29uY2VjdXRpdmUgcmVhZHMuCgpUaGlzIHdhcyBteSBsb2dpYyBmb3IgdGhlIHBhdGNoOgoKVGhl IHJlcS0+dGQtPmRtYWRlc2MgZXF1YWxzIHRvIDAgaWZmOgotLSBUaGVyZSB3YXMgYSB0cmFuc2Fj dGlvbiBlbmRpbmcgd2l0aCBhIHNob3J0IHBhY2tldCwgYW5kCi0tIFRoZSByZWFkKCkgdG8gcmVh ZCBpdCB3YXMgc2hvcnRlciB0aGFuIHRoZSB0cmFuc2FjdGlvbiBsZW5ndGgsIGFuZAotLSBUaGUg cmVhZCgpIHRvIGNvbXBsZXRlIGl0IGlzIGxvbmdlciB0aGFuIHRoZSByZXNpZHVlLgpJIGJlbGll dmUgdGhpcyBpcyB0cnVlIGZyb20gdGhlIHByaW50b3V0cyBvZiB2YXJpb3VzIGNhc2VzLApidXQg SSBjYW4ndCBiZSBwb3NpdGl2ZSBpdCBpcyBjb3JyZWN0LgoKRW50ZXJpbmcgdGhpcyBpZiwgdGhl cmUgc2hvdWxkIGJlIG5vIG1vcmUgZGF0YSBpbiB0aGUgZW5kcG9pbnQKKGEgc2hvcnQgcGFja2V0 IHRlcm1pbmF0ZWQgdGhlIHRyYW5zYWN0aW9uKS4KSWYgdGhlcmUgaXMsIHRoZSB0cmFuc2FjdGlv biB3YXNuJ3QgcmVhbGx5IGRvbmUgYW5kIHdlIHNob3VsZCBleGl0IGFuZAp3YWl0IGZvciBpdCB0 byBmaW5pc2ggZW50aXJlbHkuIFRoYXQgaXMgdGhlIGlubmVyIGlmLgpUaGF0IGlubmVyIGlmIHNo b3VsZCBuZXZlciBoYXBwZW4sIGJ1dCBpdCBpcyB0aGVyZSB0byBiZSBvbiB0aGUgc2FmZQpzaWRl LiBUaGF0IGlzIHdoeSBpdCBpcyBtYXJrZWQgd2l0aCB0aGUgY29tbWVudCAvKiBwYXJhbm9pYSAq Ly4KVGhlIHNpemUgb2YgdGhlIGRhdGEgYXZhaWxhYmxlIGluIHRoZSBlbmRwb2ludCBpcyBlcC0+ ZG1hLT5kbWFjb3VudAphbmQgaXQgaXMgcmVhZCB0byB0bXAuClRoaXMgZW50aXJlIGNsYXVzZSBp cyBiYXNlZCBvbiBteSBvd24gZWR1Y2F0ZWQgZ3Vlc3Nlcy4KCklmIHdlIHBhc3NlZCB0aGF0IGlu bmVyIGlmIHdpdGhvdXQgYnJlYWtpbmcgaW4gdGhlIG9yaWdpbmFsIGNvZGUsCnRoYW4gdG1wICYg RE1BX0JZVEVfTUFTS19DT1VOVD09IDAuClRoYXQgbWVhbnMgd2Ugd2lsbCBhbHdheXMgcGFzcyBk bWEgYnl0ZXMgY291bnQgb2YgMCB0byBkbWFfZG9uZSgpLAptZWFuaW5nIGFsbCB0aGUgcmVxdWVz dGVkIGJ5dGVzIHdlcmUgcmVhZC4KCmRtYV9kb25lKCkgcmVwb3J0cyBiYWNrIHRvIHRoZSB1cHBl ciBsYXllciB0aGF0IHRoZSByZXF1ZXN0IChyZWFkKCkpCndhcyBkb25lIGFuZCBob3cgbWFueSBi eXRlcyB3ZXJlIHJlYWQuCkluIHRoZSBvcmlnaW5hbCBjb2RlIHRoYXQgd291bGQgYWx3YXlzIGJl IHRoZSByZXF1ZXN0IHNpemUsCnJlZ2FyZGxlc3Mgb2YgdGhlIGFjdHVhbCBzaXplIG9mIHRoZSBk YXRhLgpUaGF0IGRpZCBub3QgbWFrZSBzZW5zZSB0byBtZSBhdCBhbGwuCgpIb3dldmVyLCB0aGUg b3JpZ2luYWwgdmFsdWUgb2YgdG1wIGlzIHJlcS0+dGQtPmRtYWNvdW50LAp3aGljaCBpcyB0aGUg ZG1hY291bnQgdmFsdWUgd2hlbiB0aGUgcmVxdWVzdCdzIGRtYSB0cmFuc2FjdGlvbiB3YXMKZmlu aXNoZWQuIEFuZCB0aGF0IGlzIGEgbXVjaCBtb3JlIHJlYXNvbmFibGUgdmFsdWUgdG8gcmVwb3J0 IGJhY2sgdG8KdGhlIGNhbGxlci4KClRvIHJlY3JlYXRlIHRoZSBwcm9ibGVtOgpSZWFkIGZyb20g YSBidWxrIG91dCBlbmRwb2ludCBpbiBhIGxvb3AsIDEwMjQgKiBuIGJ5dGVzIGluIGVhY2gKaXRl cmF0aW9uLgpDb25uZWN0IHRoZSBQTFggdG8gYSBob3N0IHlvdSBjYW4gY29udHJvbC4KU2VuZCB0 byB0aGF0IGVuZHBvaW50IDEwMjQgKiBuICsgeCBieXRlcywKc3VjaCB0aGF0IDAgPCB4IDwgMTAy NCAqIG4gYW5kICh4ICUgMTAyNCkgIT0gMApZb3Ugd291bGQgZXhwZWN0IHRoZSBmaXJzdCByZWFk KCkgdG8gcmV0dXJuIDEwMjQgKiBuCmFuZCB0aGUgc2Vjb25kIHJlYWQoKSB0byByZXR1cm4geC4K QnV0IHlvdSB3aWxsIGdldCB0aGUgZmlyc3QgcmVhZCB0byByZXR1cm4gMTAyNCAqIG4KYW5kIHRo ZSBzZWNvbmQgb25lIHRvIHJldHVybiAxMDI0ICogbi4KVGhhdCBpcyB0cnVlIGZvciBldmVyeSBw b3NpdGl2ZSBpbnRlZ2VyIG4uCgpDYzogRmVsaXBlIEJhbGJpIDxiYWxiaUBrZXJuZWwub3JnPgpD YzogR3JlZyBLcm9haC1IYXJ0bWFuIDxncmVna2hAbGludXhmb3VuZGF0aW9uLm9yZz4KQ2M6IGxp bnV4LXVzYkB2Z2VyLmtlcm5lbC5vcmcKU2lnbmVkLW9mZi1ieTogUmF6IE1hbm9yIDxSYXouTWFu b3JAdmFsZW5zLmNvbT4KU2lnbmVkLW9mZi1ieTogRmVsaXBlIEJhbGJpIDxmZWxpcGUuYmFsYmlA bGludXguaW50ZWwuY29tPgpTaWduZWQtb2ZmLWJ5OiBTYXNoYSBMZXZpbiA8YWxleGFuZGVyLmxl dmluQHZlcml6b24uY29tPgpTaWduZWQtb2ZmLWJ5OiBHcmVnIEtyb2FoLUhhcnRtYW4gPGdyZWdr aEBsaW51eGZvdW5kYXRpb24ub3JnPgotLS0KIGRyaXZlcnMvdXNiL2dhZGdldC91ZGMvbmV0MjI4 MC5jIHwgICAyNSArKysrKysrKysrKysrLS0tLS0tLS0tLS0tCiAxIGZpbGUgY2hhbmdlZCwgMTMg aW5zZXJ0aW9ucygrKSwgMTIgZGVsZXRpb25zKC0pCgoKCi0tClRvIHVuc3Vic2NyaWJlIGZyb20g dGhpcyBsaXN0OiBzZW5kIHRoZSBsaW5lICJ1bnN1YnNjcmliZSBsaW51eC11c2IiIGluCnRoZSBi b2R5IG9mIGEgbWVzc2FnZSB0byBtYWpvcmRvbW9Admdlci5rZXJuZWwub3JnCk1vcmUgbWFqb3Jk b21vIGluZm8gYXQgIGh0dHA6Ly92Z2VyLmtlcm5lbC5vcmcvbWFqb3Jkb21vLWluZm8uaHRtbAoK LS0tIGEvZHJpdmVycy91c2IvZ2FkZ2V0L3VkYy9uZXQyMjgwLmMKKysrIGIvZHJpdmVycy91c2Iv Z2FkZ2V0L3VkYy9uZXQyMjgwLmMKQEAgLTExNDYsMTUgKzExNDYsMTUgQEAgc3RhdGljIGludCBz Y2FuX2RtYV9jb21wbGV0aW9ucyhzdHJ1Y3QgbgogCSAqLwogCXdoaWxlICghbGlzdF9lbXB0eSgm ZXAtPnF1ZXVlKSkgewogCQlzdHJ1Y3QgbmV0MjI4MF9yZXF1ZXN0CSpyZXE7Ci0JCXUzMgkJCXRt cDsKKwkJdTMyIHJlcV9kbWFfY291bnQ7CiAKIAkJcmVxID0gbGlzdF9lbnRyeShlcC0+cXVldWUu bmV4dCwKIAkJCQlzdHJ1Y3QgbmV0MjI4MF9yZXF1ZXN0LCBxdWV1ZSk7CiAJCWlmICghcmVxLT52 YWxpZCkKIAkJCWJyZWFrOwogCQlybWIoKTsKLQkJdG1wID0gbGUzMl90b19jcHVwKCZyZXEtPnRk LT5kbWFjb3VudCk7Ci0JCWlmICgodG1wICYgQklUKFZBTElEX0JJVCkpICE9IDApCisJCXJlcV9k bWFfY291bnQgPSBsZTMyX3RvX2NwdXAoJnJlcS0+dGQtPmRtYWNvdW50KTsKKwkJaWYgKChyZXFf ZG1hX2NvdW50ICYgQklUKFZBTElEX0JJVCkpICE9IDApCiAJCQlicmVhazsKIAogCQkvKiBTSE9S VF9QQUNLRVRfVFJBTlNGRVJSRURfSU5URVJSVVBUIGhhbmRsZXMgInVzYi1zaG9ydCIKQEAgLTEx NjMsNDAgKzExNjMsNDEgQEAgc3RhdGljIGludCBzY2FuX2RtYV9jb21wbGV0aW9ucyhzdHJ1Y3Qg bgogCQkgKi8KIAkJaWYgKHVubGlrZWx5KHJlcS0+dGQtPmRtYWRlc2MgPT0gMCkpIHsKIAkJCS8q IHBhcmFub2lhICovCi0JCQl0bXAgPSByZWFkbCgmZXAtPmRtYS0+ZG1hY291bnQpOwotCQkJaWYg KHRtcCAmIERNQV9CWVRFX0NPVU5UX01BU0spCisJCQl1MzIgY29uc3QgZXBfZG1hY291bnQgPSBy ZWFkbCgmZXAtPmRtYS0+ZG1hY291bnQpOworCisJCQlpZiAoZXBfZG1hY291bnQgJiBETUFfQllU RV9DT1VOVF9NQVNLKQogCQkJCWJyZWFrOwogCQkJLyogc2luZ2xlIHRyYW5zZmVyIG1vZGUgKi8K LQkJCWRtYV9kb25lKGVwLCByZXEsIHRtcCwgMCk7CisJCQlkbWFfZG9uZShlcCwgcmVxLCByZXFf ZG1hX2NvdW50LCAwKTsKIAkJCW51bV9jb21wbGV0ZWQrKzsKIAkJCWJyZWFrOwogCQl9IGVsc2Ug aWYgKCFlcC0+aXNfaW4gJiYKIAkJCSAgIChyZXEtPnJlcS5sZW5ndGggJSBlcC0+ZXAubWF4cGFj a2V0KSAmJgogCQkJICAgIShlcC0+ZGV2LT5xdWlya3MgJiBQTFhfUENJRSkpIHsKIAotCQkJdG1w ID0gcmVhZGwoJmVwLT5yZWdzLT5lcF9zdGF0KTsKKwkJCXUzMiBjb25zdCBlcF9zdGF0ID0gcmVh ZGwoJmVwLT5yZWdzLT5lcF9zdGF0KTsKIAkJCS8qIEFWT0lEIFRST1VCTEUgSEVSRSBieSBub3Qg aXNzdWluZyBzaG9ydCByZWFkcyBmcm9tCiAJCQkgKiB5b3VyIGdhZGdldCBkcml2ZXIuICBUaGF0 IGhlbHBzIGF2b2lkcyBlcnJhdGEgMDEyMSwKIAkJCSAqIDAxMjIsIGFuZCAwMTI0OyBub3QgYWxs IGNhc2VzIHRyaWdnZXIgdGhlIHdhcm5pbmcuCiAJCQkgKi8KLQkJCWlmICgodG1wICYgQklUKE5B S19PVVRfUEFDS0VUUykpID09IDApIHsKKwkJCWlmICgoZXBfc3RhdCAmIEJJVChOQUtfT1VUX1BB Q0tFVFMpKSA9PSAwKSB7CiAJCQkJZXBfd2FybihlcC0+ZGV2LCAiJXMgbG9zdCBwYWNrZXQgc3lu YyFcbiIsCiAJCQkJCQllcC0+ZXAubmFtZSk7CiAJCQkJcmVxLT5yZXEuc3RhdHVzID0gLUVPVkVS RkxPVzsKIAkJCX0gZWxzZSB7Ci0JCQkJdG1wID0gcmVhZGwoJmVwLT5yZWdzLT5lcF9hdmFpbCk7 Ci0JCQkJaWYgKHRtcCkgeworCQkJCXUzMiBjb25zdCBlcF9hdmFpbCA9IHJlYWRsKCZlcC0+cmVn cy0+ZXBfYXZhaWwpOworCQkJCWlmIChlcF9hdmFpbCkgewogCQkJCQkvKiBmaWZvIGdldHMgZmx1 c2hlZCBsYXRlciAqLwogCQkJCQllcC0+b3V0X292ZXJmbG93ID0gMTsKIAkJCQkJZXBfZGJnKGVw LT5kZXYsCiAJCQkJCQkiJXMgZG1hLCBkaXNjYXJkICVkIGxlbiAlZFxuIiwKLQkJCQkJCWVwLT5l cC5uYW1lLCB0bXAsCisJCQkJCQllcC0+ZXAubmFtZSwgZXBfYXZhaWwsCiAJCQkJCQlyZXEtPnJl cS5sZW5ndGgpOwogCQkJCQlyZXEtPnJlcS5zdGF0dXMgPSAtRU9WRVJGTE9XOwogCQkJCX0KIAkJ CX0KIAkJfQotCQlkbWFfZG9uZShlcCwgcmVxLCB0bXAsIDApOworCQlkbWFfZG9uZShlcCwgcmVx LCByZXFfZG1hX2NvdW50LCAwKTsKIAkJbnVtX2NvbXBsZXRlZCsrOwogCX0KIAo= From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754855AbdLLM6m (ORCPT ); Tue, 12 Dec 2017 07:58:42 -0500 Received: from mail.linuxfoundation.org ([140.211.169.12]:34112 "EHLO mail.linuxfoundation.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752357AbdLLM6f (ORCPT ); Tue, 12 Dec 2017 07:58:35 -0500 From: Greg Kroah-Hartman To: linux-kernel@vger.kernel.org Cc: Greg Kroah-Hartman , stable@vger.kernel.org, Felipe Balbi , linux-usb@vger.kernel.org, Raz Manor , Felipe Balbi , Sasha Levin Subject: [PATCH 4.9 086/148] usb: gadget: udc: net2280: Fix tmp reusage in net2280 driver Date: Tue, 12 Dec 2017 13:44:56 +0100 Message-Id: <20171212124436.148277723@linuxfoundation.org> X-Mailer: git-send-email 2.15.1 In-Reply-To: <20171212124431.207182779@linuxfoundation.org> References: <20171212124431.207182779@linuxfoundation.org> User-Agent: quilt/0.65 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 4.9-stable review patch. If anyone has any objections, please let me know. ------------------ From: Raz Manor [ Upstream commit ef5e2fa9f65befa12f1113c734602d2c1964d2a5 ] In the function scan_dma_completions() there is a reusage of tmp variable. That coused a wrong value being used in some case when reading a short packet terminated transaction from an endpoint, in 2 concecutive reads. This was my logic for the patch: The req->td->dmadesc equals to 0 iff: -- There was a transaction ending with a short packet, and -- The read() to read it was shorter than the transaction length, and -- The read() to complete it is longer than the residue. I believe this is true from the printouts of various cases, but I can't be positive it is correct. Entering this if, there should be no more data in the endpoint (a short packet terminated the transaction). If there is, the transaction wasn't really done and we should exit and wait for it to finish entirely. That is the inner if. That inner if should never happen, but it is there to be on the safe side. That is why it is marked with the comment /* paranoia */. The size of the data available in the endpoint is ep->dma->dmacount and it is read to tmp. This entire clause is based on my own educated guesses. If we passed that inner if without breaking in the original code, than tmp & DMA_BYTE_MASK_COUNT== 0. That means we will always pass dma bytes count of 0 to dma_done(), meaning all the requested bytes were read. dma_done() reports back to the upper layer that the request (read()) was done and how many bytes were read. In the original code that would always be the request size, regardless of the actual size of the data. That did not make sense to me at all. However, the original value of tmp is req->td->dmacount, which is the dmacount value when the request's dma transaction was finished. And that is a much more reasonable value to report back to the caller. To recreate the problem: Read from a bulk out endpoint in a loop, 1024 * n bytes in each iteration. Connect the PLX to a host you can control. Send to that endpoint 1024 * n + x bytes, such that 0 < x < 1024 * n and (x % 1024) != 0 You would expect the first read() to return 1024 * n and the second read() to return x. But you will get the first read to return 1024 * n and the second one to return 1024 * n. That is true for every positive integer n. Cc: Felipe Balbi Cc: Greg Kroah-Hartman Cc: linux-usb@vger.kernel.org Signed-off-by: Raz Manor Signed-off-by: Felipe Balbi Signed-off-by: Sasha Levin Signed-off-by: Greg Kroah-Hartman --- drivers/usb/gadget/udc/net2280.c | 25 +++++++++++++------------ 1 file changed, 13 insertions(+), 12 deletions(-) --- a/drivers/usb/gadget/udc/net2280.c +++ b/drivers/usb/gadget/udc/net2280.c @@ -1146,15 +1146,15 @@ static int scan_dma_completions(struct n */ while (!list_empty(&ep->queue)) { struct net2280_request *req; - u32 tmp; + u32 req_dma_count; req = list_entry(ep->queue.next, struct net2280_request, queue); if (!req->valid) break; rmb(); - tmp = le32_to_cpup(&req->td->dmacount); - if ((tmp & BIT(VALID_BIT)) != 0) + req_dma_count = le32_to_cpup(&req->td->dmacount); + if ((req_dma_count & BIT(VALID_BIT)) != 0) break; /* SHORT_PACKET_TRANSFERRED_INTERRUPT handles "usb-short" @@ -1163,40 +1163,41 @@ static int scan_dma_completions(struct n */ if (unlikely(req->td->dmadesc == 0)) { /* paranoia */ - tmp = readl(&ep->dma->dmacount); - if (tmp & DMA_BYTE_COUNT_MASK) + u32 const ep_dmacount = readl(&ep->dma->dmacount); + + if (ep_dmacount & DMA_BYTE_COUNT_MASK) break; /* single transfer mode */ - dma_done(ep, req, tmp, 0); + dma_done(ep, req, req_dma_count, 0); num_completed++; break; } else if (!ep->is_in && (req->req.length % ep->ep.maxpacket) && !(ep->dev->quirks & PLX_PCIE)) { - tmp = readl(&ep->regs->ep_stat); + u32 const ep_stat = readl(&ep->regs->ep_stat); /* AVOID TROUBLE HERE by not issuing short reads from * your gadget driver. That helps avoids errata 0121, * 0122, and 0124; not all cases trigger the warning. */ - if ((tmp & BIT(NAK_OUT_PACKETS)) == 0) { + if ((ep_stat & BIT(NAK_OUT_PACKETS)) == 0) { ep_warn(ep->dev, "%s lost packet sync!\n", ep->ep.name); req->req.status = -EOVERFLOW; } else { - tmp = readl(&ep->regs->ep_avail); - if (tmp) { + u32 const ep_avail = readl(&ep->regs->ep_avail); + if (ep_avail) { /* fifo gets flushed later */ ep->out_overflow = 1; ep_dbg(ep->dev, "%s dma, discard %d len %d\n", - ep->ep.name, tmp, + ep->ep.name, ep_avail, req->req.length); req->req.status = -EOVERFLOW; } } } - dma_done(ep, req, tmp, 0); + dma_done(ep, req, req_dma_count, 0); num_completed++; }