Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1400536

RE: [PATCH] usb: gadget: f_fs: report error if excess data received

Show all headers | View raw

[Multipart message — attachments visible in raw view] - view raw

Hi,

"Du, Changbin" <changbin.du@intel.com> writes:
>> "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
>> 
> Ok, if we think this is a problem at host side that the transfer is not device
> expected, then device side should not accept the data or deliver the
> transferred data to userspace. But now we take part of the data to userspace
> and says it is ok.
> Do you agree with this point?

We deliver to userspace the part userspace requested, right? So that's
okay. The USB details WRT e.g. babble or host trying to send more data
than expected, needs to be handled within the kernel.

> IMO, we expose usb transfer as a file on device side. But file read() doesn't
> have a requirement that "sorry, you cannot read so little! you need read all
> once, else we may drop data for you. :) ".

but that's not how read() semantics work. When userspace asks to read(x)
bytes, we have three possible outcomes:

i. We have x bytes to return, so we copy_to_user(x)

ii. We have y < x bytes to return, so we copy_to_user(y)

iii. We have y > x bytes to return, so we copy_to_user(x)

This is exactly how the kernel is behaving. The only "detail" we have is
that, for some reason, host is sending too much data. what I still don't
know is if this extra data is garbage or something userspace genuinely
cares about. Do you know the answer to this?

> And some library that may retry read() until get enough data (which is
> normal For a general read). Then sometimes the buffer size for
> sys_read may not as expected. This is why I think ioctl approach is
> more appropriate for usb transfer.

no, this won't change anything. Besides, it's a pointless discussion as
cannot break userspace ABI. GadgetFS and FunctionFS have been shipping
in kernel for many years.

-- 
balbi

Back to linux.kernel | Previous | Next — Previous in thread | Next in thread | Find similar | Unroll thread

Thread

RE: [PATCH] usb: gadget: f_fs: report error if excess data received "Du, Changbin" <changbin.du@intel.com> - 2016-05-13 08:00 +0200
  RE: [PATCH] usb: gadget: f_fs: report error if excess data received Felipe Balbi <felipe.balbi@linux.intel.com> - 2016-05-13 08:40 +0200
    RE: [PATCH] usb: gadget: f_fs: report error if excess data received "Du, Changbin" <changbin.du@intel.com> - 2016-05-13 12:40 +0200
    RE: [PATCH] usb: gadget: f_fs: report error if excess data received Alan Stern <stern@rowland.harvard.edu> - 2016-05-13 16:30 +0200
      Re: [PATCH] usb: gadget: f_fs: report error if excess data received Michal Nazarewicz <mina86@mina86.com> - 2016-05-14 22:40 +0200
      RE: [PATCH] usb: gadget: f_fs: report error if excess data received Felipe Balbi <felipe.balbi@linux.intel.com> - 2016-05-16 15:10 +0200
        Re: [PATCH] usb: gadget: f_fs: report error if excess data received Michal Nazarewicz <mina86@mina86.com> - 2016-05-16 15:10 +0200
          Re: [PATCH] usb: gadget: f_fs: report error if excess data received Felipe Balbi <felipe.balbi@linux.intel.com> - 2016-05-16 15:20 +0200
            Re: [PATCH] usb: gadget: f_fs: report error if excess data received Michal Nazarewicz <mina86@mina86.com> - 2016-05-16 21:10 +0200
              RE: [PATCH] usb: gadget: f_fs: report error if excess data received "Du, Changbin" <changbin.du@intel.com> - 2016-05-17 05:00 +0200
                Re: [PATCH] usb: gadget: f_fs: report error if excess data received Michal Nazarewicz <mina86@mina86.com> - 2016-05-18 11:50 +0200
                Re: [PATCH] usb: gadget: f_fs: report error if excess data received Felipe Balbi <felipe.balbi@linux.intel.com> - 2016-05-18 12:20 +0200
                Re: [PATCH] usb: gadget: f_fs: report error if excess data received Michal Nazarewicz <mina86@mina86.com> - 2016-05-18 15:40 +0200
                RE: [PATCH] usb: gadget: f_fs: report error if excess data received "Du, Changbin" <changbin.du@intel.com> - 2016-05-19 05:00 +0200
                Re: [PATCH] usb: gadget: f_fs: report error if excess data received Michal Nazarewicz <mina86@mina86.com> - 2016-05-19 09:40 +0200
                RE: [PATCH] usb: gadget: f_fs: report error if excess data received "Du, Changbin" <changbin.du@intel.com> - 2016-05-19 11:00 +0200
                RE: [PATCH] usb: gadget: f_fs: report error if excess data received "Du, Changbin" <changbin.du@intel.com> - 2016-05-19 04:40 +0200

csiph-web