The CreateFile callback now represents ZwCreateFile() instead of CreateFile() #83

[Corillian]

The previous implementation of CreateFile() was not passing a lot of the
information that gets passed into the driver. In order to provide driver
writers with as much information as possible I modified the CreateFile
to more closely resemble the parameters of ZwCreateFile().

I added some interop structs for moving some kernel data to user mode.
Added missing kernel mode FILE_* flags
Rearranged headers to make fileinfo.h and public.h available to user
mode drivers

[Liryna]

Hi @Corillian ,

Thanks for the contribution.
I am testing it in my Win8.1 with mirror.c and I am unable to save a .txt file.
I get the error: "The parameter is incorrect"

Is it stable in your environment ?

[Corillian]

Doh, I confess I only tested with reading as my driver doesn't do any writing (I only tested Mirror with reading as well). I'll see if I can repro on Win7 as that's all I have.

[Liryna]

Hi @Corillian ,

Does this last commit fix the issue ?
Just poke me when I can test it 😋

Otherwise I just seen also that you have add "options" param. That's really great ! It will fix a request from the dokan group:
https://groups.google.com/forum/#!topic/dokan/Phwd15PMXOw

[Corillian]

Hey @Liryna yeah that should fix it!

[Liryna]

Yes, it seems to be better ! Thank you !
Mirror was perfectly working for me.

I just would like to test with the .net test project before merging it but I have to update the wrapper in the same times :D

[Liryna]

Hi @Corillian ,

After review, it is right that this pull request give more informations to the dev during createfile but it give more complexity to use dokan.
We have to keep in mind that a normal dokan dev should need noknowledge in kernel to use it.

What I mean, it is that in mirror.c,
The dev should not need to convert flags like here:
https://github.com/dokan-dev/dokany/pull/91/files#diff-210b81c018cd0036b1a9abb162fb97b9R154
or
https://github.com/dokan-dev/dokany/pull/91/files#diff-210b81c018cd0036b1a9abb162fb97b9R244

I agree that such informations can be usefull for advanced dev but probably we should make this convertion in kernel side ?

Also, this convertion make the implementation of wrapper (C#, java,...) more complex.

[Corillian]

It's a minor increase in complexity and while I agree Dokan should make it as easy as possible that should not be at the expense of important functionality. It's impossible to write a file system driver without, on some level, understanding how a file system driver works. Mapping kernel create flags to user file attributes I consider a minor inconvenience, particularly since the documentation surrounding ZwCreateFile() is more or less just as good as CreateFile().

For C# interop the only challenging change should be accessing the SECURITY_DESCRIPTOR. For Java I have no idea.

[Corillian]

We could always add a helper function that maps the kernel create flags to user file attributes if it's a common usage case.

[Liryna]

Is it not possible to have directly FileAttributes filled by MirrorMapBit ?

About CreateDisposition should we not directly convert it to CREATE_NEW,...? Is there was case where knowing CreateDisposition == FILE_CREATE is usefull ? (I ask because I do not have all the vision/knowledge of it)

It just seems for me that everyuser will have to make this "translate" no ?

[Corillian]

The kernel flags contain more information than can be represented by the user mode dwFlagsAndAttributes however we can convert MirrorMapBit into a dokan.dll helper function if its a common usage case. As for CreateDisposition the old Dokan code suggest there's almost a 1 to 1 mapping between the kernel mode CreateDisposition and user mode dwCreationDisposition. Specifically the kernel CreateDisposition includes FILE_SUPERSEDE whereas there doesn't appear to be a user mode equivalent?

I don't think we should make some parameters user mode and some kernel mode within the Dokan CreateFile callback. All or nothing and I think kernel mode flags with dokan.dll helper functions for common usage cases where appropriate would be the best middle ground. This way those who want it are able to exploit the full capability of the file system driver and those who don't want it can just call the helper functions to quickly map the kernel mode flags into user mode flags.

[Liryna]

Yes, @Corillian I agree with you!
We should propose a helper in dokan.dll that retrieve a variable fully populated with the user land flags that have "kernel friend" (like FILE_FLAG_WRITE_THROUGH == FILE_WRITE_THROUGH).
and another one for FILE_CREATE == CREATE_NEW.

Could you just add it in your PR 😢
Sorry, last time I bother you ! I promise !

Thanks again for your precise answers and the great work !

[Corillian]

Hi @Liryna sorry for the delay in my response, I've got a lot going on. I'll take care of this in my PR!

[Corillian]

Alright @Liryna that should hopefully do it. Let me know if you need me to look into anything else.

[Corillian]

Alright hopefully this takes care of everything. I don't use FUSE and only have Windows 7 so I am unable to test alternative configurations. I also fixed memory corruption caused by a race condition in GetRawDeviceName().

[Liryna]

Thank you @Corillian !
I will try to review it quickly ❤️

[Liryna]

it work perfectly with the test I made :) Thank you @Corillian !
I will change the c# wrapper and run our test.

Do you have an exemple for testing the security context ?

[Liryna]

Hi @Corillian ,

It seems that there is a dead code here: https://github.com/dokan-dev/dokany/pull/91/files#diff-210b81c018cd0036b1a9abb162fb97b9R333
The status can only be equal to SUCCESS in this part of the code. What can of parameters did you wanted to check ? "Invalid directory ZwCreateFile parameters"

[Corillian]

Hi @Liryna,

Which part of the code are you referring to?

[Liryna]

This part https://github.com/dokan-dev/dokany/blob/master/dokan_mirror/mirror.c#L333

[Corillian]

How is it guaranteed to always be a success?

[Liryna]

My link has moved with last commits :(

Status is equal to STATUS_SUCCESS here
https://github.com/dokan-dev/dokany/blob/master/dokan_mirror/mirror.c#L207
so
https://github.com/dokan-dev/dokany/blob/master/dokan_mirror/mirror.c#L320
Is impossible since status is never changed before

[Corillian]

That part looks fine, how is it dead code? Are you assuming CreateDirectory() will always succeed?

[Liryna]

it is the code before CreateDirectory():

    // It is a create directory request
    if (status != STATUS_SUCCESS) { //status is initialized line 207 with the value STATUS_SUCCESS
                                    // so this condition is impossible 
      DbgPrint(L"\tInvalid directory ZwCreateFile parameters.\n\n");
      return status;
    }

[Corillian]

Ah yes that is dead code. It's leftover from something I ended up removing. Sry :\

[Liryna]

Oh 😢 so i remove it ?

[Corillian]

Yeah go ahead