From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753215AbcERKR2 (ORCPT ); Wed, 18 May 2016 06:17:28 -0400 Received: from mga02.intel.com ([134.134.136.20]:4419 "EHLO mga02.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752790AbcERKR1 (ORCPT ); Wed, 18 May 2016 06:17:27 -0400 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.26,328,1459839600"; d="asc'?scan'208";a="983601296" From: Felipe Balbi To: Michal Nazarewicz , "Du\, Changbin" , Alan Stern Cc: Al Viro , "gregkh\@linuxfoundation.org" , "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: References: <87eg92p3cn.fsf@linux.intel.com> <87bn46p2gk.fsf@linux.intel.com> <0C18FE92A7765D4EB9EE5D38D86A563A05D30546@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: Wed, 18 May 2016 13:15:13 +0300 Message-ID: <878tz7zn7y.fsf@linux.intel.com> MIME-Version: 1.0 Content-Type: multipart/signed; boundary="=-=-="; micalg=pgp-sha256; protocol="application/pgp-signature" Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org --=-=-= Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Hi, Michal Nazarewicz writes: > On Tue, May 17 2016, Changbin Du wrote: >>> There appears to be no kfifo support for iov_iter though, so I just went >>> with a simple buffer. >>>=20 >>> I haven=E2=80=99t looked at the patch too carefully so this is an RFC r= ather >>> than an actual patch at this point. It does compile at least. >>>=20 >>> Regardless, the more I thin about it, the more I=E2=80=99m under the im= pression >>> that the whole rounding up in f_fs was a mistake. And the more I=E2=80= =99m >>> leaning towards ignoring the excess data set by the host. >>>=20 >>> ---------- >8 ---------------------------------------------------------- >>> Subject: usb: gadget: f_fs: buffer data from =E2=80=98oversized=E2=80= =99 OUT requests >>>=20 >>> f_fs rounds up read(2) requests to a multiple of a max packet size >>> which means that host may provide more data than user has space for. >>> So far, the excess data has been silently ignored. >>>=20 >>> This introduces a buffer for a tail of such requests so that they are >>> returned on next read instead of being ignored. >>>=20 >> >> Congratulations finally reach an agreement, > > To be honest, if it was up to me, I would rip request size rounding up > out of the code. we've been through this before. This needs to be done at the gadget layer. Gadget driver can over-allocate ahead of time if gadget->quirk_ep_out_aligned_size is true, then we avoid memcpy() at the UDC driver level. >> thanks Alan Stern and Michal. >> Here just have a comment - the buffered data need be dropped when the >> epfile is closed, because it means the session is terminated. > > I blame that on sleep deprivation. Another issue is what to do when > endpoint is disabled. Should the buffer be cleared as soon as the > endpoint is disabled? Or maybe when the endpoint is enabled again? Or > maybe it should never be cleared? > > If the buffer is cleared when endpoint is disabled, we again silently > drop data. On the other hand, if we don=E2=80=99t do that, read() on the > endpoint will may succeed even if the configuration is disabled which > may be surprising for users. tough decision... but seems like clearing the buffer as soon as ep is disabled is the way to go. =2D-=20 balbi --=-=-= Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 iQIcBAEBCAAGBQJXPECxAAoJEIaOsuA1yqRENu0QAKhstcDArLd1tgsFh2VOtCot HDevy79lGJS1Zk/Gbsw3PPofYeVsn66SMB44PGNCYmV8NdOvdwpRFy662gnteryQ /fJqPjw2f9M7XP/cmKpKtuMeZOeAIdqbTpheDs48RDHqEblmkLk+CNCl9VP0cAK3 C09YS/28f74FzvRYOI3e7lLszQbOQ8C+GsAlmYF7FhyyE4WE2mkUA6BJECdwGWK/ 5fQCWrKI0Jw1X0PH3nKpNWSPUacqYmGpKhQHU8ZITpvBtmKvwhltV/eiuy3X6HFo I4HhQUBMG0s1rlw1y1p4lfLUhlDNmrQnhpXylx0ck/x932KPzE98aHW2e79kUeFu Bg8Ntqd2LymG7EwKGHWqRkWxHRYhi4ZlwQtjhQ7HXqzLYe++TkZwa8/bYWacT0Vq ytwp18NcuvE+AHqsiVmTHm/Nc942Kp2+luV+w1BWHAnRnZtWnDkEY6+3x47yeE7Q QTWvLLqd7S88rybaH2AxkpG69C25tmvZjmiBP8PUVjFgnVRCzSSdMz4DLQ24ggud nZANmIQfH4o/a/n3kGY2PVyOLPmFbsSNvxsgkZRw9l5g+da+WvFX5++c9duFQ/v6 sb7o8vv8PbQvIG6rRyLw7UMSbC5tq1pXrTvL7Ao/kCB53+HW1bIPz2mxjSYcj2Kx p4GhsA9SVAPqUvnVy6a6 =56e4 -----END PGP SIGNATURE----- --=-=-=--