-
Notifications
You must be signed in to change notification settings - Fork 85
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Nexus 5, Android 4.4.4: "fatal error got type 0 (CONTAINER_UNDEFINED) in response" #75
Comments
Detected the same problem when copying all the media contents out of my Nexus 7. It seems that the culprit is the following code at the end of bulkRead at go-mtpfs/mtp.go:
...as it seems when running with -debug=mtp, the previous bulk read just before the error always returns a number of bytes divisible by 512(0x200), which is the case also in andreasf's log. I'm new to go and have no idea what the "Null read" is supposed to do. But it seems to me that the extra BulkTransfer call returns the CONTAINER_UNDEFINED status and causes the closing of the connection. Removing both the check and the extra read seems to fix the problem. |
it's been a while, but from what I can remember, you have to read until you reach the end of the stream. If the stream is exactly the size of the buffer then you need another read to make sure you really reached the end. What size is the file that generated the error? |
Sorry for the delay. Could be that this happened with files with a size of a multiple of 4096 bytes. Don't have the logs any more, sorry. The code seems to attempt what you say, but I think the problem is there is one read too many. The loop in bulkRead exits if there is an error in write or if lastRead < len(toread), where len(toreadI I believe is 4096. If the size of the file is a multiple of that, the, last read calls return values ...,4096, 4096, 4096 and finally 0, which makes the loop exit. Since 0 % 512 == 0, yet another read is made and that's one too many at least for some devices. I suppose there might be differences between devices. As said, I tested with a Nexus 7 and Android lollipop. How about changing the test to "if lastRead > 0 AND lastRead%packetSize == 0"? |
I just rsynced files from my phone, and the problem happened with a 2364404 bytes large file (not divisible by 4096). The following is repeatable for me:
Output from go-mtpfs:
|
Do you mean the error happened with my suggested change? If so, I guess it's not a good fix then. For what it's worth, those go-mtpfs errors look very familiar, especially the "got type 0 (CONTAINER_UNDEFINED) in response, want CONTAINER RESPONSE". That's what I got. I'll see if I can reproduce the problem once more. Thanks for the response, anyway! |
No, the error happens with the current HEAD. I did some debugging with Wireshark, and saw that the response to the null read is an EOVERFLOW error. After finding this in the libusb documentation, i changed
Wireshark now showed the 16 bytes response from the device:
So actually, this is the So the null read is actually harmful, and commenting it out solves the issue for me. Is it really required for some devices? |
From the PTP specification (PDF page 29): The Data phase ends when the number of bytes transferred equals the number of bytes specified in the first four bytes of the Data Block that coincide with a short or a NULL packet. A short packet or a NULL packet indicates the end of the end of a Data phase. If the number of bytes specified in the first four bytes of the Data Block are an integral multiple of the wMaxPacketSize field of the Endpoint Descriptor the Data phase will end in a NULL packet. So the null packet should be there if Concerning the size of my example file: if you add 12 bytes of headers, it is divisible by 512. |
all of these reports are for the android extensions, right? Can you confirm that things work for the standard MTP protocol (-android=false)? It's entirely possible that there are bugs in go-mtpfs, but for the android extensions, it's probably the only piece of software to exercise them, so it may also be a problem on the android side. |
I think the behavior of go-mtpfs is correct. Currently i'm trying to understand the Android MTP gadget source, which also implements the null packet case: https://android.googlesource.com/kernel/msm/+/android-5.0.2_r0.2/drivers/usb/gadget/f_mtp.c#673 |
(Edited) But actually I don't have Android 5.0.2 on my phone, I have 5.0.1, and none of the branches/tags I checked for 5.0 / 5.0.1 / 4.4 / "Lollipop Release" have this bug. So again I'm clueless about what's happening ;-) |
I did a few more packet dumps with Wireshark and this time disabled the Android extensions. The behavior is the same: neither go-mtpfs nor Wireshark receive a null packet. Here are the pcap files, if anybody wants to take a look:
I also tried a Sandisk Sansa Clip+ I had lying around. After adding "PIMA" as an allowed substring in
While the Sansa claims that the bulk transfer container size is file size + 12 bytes, the header it sends is actually 45 bytes large. I therefore tried to copy a 33 bytes smaller file, so that the real container size is divisible by 512. Again, no null packet:
|
Hanwen wrote:
I'm sorry but I'm not quite sure what the standard MTP protocol is and what are the Android extensions. My Android tablet was the first time I encountered this protocol. Wikipedia says that "MTP is part of the 'Windows Media' framework", so I guess standard MTP is something Windows specific, right? I'm afraid I cannot verify whether the problem applies to the standard Windows Media framework, as I have no idea of how to do that. I just wanted to copy media files from my Android tablet to my Linux computer and vice versa. I think andreasf has amply proved that an extraneous read after EOF is harmful with Android implementations, which indeed might be a bug in Android itself.
Looks to me like a necessary work-around in one context and a harmful feature in another. Would not be the first time. I take it from the gist of your reply that the main purpose of this software is to communicate with some Windows systems and Android is more like a sidekick. If so, I think it's quite all right for you to take no action, as we probably wouldn't want you to break something more important because of this. I'm happy with my patch which suits my purposes well and I hope andreasf is as well. Anyway, many thanks for a great piece of software. I have to say I was quite impressed of the quality of it when browsing through the source code. It's clearly superiour to any other Android <-> Linux file transfer solutions I have seen so far. |
Quite the contrary. I don't use windows and only use go-mtpfs with Android. Op do 2 apr. 2015 22:48 schreef Juhani Mäkelä notifications@github.com:
|
I'm quite confused about the missing null packet. I don't see it in
|
Hi, guys!
Thanks for the quick response, hanwen! It's very good to know that Android is the main use case. What comes to lack of time, please let us know how we could help! It's Easter here in Europe, and I have a lot of time to use and a Nexus 9 with which to test. What you andreasf are saying might be a good starting point. I have to confess you have gone so deep I haven't quite been able to follow. Anyway, now reading again what you wrote and quoted:
What does this mean? Does it mean that data ends when the number of bytes returned is less than the number requested? What is a NULL packet? If one has already gotten a zero bytes returned, why ask for another one?
I guess a NULL packet is a zero length packet that comes if the data available was divisible by the packet size requested? So that the one read before last got the requested amount and the next one gets zero?
There you are! Isn't the right reading algorithm then: keep requesting n bytes until less than n bytes is returned. When returned < n, it means there is no more data, regardless if returned is zero or greater than zero. A null packet only comes if the number of available data bytes is divisible by the requested size. Mind you, I have shot myself in the foot with this one before. In Linux/Unix/Posix there is an error condition EINTR which means that the system call was interrupted, and there is actually more data to come even if the current read was cut short. So the right algorithm is if returned == requested or erc == EINTR then keep reading... I wonder... anyway, please let me know how I can help. |
Andreas, what does your "about phone" say for the version number? The puzzling thing is: I have a stock nexus 5, and it works flawlessly here, [hanwen@pomba go-mtpfs]$ rsync --progress /tmp/x/Interne\ opslag/testfile /tmp/tf2 I was also surprised, since I recall writing test cases for these cases: add a panic in the null read cd mtp/ && go test disable null read: cd mtp/ && go test enable null read: cd mtp/ && go test in other words, over here, it seems to not make a difference if I do the null read or not. Does the test pass on your side? I have: Nexus 5, |
I could also copy the 2364404 byte file on my nexus 7, Android 5.1 build LMY47D |
2364404/512 |
@juhanima If you add the 12 bytes header, it divides to 4618 |
I see, OK. Still, wouldn't you need a bit more to prove there is no error than one isolated succesful case? After all, finding the error is usually the hardest part. |
This is the result of go test (I added some printfs for debugging...):
Nexus 5, Android 5.1, Build LMY47I (but it was the same with 5.0.1) |
Yep, that's the problem. Got one packet of zero length. Since 0 % (packet-length-whatever) == 0, try yet another time, and get an CONTAINER_UNDEFINED. Bloody Android is too strict! |
I think 512 bytes is the normal packet size, to the point that it's hardcoded in the source code. |
Yes, 512 sounds very normal indeed. |
andreas, what's the kernel version on your phone? it's also in the About Phone entry. |
Note that the android version (5.1, 5.0, LMY47I etc.) is separate from the kernel. |
Hate to quote myself, but
|
my kernel is 3.4.0-g88fbc66 (N5) |
Still suggesting
|
@hanwen, does your Linux host use xhci or ehci for the USB connection to the phone (see dmesg output)? I just stumbled upon this: http://www.spinics.net/lists/linux-usb/msg121502.html, and then tested go-mtpfs on an older Laptop with no USB3. The tests worked, and I can also see the null packet in Wireshark: http://zentrale1.com/~an/nexus5-ehci.pcapng |
that could be the problem. I have a lenovo T420, which only has USB2. $ dmesg|grep ehci (and lots more). |
I have added a -skip-null-read flag in the xhci-workaround branch. Can you verify if it works for you? |
@hanwen with this new command line flag I am able to successfully rsync/cp any data without causing any issues. Nexus 6 with relating issue #90 EDIT: I take back what i said, it seems after a second rsync run just for kicks I was able to hit the issue again.
|
@likewhoa - is it reproducible? Is it related to file size of what you're trying to transfer? |
also, this bug is for ¨fatal error got type 0 (CONTAINER_UNDEFINED) in response" for XHCI (usb3) machines for files that are a multiple of the blocksize (when adding 12 bytes of header data). Can you confirm whether the issue you see is somehitng new or not? |
@hanwen your workaround works for me. I'd like to suggest a more liberal way of handling the null packets, though: By just checking whether a null packet is received or not, users don't need to add additional parameters. I tried it on both my old and new Thinkpad, and interestingly the received size is always 16 bytes, even with USB 2.0:
So maybe this is more complicated than necessary, but I'd rather be safe. There's probably a better way, but this is how you can test the branch:
|
@hanwen I can reproduce this and I have tested by running the following commands.
I noticed that when I mount the device I always get ```[10126.276741] usb 2-7: usbfs: process 18784 (go-mtpfs) did not claim interface 0 before use
winters.gif
2015/04/22 11:16:23 AndroidGetPartialObject64 failed: mtp: cannot run operation ANDROID_GET_PARTIAL_OBJECT64, device is not open
|
@likewhoa do you have any other MTP software installed, for instance gvfs-mtp, the nautilus backend? I'm asking because gvfs-mtp tends to take over the connection. |
@andreasf The only gvfs stuff currently running is /usr/libexec/gvfsd & /usr/libexec/gvfs-udisks2-volume-monitor |
@likewhoa I'd try to uninstall gvfs-mtp and test again. The package is called gvfs-mtp for Fedora and gvfs-backends for Debian. |
@andreasf I don't have that package installed. |
@likewhoa You only have the same issue if you can find a file that reliably triggers the bug. The file size + 12 needs to be divisible by 512. The file you mentioned doesn't divide by 2. In your case, the device reset seems to come first. go-mtpfs probably shows CONTAINER_UNDEFINED because it receives no data after the reset. Have you tried whether it works on another host machine, or with an up-to-date kernel? |
@andreasf The kernel is 3.14.17 so it's not that old of a kernel but I plan on updating to 3.17.x this weekend. I can't find a relaible way to reproduce the issue and until I can then it just seems like perhaps something else triggered it. |
@andreasf - yes that looks better. Can you open a pull request? |
Thanks! PR opened. |
Affected version: 689b5b4
OS: Linux 3.17.2-300.fc21.x86_64
Related to issue: #23
After successfully mounting my phone, I try to rsync a folder to my computer. It works for a few files, until the connection is closed with the following errors:
Apparently, this happens as a result to an ANDROID_GET_PARTIAL_OBJECT64 request:
Debug logs, created with --debug=fs,mtp,usb:
rsync -av ...
: http://zentrale1.com/~an/debug-log.gzcp -ar ...
: http://zentrale1.com/~an/debug-log-cp.gzThe text was updated successfully, but these errors were encountered: