Skip to content
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

Possible race conditions in mirror.c sample. #32

Closed
storzinc opened this issue Jul 19, 2015 · 45 comments
Closed

Possible race conditions in mirror.c sample. #32

storzinc opened this issue Jul 19, 2015 · 45 comments
Labels

Comments

@storzinc
Copy link

I am trying to make my own fs using mirror.c sample as a basis, but it seems to me that there will be race conditions if multiple threads will be accessing it concurrently.
For example in the MirrorCleanup there is such code:

        CloseHandle((HANDLE)DokanFileInfo->Context);
        DokanFileInfo->Context = 0;

        if (DokanFileInfo->DeleteOnClose) {
            DbgPrint(L"\tDeleteOnClose\n");
            if (DokanFileInfo->IsDirectory) {
                DbgPrint(L"  DeleteDirectory ");
                if (!RemoveDirectory(filePath)) {
                    DbgPrint(L"error code = %d\n\n", GetLastError());
                } else {
                    DbgPrint(L"success\n\n");
                }
            } else {
                DbgPrint(L"  DeleteFile ");
                if (DeleteFile(filePath) == 0) {
                    DbgPrint(L" error code = %d\n\n", GetLastError());
                } else {
                    DbgPrint(L"success\n\n");
                }
            }
        }

But wouldn't it be possible that after CloseHandle another thread will delete this file, and create new file in it's place?
Similar problem seems to be in MirrorDeleteDirectory function, that function is checking that the directory being deleted is empty, but what if new file will be created after this check but before directory really will be deleted?

Also in multiple functions there is code similar to this:

    if (!handle || handle == INVALID_HANDLE_VALUE) {
        DbgPrint(L"\tinvalid handle, cleanuped?\n");
        handle = CreateFile(
            filePath,
            GENERIC_READ,
            FILE_SHARE_READ,
            NULL,
            OPEN_EXISTING,
            0,
            NULL);
        if (handle == INVALID_HANDLE_VALUE) {
            DbgPrint(L"\tCreateFile error : %d\n\n", GetLastError());
            return -1;
        }
        opened = TRUE;
    }

But in what cases can you get invalid handle sent to your function? And is it really okay to just reopen file here?

Maybe I am missing something?

@Liryna
Copy link
Member

Liryna commented Jul 20, 2015

Windows Kernel controls dokan driver. Dokan driver only forward the kernel call to user land.
The Kernel should take care of the situation that you described.
But some illogique call can also be made form him and it is your userland code that should handle it.

@storzinc
Copy link
Author

@Liryna It seems that I misunderstood the purpose of the DokanFileInfo->DeleteOnClose flag, I thought it corresponds to FILE_FLAG_DELETE_ON_CLOSE from CreateFile but it seems to be not the case, so my first example should be invalid. But the second one still holds, and I have made a test case for third one:

#include <stdio.h>
#include <windows.h>

char *file1="n:\\test";
char *file2="n:\\test2";

DWORD WINAPI ThreadProc(void*)
{
  while(1)
  {
    MoveFile(file1, file2);
  }
  return 0;
}

int main(int nargs,char *args[])
{
  DWORD tid;
  HANDLE thr=CreateThread(NULL, 0, ThreadProc, 0, 0, &tid);

  while(1)
  {
    HANDLE hFile=CreateFile(file1, GENERIC_WRITE|GENERIC_READ, FILE_SHARE_READ|FILE_SHARE_WRITE, 0, CREATE_NEW, FILE_FLAG_SEQUENTIAL_SCAN, 0);
    if(hFile==INVALID_HANDLE_VALUE)
      continue;
    DWORD tmp;
    WriteFile(hFile, "1", 1, &tmp, 0);
    HANDLE hMap=CreateFileMapping(hFile, 0, PAGE_READONLY, 0, 0, 0);
    char *buf=(char*)MapViewOfFile(hMap, FILE_MAP_READ, 0, 0, 0);
    CloseHandle(hMap);
    CloseHandle(hFile);

    printf("%c\n", buf[0]);
    UnmapViewOfFile(buf);

    DeleteFile(file1);
    DeleteFile(file2);
  }
  return 0;
}

before compiling n: should be replaced with the drive letter where dokan fs is mapped. On normal drives this program in working as expected, but on dokan fs it is crashing after a few cycles.

@Liryna
Copy link
Member

Liryna commented Jul 21, 2015

Is it your software that is crashing or mirrorfs ?
Could you provide mirrorfs logs ?

@storzinc
Copy link
Author

The crash is in my test program, it is crashing on the attempt to read from buf[0]. Here is the log:

Dokan: debug mode on
Dokan: use stderr
device opened
mounted: n -> \Volume{d6cc17c5-1732-4085-bce7-964f1e9f5de9}
###Create 0000
   CreateDisposition 0x00000001
CreateFile : C:\xcache\autorun.inf

    OPEN_EXISTING
    ShareMode = 0x7
    FILE_SHARE_READ
    FILE_SHARE_WRITE
    FILE_SHARE_DELETE
    AccessMode = 0x80
    FILE_READ_ATTRIBUTES
    FlagsAndAttributes = 0x2000000
    FILE_FLAG_BACKUP_SEMANTICS
    error code = 2

CreateFile status = -2
###Create 0001
   CreateDisposition 0x00000001
CreateFile : C:\xcache\AutoRun.inf

    OPEN_EXISTING
    ShareMode = 0x7
    FILE_SHARE_READ
    FILE_SHARE_WRITE
    FILE_SHARE_DELETE
    AccessMode = 0x80
    FILE_READ_ATTRIBUTES
    FlagsAndAttributes = 0x2000000
    FILE_FLAG_BACKUP_SEMANTICS
    error code = 2

CreateFile status = -2
###Create 0002
   CreateDisposition 0x00000001
CreateFile : C:\xcache\autorun.inf

    OPEN_EXISTING
    ShareMode = 0x7
    FILE_SHARE_READ
    FILE_SHARE_WRITE
    FILE_SHARE_DELETE
    AccessMode = 0x80
    FILE_READ_ATTRIBUTES
    FlagsAndAttributes = 0x2000000
    FILE_FLAG_BACKUP_SEMANTICS
    error code = 2

CreateFile status = -2
###Create 0003
   CreateDisposition 0x00000001
CreateFile : C:\xcache\AutoRun.inf

    OPEN_EXISTING
    ShareMode = 0x7
    FILE_SHARE_READ
    FILE_SHARE_WRITE
    FILE_SHARE_DELETE
    AccessMode = 0x80
    FILE_READ_ATTRIBUTES
    FlagsAndAttributes = 0x2000000
    FILE_FLAG_BACKUP_SEMANTICS
    error code = 2

CreateFile status = -2
###Create 0004
OpenDirectory : C:\xcache\

CreateFile status = 0
###QueryVolumeInfo 0004
###QueryVolumeInfo 0004
###Cleanup 0004
Cleanup: C:\xcache\

###Close 0004
Close: C:\xcache\

###Create 0005
   CreateDisposition 0x00000001
CreateFile : C:\xcache\

    OPEN_EXISTING
    ShareMode = 0x7
    FILE_SHARE_READ
    FILE_SHARE_WRITE
    FILE_SHARE_DELETE
    AccessMode = 0x100081
    FILE_READ_DATA
    FILE_READ_ATTRIBUTES
    SYNCHRONIZE
    FlagsAndAttributes = 0x2000000
    FILE_FLAG_BACKUP_SEMANTICS

CreateFile status = 0
###GetFileInfo 0005
GetFileInfo : C:\xcache\
    GetFileInformationByHandle success, file size = 65536

###Create 0006
   CreateDisposition 0x00000001
CreateFile : C:\xcache\

    OPEN_EXISTING
    ShareMode = 0x7
    FILE_SHARE_READ
    FILE_SHARE_WRITE
    FILE_SHARE_DELETE
    AccessMode = 0x100081
    FILE_READ_DATA
    FILE_READ_ATTRIBUTES
    SYNCHRONIZE
    FlagsAndAttributes = 0x2000000
    FILE_FLAG_BACKUP_SEMANTICS

CreateFile status = 0
###GetFileInfo 0006
GetFileInfo : C:\xcache\
    GetFileInformationByHandle success, file size = 65536

###Create 0007
   CreateDisposition 0x00000002
CreateFile : C:\xcache\test
###Create 0008
   CreateDisposition 0x00000001
CreateFile : C:\xcache\test

    CREATE_NEW
    ShareMode = 0x3
    FILE_SHARE_READ
    FILE_SHARE_WRITE
    AccessMode = 0x12019f
    FILE_READ_DATA
    FILE_READ_ATTRIBUTES
    FILE_READ_EA
    READ_CONTROL
    FILE_WRITE_DATA
    FILE_WRITE_ATTRIBUTES
    FILE_WRITE_EA
    FILE_APPEND_DATA
    SYNCHRONIZE
    STANDARD_RIGHTS_READ
    STANDARD_RIGHTS_WRITE
    STANDARD_RIGHTS_EXECUTE

    OPEN_EXISTING
    ShareMode = 0x7
    FILE_SHARE_READ
    FILE_SHARE_WRITE
    FILE_SHARE_DELETE
    AccessMode = 0x110080
    FlagsAndAttributes = 0x2000000
    DELETE
    FILE_FLAG_BACKUP_SEMANTICS
    FILE_READ_ATTRIBUTES
    SYNCHRONIZE
    FlagsAndAttributes = 0x0
    error code = 32

CreateFile status = -32

CreateFile status = 0
###Create 0009
   CreateDisposition 0x00000001
CreateFile : C:\xcache\test
###WriteFile 0007
WriteFile : C:\xcache\test, offset 0, length 1
    write 1, offset 0

###GetFileInfo 0007
GetFileInfo : C:\xcache\test

    OPEN_EXISTING
    ShareMode = 0x7
    FILE_SHARE_READ
    FILE_SHARE_WRITE
    FILE_SHARE_DELETE
    AccessMode = 0x110080
    GetFileInformationByHandle success, file size = 1
    DELETE

    FILE_READ_ATTRIBUTES
    SYNCHRONIZE
    FlagsAndAttributes = 0x0
###Cleanup 0007
Cleanup: C:\xcache\test

    error code = 32

CreateFile status = -32
###Create 0010
   CreateDisposition 0x00000001
CreateFile : C:\xcache\test
###Read 0007
ReadFile : C:\xcache\test
    invalid handle, cleanuped?
    read 1, offset 0


    OPEN_EXISTING
    ShareMode = 0x7
    FILE_SHARE_READ
    FILE_SHARE_WRITE
    FILE_SHARE_DELETE
    AccessMode = 0x110080
    DELETE
    FILE_READ_ATTRIBUTES
###Create 0011
    SYNCHRONIZE
   CreateDisposition 0x00000001
CreateFile : C:\xcache\test
    FlagsAndAttributes = 0x0

CreateFile status = 0
###GetFileInfo 0010
GetFileInfo : C:\xcache\test
    GetFileInformationByHandle success, file size = 1

###GetFileInfo 0010
GetFileInfo : C:\xcache\test
    GetFileInformationByHandle success, file size = 1

###Create 0012
   CreateDisposition 0x00000001
CreateFile : C:\xcache\test2

    OPEN_EXISTING
    ShareMode = 0x7
    FILE_SHARE_READ
    FILE_SHARE_WRITE
    FILE_SHARE_DELETE
    AccessMode = 0x10080
    DELETE
    FILE_READ_ATTRIBUTES
    FlagsAndAttributes = 0x0

CreateFile status = 0

    OPEN_EXISTING
###GetFileInfo 0011
    ShareMode = 0x3
GetFileInfo : C:\xcache\test
    FILE_SHARE_READ
    FILE_SHARE_WRITE
    AccessMode = 0x100002
    FILE_WRITE_DATA
    SYNCHRONIZE
    GetFileInformationByHandle success, file size = 1

    FlagsAndAttributes = 0x2000000
    FILE_FLAG_BACKUP_SEMANTICS
###SetFileInfo 0011
DeleteFile C:\xcache\test
  dispositionInfo->DeleteFile = 1
    error code = 2

CreateFile status = -2
SL_OPEN_TARGET_DIRECTORY specified
###SetFileInfo 0010
MoveFile C:\xcache\test -> C:\xcache\test2

###Close 0007
Close: C:\xcache\test

###Cleanup 0011
Cleanup: C:\xcache\test

    DeleteOnClose
  DeleteFile success

###Close 0011
Close: C:\xcache\test

###Create 0013
   CreateDisposition 0x00000001
CreateFile : C:\xcache\test2
###Cleanup 0012
Cleanup: C:\xcache\test2
    invalid handle

###Close 0012
Close: C:\xcache\test2

###Cleanup 0010
Cleanup: C:\xcache\test2
    invalid handle

###Close 0010
Close: C:\xcache\test2

###Create 0014
   CreateDisposition 0x00000001
CreateFile : C:\xcache\test

    OPEN_EXISTING
    ShareMode = 0x7
    FILE_SHARE_READ
    FILE_SHARE_WRITE
    FILE_SHARE_DELETE
    AccessMode = 0x110080
    DELETE
    FILE_READ_ATTRIBUTES
    SYNCHRONIZE
    FlagsAndAttributes = 0x2000000
    FILE_FLAG_BACKUP_SEMANTICS
    error code = 2

CreateFile status = -2
###Create 0015
   CreateDisposition 0x00000001
CreateFile : C:\xcache\test

    OPEN_EXISTING
    ShareMode = 0x7
    FILE_SHARE_READ
    FILE_SHARE_WRITE
    FILE_SHARE_DELETE
    AccessMode = 0x110080
    DELETE
    FILE_READ_ATTRIBUTES
    SYNCHRONIZE
    FlagsAndAttributes = 0x2000000
    FILE_FLAG_BACKUP_SEMANTICS
    error code = 2

CreateFile status = -2
###Create 0016
   CreateDisposition 0x00000001
CreateFile : C:\xcache\test

    OPEN_EXISTING
    ShareMode = 0x7
    FILE_SHARE_READ
    FILE_SHARE_WRITE
    FILE_SHARE_DELETE
    AccessMode = 0x110080
    DELETE
    FILE_READ_ATTRIBUTES
    SYNCHRONIZE
    FlagsAndAttributes = 0x2000000
    FILE_FLAG_BACKUP_SEMANTICS
    error code = 2

CreateFile status = -2
###Create 0017
   CreateDisposition 0x00000001
CreateFile : C:\xcache\test

    OPEN_EXISTING
    ShareMode = 0x7
    FILE_SHARE_READ
    FILE_SHARE_WRITE
    FILE_SHARE_DELETE
    AccessMode = 0x110080
    DELETE
    FILE_READ_ATTRIBUTES
    SYNCHRONIZE
    FlagsAndAttributes = 0x2000000
    FILE_FLAG_BACKUP_SEMANTICS
    error code = 2

CreateFile status = -2
###Create 0018
   CreateDisposition 0x00000001
CreateFile : C:\xcache\test

    OPEN_EXISTING
    ShareMode = 0x7
    FILE_SHARE_READ
    FILE_SHARE_WRITE
    FILE_SHARE_DELETE
    AccessMode = 0x10080
    DELETE
    FILE_READ_ATTRIBUTES
    FlagsAndAttributes = 0x2000000
    FILE_FLAG_BACKUP_SEMANTICS
    error code = 2

CreateFile status = -2
###Create 0019
   CreateDisposition 0x00000002
CreateFile : C:\xcache\test

    OPEN_EXISTING
    ShareMode = 0x7
    FILE_SHARE_READ
    FILE_SHARE_WRITE
    FILE_SHARE_DELETE
    AccessMode = 0x110080
    DELETE
    FILE_READ_ATTRIBUTES
    SYNCHRONIZE

    CREATE_NEW
    ShareMode = 0x3
    FILE_SHARE_READ
    FILE_SHARE_WRITE
    AccessMode = 0x12019f
    FILE_READ_DATA
    FILE_READ_ATTRIBUTES
    FILE_READ_EA
    READ_CONTROL
    FILE_WRITE_DATA
    FILE_WRITE_ATTRIBUTES
    FILE_WRITE_EA
    FILE_APPEND_DATA
    SYNCHRONIZE
    STANDARD_RIGHTS_READ
    STANDARD_RIGHTS_WRITE
    STANDARD_RIGHTS_EXECUTE
    FlagsAndAttributes = 0x2000000
    FILE_FLAG_BACKUP_SEMANTICS
    FlagsAndAttributes = 0x2000000
    FILE_FLAG_BACKUP_SEMANTICS
    error code = 2

CreateFile status = -2

CreateFile status = 0
###Create 0020
   CreateDisposition 0x00000001
CreateFile : C:\xcache\test
###WriteFile 0019
WriteFile : C:\xcache\test, offset 0, length 1
    write 1, offset 0

###GetFileInfo 0019
GetFileInfo : C:\xcache\test
    GetFileInformationByHandle success, file size = 1


    OPEN_EXISTING
    ShareMode = 0x7
    FILE_SHARE_READ
    FILE_SHARE_WRITE
    FILE_SHARE_DELETE
    AccessMode = 0x110080
    DELETE
    FILE_READ_ATTRIBUTES
    SYNCHRONIZE
###Cleanup 0019
Cleanup: C:\xcache\test

    FlagsAndAttributes = 0x0

CreateFile status = 0
###Read 0019
ReadFile : C:\xcache\test
###GetFileInfo 0020
    invalid handle, cleanuped?
GetFileInfo : C:\xcache\test
    GetFileInformationByHandle success, file size = 1
    CreateFile error : 32

In short, when MirrorReadFile is called, file handle is already closed, and this function is trying to reopen it using FileName but at that moment the file is already renamed and this operation fails.

@Liryna
Copy link
Member

Liryna commented Jul 21, 2015

Thank for the logs !

But what do you expect to happen ? Your file is open with FILE_SHARE_DELETE.
The file has been moved, it is normal that the read fail.

I do not see your point sorry :(

@storzinc
Copy link
Author

But without dokan windows working just fine after rename. My point is that file should be really closed only after the program is done with it, including access through memory map (in MirrorCloseFile instead of MirrorCleanup maybe?) closing it and then reopening is not reliable.

@Liryna
Copy link
Member

Liryna commented Jul 21, 2015

OK I see,
Do you run mirrorfs with more than a thread ?

@storzinc
Copy link
Author

I have been starting mirror.exe with the default thread count, but even with /t 1 argument the problem is still persist.

@Liryna
Copy link
Member

Liryna commented Jul 21, 2015

I guess this could come from IRP race in kernel driver.
We should look at https://msdn.microsoft.com/en-us/library/windows/hardware/ff560773%28v=vs.85%29.aspx

@storzinc
Copy link
Author

I am not very familiar with the kernel programming, but I have made a simpler test case demonstrating this problem, without multiple threads accessing the same file:

#include <stdio.h>
#include <windows.h>

char *file1="n:\\test";
int main(int nargs,char *args[])
{
  HANDLE hFile=CreateFile(file1, GENERIC_WRITE|GENERIC_READ, FILE_SHARE_READ, 0, CREATE_ALWAYS, FILE_FLAG_SEQUENTIAL_SCAN, 0);
  DWORD tmp;
  WriteFile(hFile, "1", 1, &tmp, 0);
  HANDLE hMap=CreateFileMapping(hFile, 0, PAGE_READWRITE, 0, 0, 0);
  char *buf=(char*)MapViewOfFile(hMap, FILE_MAP_WRITE, 0, 0, 0);
  CloseHandle(hFile);
  CloseHandle(hMap);

  HANDLE hFile2=CreateFile(file1, GENERIC_WRITE|GENERIC_READ, FILE_SHARE_READ, 0, CREATE_ALWAYS, FILE_FLAG_SEQUENTIAL_SCAN, 0);
  printf("%x %x\n", hFile2, GetLastError());

  UnmapViewOfFile(buf);
}

On the regular fs second CreateFile fails with ERROR_SHARING_VIOLATION error (as it should), but on the mirrored fs it actually succeed.

So it seems to be not a good idea to close file handle in the MirrorCleanup

@Maxhy
Copy link
Member

Maxhy commented Jul 22, 2015

I agree, it is a race condition on mirror sample (not on driver neither on the library).
I dont think handle should be closed on MirrorCloseFile but only on Cleanup, could you try by removing the code on MirrorCloseFile (just return 0)? I will give a try later.

@storzinc
Copy link
Author

@Maxhy doesn't s seems to help, MirrorCloseFile is printing error : not cleanuped file when it is closing a handle, but there was no such error in the log.

@suy
Copy link

suy commented Jul 22, 2015

So far in my application I am closing the file handle (one internal to my application, not a real file system one) on CloseFile instead of Cleanup. This seems to work fine (although I am not using the file system very aggressively), but this is against what the documentation says, and what some messages on the mailing list said.

According to the docs:

"If the user application calls CloseHandle and subsequently opens the same file, the CloseFile function of the file system application may not be invoked before the CreateFile API is called. This may cause a sharing violation error."

@Liryna
Copy link
Member

Liryna commented Jul 22, 2015

@suy I am doing the same as you.

More informations about cleanup/close difference:
https://support.microsoft.com/en-us/kb/120170

From what I have understand:
Cleanup IRP is called with CloseHandle and when no other softwares are using the same file.
But the handle created during CreateFile have to be closed only in Close IRP.

Last part of IRP_MJ_CLOSE:
The close routine is not called until all of the outstanding I/O requests are completed. Apparently, if the driver performs as planned in the IRP_MJ_CLEANUP routine, as discussed earlier, the IRP_MJ_CLOSE routine is called at the right time.
Does this could be the @storzinc case ?

@storzinc
Copy link
Author

storzinc commented Aug 3, 2015

Is there any update on this? Right now there seems to be no way to properly handle memory mapped files in dokan, MirrorCleanup is called too soon to close the handle in it, but MirrorCloseFile is called too late...

@suy
Copy link

suy commented Aug 4, 2015

My application seems to work fine with what I said: close the handle in CloseFile instead. I don't know if in some conditions I can get a sharing violation error with it, though (that's what the original documentation said), but I don't know how to provoke that.

@storzinc
Copy link
Author

storzinc commented Aug 4, 2015

I have tried moving all the code in mirror sample from MirrorCleanup to MirrorCloseFile and then executing this test program:

#include <stdio.h>
#include <windows.h>

char *file1="n:\\test";
int main(int nargs,char *args[])
{
  HANDLE hFile=CreateFile(file1, GENERIC_WRITE|GENERIC_READ, FILE_SHARE_READ, 0, CREATE_ALWAYS, FILE_FLAG_SEQUENTIAL_SCAN, 0);
  printf("%x %x\n", hFile, GetLastError());
  if(hFile==INVALID_HANDLE_VALUE)
    return 0;

  DWORD tmp;
  WriteFile(hFile, "1", 1, &tmp, 0);
  HANDLE hMap=CreateFileMapping(hFile, 0, PAGE_READWRITE, 0, 0, 0);
  char *buf=(char*)MapViewOfFile(hMap, FILE_MAP_WRITE, 0, 0, 0);
  CloseHandle(hFile);
  CloseHandle(hMap);

  printf("%c\n", buf[0]);
  UnmapViewOfFile(buf);
}

On the first run all seems to be okay, but on the second CreateFile is failing with ERROR_SHARING_VIOLATION. So it seems that MirrorCloseFile was never called, even after termination of the original process which have created the file.

@Liryna
Copy link
Member

Liryna commented Aug 6, 2015

Sorry for the delay @storzinc .

Here is the debug output after two run:

[DokanFS] ==> DokanCreate
[DokanFS]   ProcessId 2172
[DokanFS]   FileName:\test.txt
[DokanFS]    IdType = VCB
[DokanFS]   IrpSp->Flags = 0
[DokanFS]   Allocate FCB
[DokanFS] <== DokanCreate
[DokanFS] ==> DokanWrite
[DokanFS]   ProcessId 2172
[DokanFS]   FileName: \test.txt FCB.FileName: \test.txt
[DokanFS]   use MdlAddress
[DokanFS]   Synchronous IO
[DokanFS]    Offset 0:0, Length 1
[DokanFS] <== DokanWrite
[DokanFS] ==> DokanQueryInformation
[DokanFS]   FileInfoClass 5
[DokanFS]   ProcessId 2172
[DokanFS]   FileName: \test.txt FCB.FileName: \test.txt
[DokanFS]   FileStandardInformation
[DokanFS] ==> DokanCleanup
[DokanFS]   ProcessId 2172
[DokanFS]   FileName: \test.txt FCB.FileName: \test.txt
[DokanFS] <== DokanCleanup
[DokanFS] ==> DokanRead
[DokanFS]   ProcessId 2172
[DokanFS]   FileName: \test.txt FCB.FileName: \test.txt
[DokanFS]   ByteCount:4096 ByteOffset:0
[DokanFS]   Paging IO
[DokanFS]   Synchronous IO
[DokanFS]   Nocache
[DokanFS] <== DokanRead

[DokanFS] ==> DokanCreate
[DokanFS]   ProcessId 2380
[DokanFS]   FileName:\test.txt
[DokanFS]    IdType = VCB
[DokanFS]   IrpSp->Flags = 0
[DokanFS] <== DokanCreate
[DokanFS] ==> DokanWrite
[DokanFS]   ProcessId 2380
[DokanFS]   FileName: \test.txt FCB.FileName: \test.txt
[DokanFS]   use MdlAddress
[DokanFS]   Synchronous IO
[DokanFS]    Offset 0:0, Length 1
[DokanFS] <== DokanWrite
[DokanFS] ==> DokanQueryInformation
[DokanFS]   FileInfoClass 5
[DokanFS]   ProcessId 2380
[DokanFS]   FileName: \test.txt FCB.FileName: \test.txt
[DokanFS]   FileStandardInformation
[DokanFS] <== DokanQueryInformation
[DokanFS] ==> DokanCleanup
[DokanFS]   ProcessId 2380
[DokanFS]   FileName: \test.txt FCB.FileName: \test.txt
[DokanFS] <== DokanCleanup
[DokanFS] ==> DokanClose
[DokanFS]   ProcessId 2380
[DokanFS]   FileName: \test.txt FCB.FileName: \test.txt
[DokanFS]    UserContext:2C8D10E0
[DokanFS]    Free CCB:2FA3D460
[DokanFS]   status = STATUS_SUCCESS
[DokanFS] <== DokanClose

MirroFS


###Create 0005
   CreateDisposition 0x00000005
CreateFile : C:\Users\test.txt
  AccountName: Liryna, DomainName: WIN-9RM07VT4S97
        CREATE_ALWAYS
        ShareMode = 0x1
        FILE_SHARE_READ
        AccessMode = 0x12019f
        FILE_READ_DATA
        FILE_READ_ATTRIBUTES
        FILE_READ_EA
        READ_CONTROL
        FILE_WRITE_DATA
        FILE_WRITE_ATTRIBUTES
        FILE_WRITE_EA
        FILE_APPEND_DATA
        SYNCHRONIZE
        STANDARD_RIGHTS_READ
        STANDARD_RIGHTS_WRITE
        STANDARD_RIGHTS_EXECUTE
        FlagsAndAttributes = 0x0

CreateFile status = 0
###WriteFile 0005
WriteFile : C:\Users\test.txt, offset 0, length 1
        write 1, offset 0

###GetFileInfo 0005
GetFileInfo : C:\Users\test.txt
        GetFileInformationByHandle success, file size = 1

###Cleanup 0005
Cleanup: C:\Users\test.txt

###Read 0005
ReadFile : C:\Users\test.txt
        invalid handle, cleanuped?
        read 1, offset 0

###Create 0006
   CreateDisposition 0x00000005
CreateFile : C:\Users\test.txt
  AccountName: Liryna, DomainName: WIN-9RM07VT4S97
        CREATE_ALWAYS
        ShareMode = 0x1
        FILE_SHARE_READ
        AccessMode = 0x12019f
        FILE_READ_DATA
        FILE_READ_ATTRIBUTES
        FILE_READ_EA
        READ_CONTROL
        FILE_WRITE_DATA
        FILE_WRITE_ATTRIBUTES
        FILE_WRITE_EA
        FILE_APPEND_DATA
        SYNCHRONIZE
        STANDARD_RIGHTS_READ
        STANDARD_RIGHTS_WRITE
        STANDARD_RIGHTS_EXECUTE
        FlagsAndAttributes = 0x0

CreateFile status = 0
###WriteFile 0006
WriteFile : C:\Users\test.txt, offset 0, length 1
        write 1, offset 0

###GetFileInfo 0006
GetFileInfo : C:\Users\test.txt
        GetFileInformationByHandle success, file size = 1

###Cleanup 0006
Cleanup: C:\Users\test.txt

###Close 0006
Close: C:\Users\test.txt

The Dokan seems to be never called to Close the first handle by Windows Kernel for an unknown reason.

@Liryna
Copy link
Member

Liryna commented Aug 7, 2015

@storzinc I solved the issue by moving UnmapViewOfFile before CloseHandle.

#include <stdio.h>
#include <windows.h>

char *file1 = "m:\\test.txt";
int main(int nargs, char *args[])
{
    HANDLE hFile = CreateFile(L"m:\\test2.txt", GENERIC_WRITE | GENERIC_READ, FILE_SHARE_READ, 0, CREATE_ALWAYS, FILE_FLAG_SEQUENTIAL_SCAN, 0);
    printf("%x %x\n", hFile, GetLastError());
    if (hFile == INVALID_HANDLE_VALUE)
        return 0;

    DWORD tmp;
    WriteFile(hFile, "1", 1, &tmp, 0);
    HANDLE hMap = CreateFileMapping(hFile, 0, PAGE_READWRITE, 0, 0, 0);
    char *buf = (char*)MapViewOfFile(hMap, FILE_MAP_WRITE, 0, 0, 0);

    printf("%c\n", buf[0]);

    UnmapViewOfFile(buf);
    CloseHandle(hFile);
    CloseHandle(hMap);
}

I successfully get a call from close after the cleanup at each launch of this test code.

Observation about your example:

It is right that close in not called after cleanup but this only happening for the first test, every other run successfully get a call from close after the cleanup.

Therefore, to fully close a file mapping object, an application must unmap all mapped views
of the file mapping object by calling UnmapViewOfFile and close the file mapping object
handle by calling CloseHandle. These functions can be called in any order.
https://msdn.microsoft.com/en-us/library/windows/desktop/aa366537%28v=vs.85%29.aspx

So even if the Microsoft documentation say that it can be called in any order, it seems that there is a different behaviour in the windows kernel when UnmapViewOfFile is called after CloseHandle.

Does this seems alright for you ?

@storzinc
Copy link
Author

storzinc commented Aug 7, 2015

In windows it is perfectly legal to close all of the file handles and work only with the memory map, many programs are relying on this, I have moved UnmapViewOfFile to the end just to demonstrate the problem with dokan. And either way, even if the program did something wrong, there should be no access violations after the process is terminated.

@Liryna
Copy link
Member

Liryna commented Aug 7, 2015

It is right that close in not called after cleanup but this only happening for the first test, every other run successfully get a call from close after the cleanup.

This part of the observation is not related to dokan but every file system driver.
If you take a look at dokan code, you will understand from the kernel logs I have provide that Dokan is not called at ALL by the kernel for the first file of the file system.

If you have any informations/fix to provide, let me know and I will look more about it. For me this issue is related to Windows and it should forward to them.

@storzinc
Copy link
Author

storzinc commented Aug 7, 2015

I have tried to figure out how memory mapping is working on the regular fs, without dokan, and I got interesting result with Process Monitor (https://technet.microsoft.com/en-us/library/bb896645.aspx)

With this test program:

#include <stdio.h>
#include <windows.h>

int main(int nargs,char *args[])
{
  HANDLE hFile=CreateFile("test.txt", GENERIC_WRITE|GENERIC_READ, FILE_SHARE_READ, 0, CREATE_ALWAYS, FILE_FLAG_SEQUENTIAL_SCAN, 0);
  DWORD tmp;
  WriteFile(hFile, "1", 1, &tmp, 0);
  HANDLE hMap=CreateFileMapping(hFile, 0, PAGE_READWRITE, 0, 0, 0);
  char *buf=(char*)MapViewOfFile(hMap, FILE_MAP_WRITE, 0, 0, 0);
  CloseHandle(hFile);
  CloseHandle(hMap);

  printf("%c\n", buf[0]);


  HANDLE hFile2=CreateFile("test.txt", GENERIC_WRITE|GENERIC_READ, FILE_SHARE_READ, 0, CREATE_ALWAYS, FILE_FLAG_SEQUENTIAL_SCAN, 0);
  printf("%x %x\n", hFile2, GetLastError());

  UnmapViewOfFile(buf);

  hFile2=CreateFile("test.txt", GENERIC_WRITE|GENERIC_READ, FILE_SHARE_READ, 0, CREATE_ALWAYS, FILE_FLAG_SEQUENTIAL_SCAN, 0);
  printf("%x %x\n", hFile2, GetLastError());
}

I get this log with Process Monitor:

Time Operation Path Result
43:56,455 IRP_MJ_CREATE C:\15.08.07 18_30_17\test.txt SUCCESS
43:56,455 IRP_MJ_WRITE C:\15.08.07 18_30_17\test.txt SUCCESS
43:56,456 FASTIO_ACQUIRE_FOR_SECTION_ SYNCHRONIZATION C:\15.08.07 18_30_17\test.txt FILE LOCKED WITH WRITERS
43:56,456 FASTIO_QUERY_INFORMATION C:\15.08.07 18_30_17\test.txt SUCCESS
43:56,456 FASTIO_RELEASE_FOR_SECTION_ SYNCHRONIZATION C:\15.08.07 18_30_17\test.txt SUCCESS
43:56,456 FASTIO_ACQUIRE_FOR_SECTION_ SYNCHRONIZATION C:\15.08.07 18_30_17\test.txt SUCCESS
43:56,456 FASTIO_RELEASE_FOR_SECTION_ SYNCHRONIZATION C:\15.08.07 18_30_17\test.txt SUCCESS
43:56,456 IRP_MJ_CLEANUP C:\15.08.07 18_30_17\test.txt SUCCESS
43:56,456 IRP_MJ_CREATE C:\15.08.07 18_30_17\test.txt SHARING VIOLATION
43:56,456 IRP_MJ_QUERY_SECURITY C:\15.08.07 18_30_17 SUCCESS
43:56,456 IRP_MJ_CREATE C:\15.08.07 18_30_17\test.txt SUCCESS
...
43:56,457 Thread Exit SUCCESS
...
43:56,459 Process Exit SUCCESS
...
43:56,460 IRP_MJ_CLEANUP C:\15.08.07 18_30_17\test.txt SUCCESS
43:56,460 IRP_MJ_CLOSE C:\15.08.07 18_30_17\test.txt SUCCESS
43:56,460 IRP_MJ_CLOSE C:\15.08.07 18_30_17\test.txt SUCCESS

Here IRP_MJ_CLEANUP was sent before the second IRP_MJ_CREATE, and IRP_MJ_CLOSE was sent after the third, but still, system somehow knew that the second IRP_MJ_CREATE should be rejected with SHARING VIOLATION and third should be accepted.

Maybe there is another API to work with memory mapping that the system is using?

@marinkobabic
Copy link
Contributor

Dokany driver actually never raises the SHARING VIOLATION because the driver does actually not implement the ReferenceCount and OpendHandleCount

dokany/sys/dokan.h

Lines 258 to 259 in 837633a

//uint32 ReferenceCount;
//uint32 OpenHandleCount;

What is the expected timeframe for the implementation of the missing functionality from your point of view?

@Liryna
Copy link
Member

Liryna commented Aug 10, 2015

Ah you are right 😄 !
It is the FS that have to handle STATUS_SHARING_VIOLATION by checking the count reference.
https://github.com/Microsoft/Windows-driver-samples/blob/2e757e5eea3cecf70d1f2b3030bb5ce55540cd3c/filesys/fastfat/create.c#L2229-L2242

I just do not get the difference of OpenCount (ReferenceCount ?) & OpenFileCount(OpendHandleCount ?) in their example for now.
ReadOnlyCount has to been handle as well.

@marinkobabic
Copy link
Contributor

ReferenceCount is the actual FileCount in the dokan code. You can reference a file for various operations (not only open) and this prevents the fcb from being freed. OpenFileCount is meant for how many times is the file opened. If you try to open the file > 0 times, then the check should be performed.

In the example above provided by microsoft fastfat the count is used for the volume and not for the file it self. But the code would look the similar way.

@marinkobabic
Copy link
Contributor

The problem I actually see is that CreateFileMapping does not send an IRP request to the filesystem. So the file system has no idea, that the file is opened a second time. On NTFS filesystem this works properly whyever 😟

In the actual version of dokan I think when you do a CreateFile of the same file two times (without to close the handle), even then you will not get SHARED_ACCESS_VIOLATION. This is something which can be fixed.

@Liryna
Please can you confirm my second statement?

@Liryna
Copy link
Member

Liryna commented Aug 19, 2015

@marinkobabic I think CreateFileMapping is using the file context of CreateFile since the handle is given as argument. The issue come from CreateFileMapping is not cleaning well the context of the file in some case, so it stay open for the file system.

You are right about the second statement.
Dokan does not check if there is a sharing acces conflict in CreateFile with the previous calls.

@marinkobabic
Copy link
Contributor

@Liryna
This is the reason that OpenFileCount can't be increased by filesystem, because it has no idea about it. Please check my last checking of my fork.

@Liryna
Copy link
Member

Liryna commented Aug 19, 2015

@marinkobabic I do not see new commit in your fork 😢

@marinkobabic
Copy link
Contributor

@Liryna Should now be available

@Liryna
Copy link
Member

Liryna commented Aug 19, 2015

@marinkobabic Do you have a direct link ?
Because I see nothing in your old fork https://github.com/marinkobabic/dokanx/tree/Windows8DeleteIssue

@marinkobabic
Copy link
Contributor

@Liryna
The change is 6 days old https://github.com/marinkobabic/dokanx/tree/Windows8DeleteIssue/sys
Please check my changes and test it well :-)

@Liryna
Copy link
Member

Liryna commented Aug 19, 2015

@marinkobabic Oh sorry
I have add your changes to the develop branch:
ac016e9

Output of the ConsoleApplication3 (it is the last example of @storzinc)

1
3c 0
ffffffff 20

DokanFS Logs http://pastebin.com/np96j8rb
We can see that STATUS_SHARING_VIOLATION is return for the last createfile.
But normally it should be the second who return STATUS_SHARING_VIOLATION and the third to succeed.

@marinkobabic
Copy link
Contributor

The first handle is closed as you can see above

CloseHandle(hFile);
CloseHandle(hMap);

@Liryna
Copy link
Member

Liryna commented Aug 19, 2015

Yes it is right but if I run it on NTFS:

1
ffffffff 20
3c b7

The STATUS_SHARING_VIOLATION happen in the second createfile.

@marinkobabic
Copy link
Contributor

You are right, this is because I have no idea how to manage the CreateFileMapping. On NTFS event if you close the hand to the file and hMap and you did not UnmapViewOfFile the handle somehow remains. This is the reason why you get on NTFS the exception on the second file and dokan returns the exception on the third file. Dokan gets only notified when you do CreateFile and CloseHandle.

@Liryna
Copy link
Member

Liryna commented Aug 19, 2015

I was thinking ReferenceCount should do the job.
Since the first close is called after the second createfile. We could manage to check in the second createfile that there is a sharing issue and we would return the error ourself.

@xaqq
Copy link

xaqq commented Aug 19, 2015

What happens if you use FAT as the underlying file system?

On Wed, Aug 19, 2015 at 5:44 PM, Marinko notifications@github.com wrote:

You are right, this is because I have no idea how to manage the
CreateFileMapping. On NTFS event if you close the hand to the file and hMap
and you did not UnmapViewOfFile the handle somehow remains. This is the
reason why you get on NTFS the exception on the second file and dokan
returns the exception on the third file. Dokan gets only notified when you
do CreateFile and CloseHandle.


Reply to this email directly or view it on GitHub
#32 (comment).

Kapp Arnaud - Xaqq

@marinkobabic
Copy link
Contributor

@Liryna
This is not an option. I have seen now, that it may work when we catch the IRP_MJ_ACQUIRE_FOR_SECTION_SYNCHRONIZATION. But we need to investigate this one to get it work.

@marinkobabic
Copy link
Contributor

Seems to be an IRP for minifilter. But there must be one for file system driver.

@Liryna
Copy link
Member

Liryna commented Aug 20, 2015

From what it worth, fat32 have the same behaviour.

@Liryna Liryna added the Bug label Aug 20, 2015
@Liryna
Copy link
Member

Liryna commented Aug 21, 2015

@marinkobabic Since your changes "implement shareaccess" does not affect this issue, have you seen some case where it work ?
I just wanna know if this changes should be include in the next release.

@marinkobabic
Copy link
Contributor

@Liryna
Before the change you can do CreateFile as many times as you want, you will get always SUCCESS message. The change checks the share, if you open the same file a second time.

SectionObjectPointers have the information about Memory Manager and if the file is memory mapped. But we need to implement it in a clean way.

I would suggest to keep the change in the development branch, except somebody requests the change. Just put it on your todo list.

@Liryna Liryna mentioned this issue Aug 21, 2015
12 tasks
@Liryna
Copy link
Member

Liryna commented Aug 21, 2015

Thank you for the clear description ! Added to the TODO

@Liryna
Copy link
Member

Liryna commented Feb 1, 2016

I have created a ticket with the main issue we face here(#152)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

6 participants