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

DeleteFile() is never called #1082

Closed
3 of 5 tasks
onexzero opened this issue Mar 25, 2022 · 15 comments
Closed
3 of 5 tasks

DeleteFile() is never called #1082

onexzero opened this issue Mar 25, 2022 · 15 comments

Comments

@onexzero
Copy link

onexzero commented Mar 25, 2022

Feature request can skip this form. Bug report must complete it. Check List must be 100% match or it will be automatically closed without further discussion. Please remove this line.

Environment

  • Windows version: Window 10
  • Processor architecture: AMD 5800X
  • Dokany version: 2.0.3.1
  • Library type (Dokany/FUSE): DokAny

Check List

  • I checked my issue doesn't exist yet
  • My issue is valid with mirror default sample and not specific to my user-mode driver implementation
  • I can always reproduce the issue with the provided description below.
  • I have updated Dokany to the latest version and have reboot my computer after.
  • I tested one of the last snapshot from appveyor CI

Description

I found some similar issue with related this but i report.
I think all file deletion call's currently processed in cleanup() with delete_on_close. but this is not right way.
This will give the application no control over file deletion. When SetDispositionInformationFile is called with DELETE flag, DeleteFile() on the user side must be explicitly called.

Logs

Please attach in separate files: mirror output, library logs and kernel logs.
In case of BSOD, please attach minidump or dump analyze output.

@Liryna
Copy link
Member

Liryna commented Mar 25, 2022

This is the Windows design.
I am not sure what is the issue here. You can deny the delete during DeleteFile
https://dokan-dev.github.io/dokany-doc/html/struct_d_o_k_a_n___o_p_e_r_a_t_i_o_n_s.html#abe28c21be4be0c61148c9adc631bb5db

@onexzero
Copy link
Author

I want to handle it, but the problem is that MirrorDeleteFile () is not called.

@Liryna
Copy link
Member

Liryna commented Mar 26, 2022

It must be createfile called with delete on close in that case.
You can also return an error during createfile if you want to refuse de deletion

@onexzero
Copy link
Author

I know that.. However, if you do this, the user will not receive a deletion error message.

@Liryna
Copy link
Member

Liryna commented Mar 26, 2022

The user will have message if the app issuing the delete displays one. Dokany cannot do anything here.

@onexzero
Copy link
Author

The current Mirror sample is implemented so that the read-only file can not be deleted. However, if you delete a read-only file from Windows Explorer, you will see just deleted without displaying any message. But when renewal, it appears again. This is not normal.

@Liryna
Copy link
Member

Liryna commented Mar 27, 2022

If the user implementation allows the CreateFile that deletes the read only file to succeed, it is normal there is no error displayed since it allowed it.
Explorer is not going to question what the user filesystem decided.

@Liryna Liryna closed this as completed Mar 27, 2022
@onexzero
Copy link
Author

I cannot agree with what you said. Mirror sample is implemented to could not delete read-only files, and are not actually deleted.
In Windows Explorer, it appears as if it were deleted, but when you refresh, the files that disappeared are displayed again.
Is this nomal? When this case error message should displayed from windows explorer. and dokan should return appropriate code to explorer to ensure this.

MirrorDeleteFile() <--- Never called when delete read-only file from windows explorer.

and you can see below code in MirrorCreateFile().

// Cannot delete a read only file
if ((fileAttr != INVALID_FILE_ATTRIBUTES &&
     (fileAttr & FILE_ATTRIBUTE_READONLY) ||
     (fileAttributesAndFlags & FILE_ATTRIBUTE_READONLY)) &&
    (fileAttributesAndFlags & FILE_FLAG_DELETE_ON_CLOSE))
  return STATUS_CANNOT_DELETE;

@Liryna
Copy link
Member

Liryna commented Mar 28, 2022

MirrorDeleteFile() <--- Never called when delete read-only file from windows explorer.

It does not need. Explorer open the file with DeleteOnClose which means the file needs to be deleted during cleanup.
Mirror has the code you pointed to deny the delete of read only files like on NTFS. It is actually tested by Microsoft own tool IFTEST ReadOnlyAttributeTest .

In Windows Explorer, it appears as if it were deleted, but when you refresh, the files that disappeared are displayed again.

I am not able to reproduce this. What are the steps ?

@onexzero
Copy link
Author

mirror /r C:\Users\Raccoon\Desktop\Temp /l r /d

DeleteFile.mp4

"Explorer open the file with DeleteOnClose which means the file needs to be deleted during cleanup. "
<--
If you test this part on a local drive rather than a mirror drive, you can see that the OS first calls setdispositionInformationfile with delete=true. But there is no handling of DokanDispatchSetInformation() FileInformationClass==FileDispositionInformation of dokan driver. So I'm not sure, but I believe CreateFile is called again with DELETE_ON_CLOSE. I think this is also the reason why MirrorDeleteFile() is not called.

@onexzero
Copy link
Author

onexzero commented Mar 28, 2022

NTSTATUS
DokanSetDispositionInformation(PEVENT_CONTEXT EventContext,
                               PDOKAN_FILE_INFO FileInfo,
                               PDOKAN_OPERATIONS DokanOperations) {

  BOOLEAN DeleteFileFlag = FALSE;
  NTSTATUS result;

  switch (EventContext->Operation.SetFile.FileInformationClass) {
  case FileDispositionInformation: {
    PFILE_DISPOSITION_INFORMATION dispositionInfo =
        (PFILE_DISPOSITION_INFORMATION)(
            (PCHAR)EventContext + EventContext->Operation.SetFile.BufferOffset);
    DeleteFileFlag = dispositionInfo->DeleteFile;
  } break;
  case FileDispositionInformationEx: {
    PFILE_DISPOSITION_INFORMATION_EX dispositionexInfo =
        (PFILE_DISPOSITION_INFORMATION_EX)(
            (PCHAR)EventContext + EventContext->Operation.SetFile.BufferOffset);

    DeleteFileFlag = (dispositionexInfo->Flags & FILE_DISPOSITION_DELETE) != 0;
  } break;
  default:
    return STATUS_INVALID_PARAMETER;
  }

  if (!DokanOperations->DeleteFile || !DokanOperations->DeleteDirectory)
    return STATUS_NOT_IMPLEMENTED;

>   **//below 3-line of code are very strange. 
>   if (DeleteFileFlag == FileInfo->DeleteOnClose) {
>     return STATUS_SUCCESS;
>   }**



>   // These  are no effect at all
>   if (DokanOperations->GetFileInformation && DeleteFileFlag) {
>     BY_HANDLE_FILE_INFORMATION byHandleFileInfo;
>     ZeroMemory(&byHandleFileInfo, sizeof(BY_HANDLE_FILE_INFORMATION));
>     result = DokanOperations->GetFileInformation(
>         EventContext->Operation.SetFile.FileName, &byHandleFileInfo, FileInfo);
> 
>     if (result == STATUS_SUCCESS &&
>         (byHandleFileInfo.dwFileAttributes & FILE_ATTRIBUTE_READONLY) != 0)
>       return STATUS_CANNOT_DELETE;
>   }
>

  FileInfo->DeleteOnClose = DeleteFileFlag;

  if (FileInfo->IsDirectory) {
    result = DokanOperations->DeleteDirectory(
        EventContext->Operation.SetFile.FileName, FileInfo);
  } else {
  
    result = DokanOperations->DeleteFile(EventContext->Operation.SetFile.FileName,
                                       FileInfo);
  }

>   //Double set for later be sure FS user did not changed it
>   **FileInfo->DeleteOnClose = DeleteFileFlag;  // What if above DokanOperations->DeleteFile() returns Status_access_denied?**

 
  return result;
}

@Liryna
Copy link
Member

Liryna commented Mar 28, 2022

Thanks for the video but I am unable to reproduce on my side. I get the same behavior on NTFS and Dokan: The file is deleted without warning even if marked read only.

Explorer removes the read only attribute after doing a getfileinformation, call SetDispositionInformationFile delete and then restore the read only attribute before the file is removed.

there is no handling of DokanDispatchSetInformation() FileInformationClass==FileDispositionInformation of dokan driver

dokany/sys/fileinfo.c

Lines 843 to 844 in a1a557b

case FileDispositionInformation:
case FileDispositionInformationEx: {

dokany/sys/fileinfo.c

Lines 898 to 910 in a1a557b

case FileDispositionInformation:
case FileDispositionInformationEx:
if (RequestContext->IrpSp->FileObject->DeletePending) {
if (DokanFCBFlagsIsSet(fcb, DOKAN_FILE_DIRECTORY)) {
DokanNotifyReportChange(RequestContext, fcb,
FILE_NOTIFY_CHANGE_DIR_NAME,
FILE_ACTION_REMOVED);
} else {
DokanNotifyReportChange(RequestContext, fcb,
FILE_NOTIFY_CHANGE_FILE_NAME,
FILE_ACTION_REMOVED);
}
}

//below 3-line of code are very strange.

It just avoid an unnecessary call when the DeleteOnClose is already set to the wanted new state.

// These are no effect at all

Why ? We check if the file is read only before trying to ask for deletion.

// What if above DokanOperations->DeleteFile() returns Status_access_denied?**

Then result will have the STATUS_ACCESS_DENIED and DeleteOnClose value will be ignored in kernel.

Maybe this is an issue linked to your environment where the source file actually cannot be removed by the mirror sample ?

@onexzero
Copy link
Author

onexzero commented Mar 29, 2022

Explorer removes the read only attribute after doing a getfileinformation, call SetDispositionInformationFile delete and then restore the read only attribute before the file is removed.

<-- This is not true. see attached procmon log.

case FileDispositionInformation:
case FileDispositionInformationEx:

<-- Invalid_parameter is returned from DokanDispatchSetInformation, is this part called? I'm just asking because I don't know.
DokanSetDispositionInformation() is not called actually during file deletion.

Then result will have the STATUS_ACCESS_DENIED and DeleteOnClose value will be ignored in kernel.

<-- I also expected that . but on MirrorCleanup FileInfo->DeleteOnClose flag is still active even if i clear that flag on MirrorCreateFile.

Delete.zip

@Liryna
Copy link
Member

Liryna commented Mar 29, 2022

Invalid_parameter is returned from DokanDispatchSetInformation, is this part called? I'm just asking because I don't know.
DokanSetDispositionInformation() is not called actually during file deletion.

Looking at your procmon it will not be called by the system since the delete happens with DeleteOnClose.

I also expected that . but on MirrorCleanup FileInfo->DeleteOnClose flag is still active even if i clear that flag on MirrorCreateFile.

That's not how it works. The system asks you to open a file with DeleteOnClose and you have to agree with a status_success or to refuse with access denied. From your procmon you return success so you need to issue a delete on the file during cleanup.

I would suggest you read the Microsoft documentation regarding deletion.

I must say that is very difficult to help you as you are providing information chunk by chunk without even logs.
I thought you were using the official mirror sample but looks like you are using a version of it with your change. It is just impossible for me to figure it out what is happening and provide proper guidance after.

@onexzero
Copy link
Author

onexzero commented Mar 29, 2022

I tested it again on another VM. I don't know yet if there are any differences between the VMs, but I've confirmed that it works just like you said.

5:59:55.4366844 PM Explorer.EXE 4516 3428 CreateFile 0.0019685 R:\ccccc.txt CANNOT DELETE Desired Access: Delete, Disposition: Open, Options: Non-Directory File, Delete On Close, Attributes: n/a, ShareMode: Delete, AllocationSize: n/a
5:59:55.4403600 PM Explorer.EXE 4516 3428 CreateFile 0.0025361 R:\ccccc.txt SUCCESS Desired Access: Read Attributes, Delete, Disposition: Open, Options: Non-Directory File, Open Reparse Point, Attributes: n/a, ShareMode: Read, Write, Delete, AllocationSize: n/a, OpenResult: Opened
5:59:55.4429080 PM Explorer.EXE 4516 3428 QueryAttributeTagFile 0.0002237 R:\ccccc.txt SUCCESS Attributes: RA, ReparseTag: 0x0
5:59:55.4431592 PM Explorer.EXE 4516 3428 SetDispositionInformationEx 0.0001643 R:\ccccc.txt CANNOT DELETE Flags: FILE_DISPOSITION_DELETE, FILE_DISPOSITION_POSIX_SEMANTICS, FILE_DISPOSITION_FORCE_IMAGE_SECTION_CHECK
5:59:55.4433469 PM Explorer.EXE 4516 3428 CloseFile 0.0001310 R:\ccccc.txt SUCCESS
5:59:55.4461309 PM Explorer.EXE 4516 3428 CreateFile 0.0007537 R:\ccccc.txt SUCCESS Desired Access: Read Attributes, Disposition: Open, Options: Open Reparse Point, Attributes: n/a, ShareMode: Read, Write, Delete, AllocationSize: n/a, OpenResult: Opened
5:59:55.4469025 PM Explorer.EXE 4516 3428 QueryBasicInformationFile 0.0002231 R:\ccccc.txt SUCCESS CreationTime: 3/29/2022 5:57:26 PM, LastAccessTime: 3/29/2022 5:59:21 PM, LastWriteTime: 3/25/2022 4:08:57 PM, ChangeTime: 3/25/2022 4:08:57 PM, FileAttributes: RA
5:59:55.4471543 PM Explorer.EXE 4516 3428 CloseFile 0.0001275 R:\ccccc.txt SUCCESS
5:59:55.4486140 PM Explorer.EXE 4516 3428 CreateFile 0.0036190 R:\ccccc.txt SUCCESS Desired Access: Write Attributes, Synchronize, Disposition: Open, Options: Synchronous IO Non-Alert, Open Reparse Point, Attributes: n/a, ShareMode: Read, Write, Delete, AllocationSize: n/a, OpenResult: Opened
5:59:55.4498749 PM Explorer.EXE 4516 5292 CreateFile 0.0017903 R:\ SUCCESS Desired Access: Synchronize, Disposition: Open, Options: Directory, Synchronous IO Non-Alert, Attributes: n/a, ShareMode: Read, Write, AllocationSize: n/a, OpenResult: Opened
5:59:55.4516801 PM Explorer.EXE 4516 5292 QueryNameInformationFile 0.0000039 R:\ SUCCESS Name:
5:59:55.4516952 PM Explorer.EXE 4516 5292 QueryInformationVolume 0.0004389 R:\ SUCCESS VolumeCreationTime: 1/1/1601 9:00:00 AM, VolumeSerialNumber: 1983-1116, SupportsObjects: False, VolumeLabel: DOKAN
5:59:55.4521627 PM Explorer.EXE 4516 5292 QueryAttributeInformationVolume 0.0003787 R:\ SUCCESS FileSystemAttributes: Unicode, ACLs, Named Streams, MaximumComponentNameLength: 255, FileSystemName: NTFS
5:59:55.4522512 PM Explorer.EXE 4516 3428 SetBasicInformationFile 0.0007231 R:\ccccc.txt SUCCESS CreationTime: 1/1/1601 9:00:00 AM, LastAccessTime: 1/1/1601 9:00:00 AM, LastWriteTime: 1/1/1601 9:00:00 AM, ChangeTime: 1/1/1601 9:00:00 AM, FileAttributes: AN
5:59:55.4525667 PM Explorer.EXE 4516 5292 CloseFile 0.0003438 R:\ SUCCESS
5:59:55.4529947 PM Explorer.EXE 4516 3428 CloseFile 0.0001625 R:\ccccc.txt SUCCESS
5:59:55.4547594 PM Explorer.EXE 4516 3428 CreateFile 0.0018910 R:\ccccc.txt SUCCESS Desired Access: Delete, Disposition: Open, Options: Non-Directory File, Delete On Close, Attributes: n/a, ShareMode: Delete, AllocationSize: n/a, OpenResult: Opened

I'll have to check this out a bit more when I have time. Something is still not clear.

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

No branches or pull requests

2 participants