magnusja / libaums

Open source library to access USB Mass Storage devices on Android without rooting your device
Apache License 2.0
1.28k stars 272 forks source link

Having more than 128 files in the root directory corrupts the filesystem when reading or writing #298

Closed stbelousov closed 1 year ago

stbelousov commented 3 years ago
  1. Format a USB stick to FAT32.
  2. Create 129 (or any higher number of) files in the root directory of the USB stick. Files can be anything, even empty ones. Filenames also can be anything, even file1, file2, ...
  3. Plug in the USB stick to an Android device.
  4. Initialize the UsbMassStorageDevice as in documentation and print the storageDevice.partitions[0].fileSystem.rootDirectory.listFiles().size. Instead of printing the real number of files, it prints that number modulo 128 (e.g., if you have 129 files on the USB stick, it prints 1, and so on). If you try to read or write any files then, it corrupts the filesystem on the USB stick.
stbelousov commented 3 years ago

The bug is there: https://github.com/tracmap/libaums/blob/develop/libaums/src/main/java/com/github/mjdev/libaums/driver/scsi/ScsiBlockDevice.kt#L258

inBuffer may already contain data from previous reads (e.g., when we call rootDirectory.listFiles(), and filenames span over multiple clusters). inBuffer.clear() wipes all that data.

stbelousov commented 3 years ago

Just in case, maybe it could be helpful. Fixed that in our fork: https://github.com/tracmap/libaums/commit/7b9219106419142dec6b1a179067037a9e73c18d Seems to work fine now.

That's not an elegant solution (I just create a temporary buffer every time) but we just needed to make it work quickly.

magnusja commented 3 years ago

Hey there,

thanks for the fix I will have a look, but first I need to figure this bintray/maven repo thing out.

stbelousov commented 3 years ago

Any updates on this one?

magnusja commented 3 years ago

Can you help reproducing this issue? I added a test but this does not work https://github.com/magnusja/libaums/pull/314

stbelousov commented 3 years ago

@magnusja The test does not work because it does not really call the transferOneCommand function from ScsiBlockDevice.kt (that contains the actual bug). This can be observed by adding throw IOException("FAIL") to the beginning of the function. The test still runs fine in this case.

The issue is easily reproducible on a real Android device.

JetpackDuba commented 3 years ago

Not sure if related or not. I have got an issue where if I write a code to create 130 files with "hello" as their content, many files are not created and others are duplicated with the exact same name (not sure how is that even possible). It happens in any folder, not just the root directory.

Here goes a code snippet to reproduce it:

// Only uses the first partition on the device
val currentFs = device.partitions[0].fileSystem
val root = currentFs.rootDirectory

val newDir = root.createDirectory("foo")

repeat(130) {
    val file = newDir.createFile("bar_$it.txt")

    // write to a file
    val os = UsbFileOutputStream(file)

    os.write("hello".toByteArray())
    os.close()
    file.close()
}

After doing this, I have plugged in the USB in my PC to check the its content with the following results:

image

bar_62.txt is the file with the biggest number but what's more surprising is the existance of multiple files with the same name like bar_0.txt.

Having a total of 189 files: image

At this point I guess that the filesystem got corrupted because I can't even add or remove files, I have to reformat the device.

Doing the same test but with 63 iterations works nicely:

image

However with anything higher than 63 the issue is reproduced. Example with 64 iterations where at every file is at least duplicated once:

image

I will test @stbelousov 's fork to see if it works but let me know if I can somehow help you (or if you can provide me some hints where to look at to debug it myself).

JetpackDuba commented 3 years ago

Hello again,

I have tested @stbelousov workaround and seems to work fine, I have been able to create the 130 files without problem:

image

I had an issue where the last file content wasn't written but it was because I forgot to close the file in each iteration (although I thought it would be enough closing the OutputStream, let me know if that's not the expected behavior).

devkumar2486 commented 2 years ago

[@magnusja I also faced the issue reported by @stbelousov, If we put more than 100 files in usb, we get count of lesser files and also the rest of the files got deleted, number of files are not fixed, so it may also depends on size of files or how the files are stored in usb, also I found that most of the time the name of last file remain in the usb get changed, I used 300 small size image from this zip small-size_300_images.zip After scan only 45 images remain in the usb from file "gen_257_1604472283911.jpg" to "gen_300_1604472289002.jpg" (44 files) and "gen_2~6.jpg" (original name of the file was "gen_256_1604472283828.jpg") this supports this comment https://github.com/magnusja/libaums/issues/298#issuecomment-849216577 Finally I used the fix provided by @stbelousov in his fork https://github.com/tracmap/libaums/commit/7b9219106419142dec6b1a179067037a9e73c18d and this solved the issue for me. I suggest you to integrate his fix in the repo ASAP. Thanks in advance, also thanks to @stbelousov for providing fix

froctest commented 1 year ago

Is the problem solved, because of this problem, I'm not sure if I should use this open source library (sorry my English was translated from Chinese with translation software, maybe the description is not accurate)

magnusja commented 1 year ago

Sorry for the insane dealy.

I am in process of fixing in #298

In contrast to your solution, I am not using a temporary byter buffer. Instead I am settings limit to position + transferLength and avoid calling clear. Then I can write directly in the byte buffer.

Thanks a lot for spotting!