All of lore.kernel.org
 help / color / mirror / Atom feed
From: Felipe Balbi <felipe.balbi@linux.intel.com>
To: "Du\, Changbin" <changbin.du@intel.com>,
	Al Viro <viro@zeniv.linux.org.uk>
Cc: "gregkh\@linuxfoundation.org" <gregkh@linuxfoundation.org>,
	"mina86\@mina86.com" <mina86@mina86.com>,
	"rui.silva\@linaro.org" <rui.silva@linaro.org>,
	"k.opasiak\@samsung.com" <k.opasiak@samsung.com>,
	"lars\@metafoo.de" <lars@metafoo.de>,
	"linux-usb\@vger.kernel.org" <linux-usb@vger.kernel.org>,
	"linux-kernel\@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: RE: [PATCH] usb: gadget: f_fs: report error if excess data received
Date: Thu, 12 May 2016 14:22:44 +0300	[thread overview]
Message-ID: <878tzfed17.fsf@linux.intel.com> (raw)
In-Reply-To: <0C18FE92A7765D4EB9EE5D38D86A563A05D2F35C@SHSMSX103.ccr.corp.intel.com>


Hi,

"Du, Changbin" <changbin.du@intel.com> 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

  reply	other threads:[~2016-05-12 11:26 UTC|newest]

Thread overview: 38+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-05-11 10:19 [PATCH] usb: gadget: f_fs: report error if excess data received changbin.du
2016-05-11 10:59 ` Felipe Balbi
2016-05-11 12:30   ` Michal Nazarewicz
2016-05-12  4:25     ` Du, Changbin
2016-05-12  4:21   ` Du, Changbin
2016-05-12  6:52     ` Felipe Balbi
2016-05-12  7:30       ` Du, Changbin
2016-05-12  7:46         ` Felipe Balbi
2016-05-12  8:16           ` Du, Changbin
2016-05-12  9:15             ` Felipe Balbi
2016-05-12  9:22               ` Felipe Balbi
2016-05-12  9:51                 ` Du, Changbin
2016-05-12  9:39               ` Du, Changbin
2016-05-12 10:13                 ` Felipe Balbi
2016-05-12 10:14                 ` Felipe Balbi
2016-05-12 10:45                   ` Du, Changbin
2016-05-12 11:22                     ` Felipe Balbi [this message]
2016-05-13  5:52                       ` Du, Changbin
2016-05-13  6:36                         ` Felipe Balbi
2016-05-13 10:32                           ` Du, Changbin
2016-05-13 14:29                           ` Alan Stern
2016-05-14 20:39                             ` Michal Nazarewicz
2016-05-16 12:57                             ` Felipe Balbi
2016-05-16 13:08                               ` Michal Nazarewicz
2016-05-16 13:16                                 ` Felipe Balbi
2016-05-16 19:09                                   ` Michal Nazarewicz
2016-05-17  2:53                                     ` Du, Changbin
2016-05-18  9:45                                       ` Michal Nazarewicz
2016-05-18 10:15                                         ` Felipe Balbi
2016-05-18 13:39                                           ` Michal Nazarewicz
2016-05-19  2:54                                             ` Du, Changbin
2016-05-19  7:34                                               ` Michal Nazarewicz
2016-05-19  8:49                                                 ` Du, Changbin
2016-05-19  2:31                                           ` Du, Changbin
2016-05-16 16:05 ` Michal Nazarewicz
2016-05-16 16:27   ` Lars-Peter Clausen
2016-05-16 16:48     ` Michal Nazarewicz
2016-05-16 16:35   ` Krzysztof Opasiak

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=878tzfed17.fsf@linux.intel.com \
    --to=felipe.balbi@linux.intel.com \
    --cc=changbin.du@intel.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=k.opasiak@samsung.com \
    --cc=lars@metafoo.de \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-usb@vger.kernel.org \
    --cc=mina86@mina86.com \
    --cc=rui.silva@linaro.org \
    --cc=viro@zeniv.linux.org.uk \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.