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

[Felipe Balbi <balbi@kernel.org>]

Hi,

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