From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932467AbcELL0F (ORCPT ); Thu, 12 May 2016 07:26:05 -0400 Received: from mga03.intel.com ([134.134.136.65]:51561 "EHLO mga03.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932263AbcELLY4 (ORCPT ); Thu, 12 May 2016 07:24:56 -0400 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.24,609,1455004800"; d="scan'208";a="804671811" From: Felipe Balbi To: "Du\, Changbin" , Al Viro 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: <0C18FE92A7765D4EB9EE5D38D86A563A05D2F35C@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> <87a8jvg43q.fsf@linux.intel.com> <0C18FE92A7765D4EB9EE5D38D86A563A05D2F156@SHSMSX103.ccr.corp.intel.com> <874ma3g1lq.fsf@linux.intel.com> <0C18FE92A7765D4EB9EE5D38D86A563A05D2F183@SHSMSX103.ccr.corp.intel.com> <87zirveixx.fsf@linux.intel.com> <0C18FE92A7765D4EB9EE5D38D86A563A05D2F2C8@SHSMSX103.ccr.corp.intel.com> <87shxneg6t.fsf@linux.intel.com> <0C18FE92A7765D4EB9EE5D38D86A563A05D2F35C@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 14:22:44 +0300 Message-ID: <878tzfed17.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: >> right, and that was my point: if we copy more to userspace, then we have >> a real big problem. >> > Yes, we drop the data because we userspace buffer is not enough this time. > The problem here is that really can we just drop it silently? Maybe not. Yeah, it probably deserves a pr_err() or pr_debug(), but host sending more data than it should, is another problem altogether which needs to be addressed at the host. Adding a print to aid debugging is a good idea, but bailing out on the peripheral side is not :-s >> > And this missed AIO path. This is identify to my patch after remove the >> >> right, it's more of a debug patch since I don't have the setup to >> trigger this (I'm assuming you're using adb?) >> > Right. And adb can detect this unexpected behavior(data mismatch) quickly > because it has some selfcheck for the data content. cool >> > Byw, we not need add the field "expected_len", we can get it from the >> > struct ffs_io_data. >> >> without expected_len we can copy more data to userspace, right ? If >> req->actual > data_len_before_aligning_to_maxpacket, then we will copy >> more data then we should to userspace and this was a regression caused >> by Al's commit, AFAICT. >> > No, expected_len equals to iov_iter_count(&io_data->data), right? So we > do not need a new field. /me goes read iov_iter_count() you're right, we don't need expected len at all ;-) in any case, did you figure out if the extra data host sends is important data at all or just garbage ? -- balbi