dokan-dev / dokany

User mode file system library for windows with FUSE Wrapper
http://dokan-dev.github.io
5.21k stars 661 forks source link

FUSE - Fix compare size_t to zero in fuse utils #1038

Closed ATRiiX closed 2 years ago

ATRiiX commented 2 years ago

We've got an infinite loop problem in 'convert_char' function when the get_func fail, because the return value in size_t would never less than zero.

Checklist

Changes proposed in this pull request:

Liryna commented 2 years ago

Hi @ATRiiX ,

Thank you for the pull request. Would you have a sample of input to reproduce the utf8_to_wchar_buf loop ? Using -1 with size_t isn't the most pretty code but it should work

ATRiiX commented 2 years ago

Hi @Liryna

This problem can only be reproduced on a spcific test laptop, and the behavior is that explorer open the fuse drive freeze until it fails. I dump the process to find the il variable was added and much bigger than the src_len, but the input seems a normal string contains several Chinise charactors. I copyed it but cannot reproduce it on my own machine.

I think if the input can meet the utf8_lengths[p[0]] == 0 condition, then return to the readed < 0 judgment in convert_char would never be enter before. After fix it, I found the problem on that laptop was solved. And these -1 are just some more cleanups.

ATRiiX commented 2 years ago

If the input is not a valid utf-8 string, for example, a GBK string in chinese, the function 'utf8_to_wchar_buf' may fail. I have tried a chinese charactor '测', as it's {0xb2, 0xe2} in GBK char array, can trigger the loop problem. FYI.

Liryna commented 2 years ago

Thanks @ATRiiX for the details! Shouldn't we just stop using those custom functions that are error prone and simply use https://stackoverflow.com/a/14809553 ? Would you be willing to implement it ?

ATRiiX commented 2 years ago

That's a good idea. Using the standard library function would be more simple and clean. I can spare some time to write and test it. :)

ATRiiX commented 2 years ago

Hi @Liryna , I have update a version using MultiByteToWideChar and WideCharToMultiByte to convert between utf8 and wchar. And for checking the invalid input of utf8 string, I add a check of 0xfffd as documented in msdn. I have test it on our application, and the readdir is working correctly.

Liryna commented 2 years ago

Thank you very much @ATRiiX for the contribution and for being so quick! Very appreciated 🏆

Liryna commented 2 years ago

@ATRiiX https://github.com/ATRiiX/dokany/commit/3193316abaf4f40eb9ef9a93073a32360a12b22a This looks interesting, do you have a repro or described how it happened ?

ATRiiX commented 2 years ago

@Liryna 🍺Happy 2022! Glad to hear you are interested to this patch. I just ported it to the master branch, but I donnot have the environment for testing the self-compiled driver recently. So I have not send the pr yet. This is to fix issue #355 , as I tested on v1.3.0.1000 earlier last year. The problem is caused by the different behavior of IOCTL_MOUNTDEV_QUERY_DEVICE_NAME on sub directories under windows local drive and dokan drive. I use the windows process monitor to traceing the syscalls of opening the properties, and find that the local dirs ioctl return STATUS_INVALID_PARAMETER where the dokan sub dir return SUCCESS. And I added the original IsVolumeOpen check on this ioctl, but it's not working. Finally found the FsContext can be null by adding log there, so I modify the original IsVolumeOpen. (As far as I remember) I may still need some days ready to test it after the holiday, or if you can pick it and help testing the properties issue.

Liryna commented 2 years ago

Happy 2022 you too! 🥳 Oh I see. Thanks for the detailed response! I believe it can wait so feel free to ping me when you are able to take a look at it again.