From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752138AbcELGyx (ORCPT ); Thu, 12 May 2016 02:54:53 -0400 Received: from mga02.intel.com ([134.134.136.20]:11552 "EHLO mga02.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751402AbcELGyw (ORCPT ); Thu, 12 May 2016 02:54:52 -0400 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.24,609,1455004800"; d="scan'208";a="974129010" From: Felipe Balbi To: "Du\, Changbin" Cc: "gregkh\@linuxfoundation.org" , "mina86\@mina86.com" , "rui.silva\@linaro.org" , "k.opasiak\@samsung.com" , "lars\@metafoo.de" , "linux-usb\@vger.kernel.org" , "linux-kernel\@vger.kernel.org" Subject: RE: [PATCH] usb: gadget: f_fs: report error if excess data received In-Reply-To: <0C18FE92A7765D4EB9EE5D38D86A563A05D2F01F@SHSMSX103.ccr.corp.intel.com> References: <1462961970-2001-1-git-send-email-changbin.du@intel.com> <87twi4g8s2.fsf@linux.intel.com> <0C18FE92A7765D4EB9EE5D38D86A563A05D2F01F@SHSMSX103.ccr.corp.intel.com> User-Agent: Notmuch/0.22+11~g124a67e (http://notmuchmail.org) Emacs/25.0.93.2 (x86_64-pc-linux-gnu) Date: Thu, 12 May 2016 09:52:41 +0300 Message-ID: <87a8jvg43q.fsf@linux.intel.com> MIME-Version: 1.0 Content-Type: text/plain Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi, "Du, Changbin" writes: >> > If it happen, we can keep the excess data for next i/o, or >> > report an error. But we cannot silently drop data, because >> > USB layer should ensure the data integrality it has transferred, >> > otherwise applications may get corrupt data if it doesn't >> > detect this case. >> >> and when has this actually happened ? Host should not send more data in >> this case, if it does, it's an error on the host side. Also, returning >> -EOVERFLOW is not exactly correct here, because you'd violate POSIX >> specification of read(), right ? >> > This can happen if the host side app force kill-restart, not taking care of this > special condition(and we are not documented), or even it is a bug. Usually APPs > may has a protocol to control the packet size, but protocol mismatch can happen > if either side encounter an error. > > Anyway, this is real. If kernel return success and drop data, the error may > explosion later, or its totally hided (but why some data lost in kernel? > Kernel cannot tell userspace we cannot be trusted sometimes, right?). > so IMO, if this is an error, we need report an error or fix it, not hide it. > > The POSIX didn't say read cannot return "-EOVERFLOW", it says: > " Other errors may occur, depending on the object connected to fd." > > If "-EOVERFLOW" is not suitable, EFAULT, or any suggestions? > >> > Here, we simply report an error to userspace to let userspace >> > proccess. Actually, userspace applications should negotiate >> >> no, this violates POSIX. Care to explain what problem are you actually >> facing ? >> > Why this violates POSIX? Could you give more details? read(5) should return at mode 5 bytes. If there are more, than 5 bytes, we don't error out, we just return the requested 5 bytes and wait for a further read. What I'm more concerned, however, is why we received more than expected data. What's on the extra bytes ? Can you capture dwc3 traces ? Perhaps add a few traces doing a hexdump (using __print_hex()) of the data in req->buf. > The problem is device side app sometimes received incorrect data caused > by the dropping. Most times the error can be detected by APP itself, but why ? app did e.g. read(5), that caused driver to queue a usb_request with length set to 512. Host sent more data than the expected 5 bytes, why did host do that ? And if that data was needed, why didn't userspace read() more than 5 ? -- balbi