search

dokan-dev / dokany

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

FUSE: ACCESS_VIOLATION on fuse_exit() call #985

Closed infeo closed 3 years ago

infeo commented 3 years ago

Environment

Check List

Description

Building Dokany from the repository and using dokanfuse1.dll to mount a filesystem via fuse, an application making a blocking call to fuse_main_real(...) crashes on calling fuse_exit() with EXCEPTION_ACCESS_VIOLATION. This does not happen with other virtual filesystem providers implementing FUSE (e.g. winfsp).

Logs

Up to now i did not achieved to enable kernel debug output. 😓

Liryna commented 3 years ago

Hi @infeo ,

fuse_exit() was changed at head https://github.com/dokan-dev/dokany/commit/de4c04c449c046b103b1f64b6e3a554b26470020 You do not need kernel log here. It is the library that crash somewhere. Would you be able to attach a debugger or add logs to find out what is the reason ?

infeo commented 3 years ago

It fails when calling the actual unmount method of dokan in the fuse_chan struct: https://github.com/dokan-dev/dokany/blob/4208150968d650cc4102c07b715b4c9658c42afb/dokan_fuse/src/dokanfuse.cpp#L695

Another thing i can see in the debugger is, that the mountpoint of this struct is not the actual mountpoint.

If I remove the if-condition from https://github.com/dokan-dev/dokany/blob/4208150968d650cc4102c07b715b4c9658c42afb/dokan_fuse/src/dokanfuse.cpp#L679-L684

(i.e. always re-initializing the struct) and set a hardcoded mountpoint unmounting works .

All these things indicate, that this struct is either wrong initialized or memory is later cleared.

Liryna commented 3 years ago

Thanks for the information. I still have difficulties to see where fuse_exit is called with an invalid struct ? Do you have a callstack to share ?

fuse_main_real will destroy the mount_point and the struct during fuse_teardown so if this one is used, it can be unsafe at some point.

infeo commented 3 years ago

I looked further into it and found the cause.

The FUSE library I use calls fuse_exit( struct fuse *f) with the pointer inside the fuse context returned by fuse_get_context(void): https://github.com/dokan-dev/dokany/blob/4208150968d650cc4102c07b715b4c9658c42afb/dokan_fuse/src/fusemain.cpp#L63-L67

But this pointer is not the same as the one of the created fuse object in fuse_main_real(): https://github.com/dokan-dev/dokany/blob/4208150968d650cc4102c07b715b4c9658c42afb/dokan_fuse/src/dokanfuse.cpp#L797-L805

Liryna commented 3 years ago

Is fuse_get_context called during an operation? The pointer returned is only valid during that time.

If that's the case, I do not see how the pointer can be invalid.

infeo commented 3 years ago

Is fuse_get_context called during an operation?

Yes, get_context is called during init(struct fuse_conn_info *conn).

The pointer returned is only valid during that time.

But only the pointer to the context. The content will stay valid even after the return of the callback, especially the contained pointer to the actual fuse object, right?

Liryna commented 3 years ago

I am not currently able to get an env to test but by reading the code, yes calling the function during init is safe. You will get the pointer of the impl_fuse_context that lives in do_fuse_loop (you can log here the object memory address and compare with the pointer you received from the get context). The pointer is also set to the Global context of the Dokanoptions struct which is used during all operations.

I am having difficulty to see how the value of the pointer could be different. If you call the get context in other operations, does the pointer is again different ?

infeo commented 3 years ago

If you call the get context in other operations, does the pointer is again different ? No.

But i have found the cause. Instead of the actual fuse object, Dokany returns on a fuse_get_context() call a structure called impl_fuse_context (casted to a fuse struct): https://github.com/dokan-dev/dokany/blob/4208150968d650cc4102c07b715b4c9658c42afb/dokan_fuse/src/fusemain.cpp#L45-L48

The problem about this structure is, that it does not contain the pointer to the dokan function mappings (fuse_chan). Compare the fuse struct https://github.com/dokan-dev/dokany/blob/4208150968d650cc4102c07b715b4c9658c42afb/dokan_fuse/include/dokanfuse.h#L69-L74

with impl_fuse_context https://github.com/dokan-dev/dokany/blob/4208150968d650cc4102c07b715b4c9658c42afb/dokan_fuse/include/fusemain.h#L71-L85

Liryna commented 3 years ago

@infeo I made a change to return a valid fuse value https://github.com/dokan-dev/dokany/commit/c9e159c84ee7e653bf7033db6f21389e51b63498 Coud you give a try and let me know if it fix your crash ?

infeo commented 3 years ago

I tested it and now everything works as expected 🙂

@Liryna Looking at the commit, since the impl_fuse_context now contains directly the fuse object, there is a lot of duplicate data contained in it. I'm not deep enough in the code, but it might be worth to remove some fields and use the "detour" over the fuse pointer ( I don't think this will introduce a lot of overhead).

Are there reasons against this idea? If not, i might have some free time to clean the code a little😉

Liryna commented 3 years ago

@infeo Agree that would be a good change. I still wonder why it was not the case at first and why it was explicitly a hack :( I would be glad to review your PR!