From patchwork Tue Nov 5 09:41:09 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Wen-chien Jesse Sung X-Patchwork-Id: 1189474 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=none (no SPF record) smtp.mailfrom=lists.ubuntu.com (client-ip=91.189.94.19; helo=huckleberry.canonical.com; envelope-from=kernel-team-bounces@lists.ubuntu.com; receiver=) Authentication-Results: ozlabs.org; dmarc=fail (p=none dis=none) header.from=canonical.com Received: from huckleberry.canonical.com (huckleberry.canonical.com [91.189.94.19]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 476l6X3Cnjz9sPF; Tue, 5 Nov 2019 20:41:23 +1100 (AEDT) Received: from localhost ([127.0.0.1] helo=huckleberry.canonical.com) by huckleberry.canonical.com with esmtp (Exim 4.86_2) (envelope-from ) id 1iRvKw-0008AX-Fh; Tue, 05 Nov 2019 09:41:18 +0000 Received: from youngberry.canonical.com ([91.189.89.112]) by huckleberry.canonical.com with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.86_2) (envelope-from ) id 1iRvKv-0008AN-0i for kernel-team@lists.ubuntu.com; Tue, 05 Nov 2019 09:41:17 +0000 Received: from 114-34-116-233.hinet-ip.hinet.net ([114.34.116.233] helo=cola.voip.idv.tw) by youngberry.canonical.com with esmtpsa (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.86_2) (envelope-from ) id 1iRvKu-00036K-B9; Tue, 05 Nov 2019 09:41:16 +0000 From: Wen-chien Jesse Sung To: kernel-team@lists.ubuntu.com Subject: [Xenial][PATCH 1/1] USB: cdc-wdm: ignore -EPIPE from GetEncapsulatedResponse Date: Tue, 5 Nov 2019 17:41:09 +0800 Message-Id: <20191105094109.25110-2-jesse.sung@canonical.com> X-Mailer: git-send-email 2.20.1 In-Reply-To: <20191105094109.25110-1-jesse.sung@canonical.com> References: <20191105094109.25110-1-jesse.sung@canonical.com> MIME-Version: 1.0 X-BeenThere: kernel-team@lists.ubuntu.com X-Mailman-Version: 2.1.20 Precedence: list List-Id: Kernel team discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: kernel-team-bounces@lists.ubuntu.com Sender: "kernel-team" From: Bjørn Mork BugLink: https://launchpad.net/bugs/1851347 The driver will forward errors to userspace after turning most of them into -EIO. But all status codes are not equal. The -EPIPE (stall) in particular can be seen more as a result of normal USB signaling than an actual error. The state is automatically cleared by the USB core without intervention from either driver or userspace. And most devices and firmwares will never trigger a stall as a result of GetEncapsulatedResponse. This is in fact a requirement for CDC WDM devices. Quoting from section 7.1 of the CDC WMC spec revision 1.1: The function shall not return STALL in response to GetEncapsulatedResponse. But this driver is also handling GetEncapsulatedResponse on behalf of the qmi_wwan and cdc_mbim drivers. Unfortunately the relevant specs are not as clear wrt stall. So some QMI and MBIM devices *will* occasionally stall, causing the GetEncapsulatedResponse to return an -EPIPE status. Translating this into -EIO for userspace has proven to be harmful. Treating it as an empty read is safer, making the driver behave as if the device was conforming to the CDC WDM spec. There have been numerous reports of issues related to -EPIPE errors from some newer CDC MBIM devices in particular, like for example the Fibocom L831-EAU. Testing on this device has shown that the issues go away if we simply ignore the -EPIPE status. Similar handling of -EPIPE is already known from e.g. usb_get_string() The -EPIPE log message is still kept to let us track devices with this unexpected behaviour, hoping that it attracts attention from firmware developers. Cc: Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=100938 Reported-and-tested-by: Christian Ehrig Reported-and-tested-by: Patrick Chilton Reported-and-tested-by: Andreas Böhler Signed-off-by: Bjørn Mork Acked-by: Oliver Neukum Signed-off-by: Greg Kroah-Hartman (backported from commit 8fec9355a968ad240f3a2e9ad55b823cf1cc52ff) Signed-off-by: Wen-chien Jesse Sung Acked-by: Connor Kuehl Acked-By: AceLan Kao --- drivers/usb/class/cdc-wdm.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/drivers/usb/class/cdc-wdm.c b/drivers/usb/class/cdc-wdm.c index 71ad04d54212..c2e3dcfa4ccd 100644 --- a/drivers/usb/class/cdc-wdm.c +++ b/drivers/usb/class/cdc-wdm.c @@ -188,7 +188,12 @@ static void wdm_in_callback(struct urb *urb) } } - desc->rerr = status; + /* + * Avoid propagating -EPIPE (stall) to userspace since it is + * better handled as an empty read + */ + desc->rerr = (status != -EPIPE) ? status : 0; + if (length + desc->length > desc->wMaxCommand) { /* The buffer would overflow */ set_bit(WDM_OVERFLOW, &desc->flags);