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

SetFileInfo triggers Notify when it doesn't change file attribute #1175

Open
Lucasq11 opened this issue Sep 26, 2023 · 3 comments
Open

SetFileInfo triggers Notify when it doesn't change file attribute #1175

Lucasq11 opened this issue Sep 26, 2023 · 3 comments

Comments

@Lucasq11
Copy link

Environment

  • Windows version: Windows 10, 2004, 19041.264
  • Processor architecture: x64
  • Dokany version: 1.4.0.1
  • Library type (Dokany/FUSE): dokan, dokanfuse

Check List

  • [Y] I checked my issue doesn't exist yet
  • [Y] I can always reproduce the issue with the provided description below.
  • [N] I have updated Dokany to the latest version and have reboot my computer after.

Description

I found that the setfileattribute function could trigger Notify to report a change even the file attribute is not changed.
Here is fuction DokanSetBasicInformation in dokan/setfile.c:

NTSTATUS
DokanSetBasicInformation(PEVENT_CONTEXT EventContext, PDOKAN_FILE_INFO FileInfo,
                         PDOKAN_OPERATIONS DokanOperations) {
  FILETIME creation, lastAccess, lastWrite;
  NTSTATUS status;

  ...

  status = DokanOperations->SetFileAttributes(
      EventContext->Operation.SetFile.FileName, basicInfo->FileAttributes,
      FileInfo);

  if (status != STATUS_SUCCESS)
    return status;

  ...

  return DokanOperations->SetFileTime(EventContext->Operation.SetFile.FileName,
                                      &creation, &lastAccess, &lastWrite,
                                      FileInfo);
}

And the implementation of SetFileTime in dokanfuse/fusemain.cpp:

int impl_fuse_context::set_file_time(PCWSTR file_name,
                                     const FILETIME *creation_time,
                                     const FILETIME *last_access_time,
                                     const FILETIME *last_write_time,
                                     PDOKAN_FILE_INFO dokan_file_info) {
  if (!ops_.utimens && !ops_.utime && !ops_.win_set_times)
    return -EINVAL;

  // I didn't implement this function in my fuse application
  if (ops_.win_set_times) {
  }

  if (!ops_.getattr)
    return -EINVAL;

  std::string fname = unixify(wchar_to_utf8_cstr(file_name));
  CHECKED(check_and_resolve(&fname));

  struct FUSE_STAT st = {0};
  CHECKED(ops_.getattr(fname.c_str(), &st));

  if (ops_.utimens) {
    struct timespec tv[2] = {0};
    // TODO: support nanosecond resolution
    // Access time
       CHECKED(helper_set_time_struct(last_access_time, st.st_atime,
                                      &(tv[0].tv_sec)));
    // Modification time
       CHECKED(helper_set_time_struct(last_write_time, st.st_mtime,
                                      &(tv[1].tv_sec)));

    return ops_.utimens(fname.c_str(), tv);
  } else {
  ...
  }
}

In a process, set_file_time is called and param creation_time, last_access_time, and last_write_time are all zero, in this situation atime and mtime should be set to the original values of this file from getattr. My fuse function utimens set this times from getattr and returns true to SetFileTime and DokanSetBasicInformation, in kernal mode, case FileBasicInformation is met in DokanCompleteSetInformation and DokanNotifyReportChange is called:

VOID DokanCompleteSetInformation(__in PIRP_ENTRY IrpEntry,
                                 __in PEVENT_INFORMATION EventInfo) {
  PIRP irp;
  PIO_STACK_LOCATION irpSp;
  NTSTATUS status;

  ...

  status = EventInfo->Status;

  __try {

  ...

  if (NT_SUCCESS(status)) {
      switch (irpSp->Parameters.SetFile.FileInformationClass) {
      case FileAllocationInformation:
        DokanNotifyReportChange(fcb, FILE_NOTIFY_CHANGE_SIZE,
                                FILE_ACTION_MODIFIED);
        break;
      case FileBasicInformation:
        DokanNotifyReportChange(
            fcb,
            FILE_NOTIFY_CHANGE_ATTRIBUTES | FILE_NOTIFY_CHANGE_LAST_WRITE |
                FILE_NOTIFY_CHANGE_LAST_ACCESS | FILE_NOTIFY_CHANGE_CREATION,
            FILE_ACTION_MODIFIED);
        break;

Here is the problem I'm facing:
An application keeps calling set_file_time to a directory in my fuse application and setting excatly the same times for the directory, and notify is trigger every time and the caller application is stuck for good.
I think there should be some conditions that will judge whether the file times or attributes are changed before calling DokanNotifyReportChange to notify the server. Or is there any other method I can follow to get rid of this endless notification?

@Liryna
Copy link
Member

Liryna commented Sep 26, 2023

Hi @Lucasq11 ,

Thanks for the report and the analysis. Agree we should not notify if nothing has changed.

If we had the item cache in the Kernel, we could know whether a value changed or not.
The other option would be to have userland return a specific STATUS that informs that nothing changed and therefore no notification should be sent. The status should still be a success since the change was applied but just a NO_OP.
https://learn.microsoft.com/en-us/openspecs/windows_protocols/ms-erref/596a1078-e883-4972-9bbc-49e60bebca55
On top of my head, I don't know if there is an existing no op status.

@Lucasq11
Copy link
Author

Hi @Lucasq11 ,

Thanks for the report and the analysis. Agree we should not notify if nothing has changed.

If we had the item cache in the Kernel, we could know whether a value changed or not. The other option would be to have userland return a specific STATUS that informs that nothing changed and therefore no notification should be sent. The status should still be a success since the change was applied but just a NO_OP. https://learn.microsoft.com/en-us/openspecs/windows_protocols/ms-erref/596a1078-e883-4972-9bbc-49e60bebca55 On top of my head, I don't know if there is an existing no op status.

I'm trying to modify the driver to avoid this issue, there's problem about signing. How can I acquire an avaliable cert to sign dokan1.sys when testsigning is off?

@LTRData
Copy link
Contributor

LTRData commented Sep 27, 2023

Hi @Lucasq11 ,
Thanks for the report and the analysis. Agree we should not notify if nothing has changed.
If we had the item cache in the Kernel, we could know whether a value changed or not. The other option would be to have userland return a specific STATUS that informs that nothing changed and therefore no notification should be sent. The status should still be a success since the change was applied but just a NO_OP. https://learn.microsoft.com/en-us/openspecs/windows_protocols/ms-erref/596a1078-e883-4972-9bbc-49e60bebca55 On top of my head, I don't know if there is an existing no op status.

I'm trying to modify the driver to avoid this issue, there's problem about signing. How can I acquire an avaliable cert to sign dokan1.sys when testsigning is off?

You can order an EV signing certificate for your company. I have used GlobalSign for that for example. You get a USB token that you download the certificate to, use that to sign a cab file with a .inf and the driver, upload to Microsoft's attestant signing portal and after a few hours you get a zip file back with the signed driver.

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

No branches or pull requests

3 participants