search

dokan-dev / dokany

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

Compatibility with Rising antivirus software #1211

Open fcyinxunpeng opened 2 months ago

fcyinxunpeng commented 2 months ago

Environment

Check List

Description

When I run memfs.exe with administrator privileges on a PC with certain antivirus software installed, I get a BSOD，seemingly crashing in dokan2.sys

If I disable the antivirus software or stop its USB protection feature, memfs.exe can work normally. Also, if I enable the antivirus software and its USB protection feature but run memfs.exe without administrator privileges, memfs.exe can work normally. I've tested version 2.1.0.1000 on both Windows 7 and Windows 10. The name of the antivirus software is Rising, and it's from China. Here is the download page: rising v17 download page

I am a novice and not very familiar with Windows driver development. I have conducted some basic debugging and it seems that the buffer obtained through the MmGetSystemAddressForMdlNormalSafe() function points to an incorrect address. When executing RtlZeroMemory() on this buffer, an error occurred.

I looked into the code, and I have a question: In the DokanQueryDirectory() function (sys/directory.c:106), could the value of RequestContext->Irp->MdlAddress be filled by the program of antivirus software?

My English is bad. This is the translation provided by ChatGPT. Thank you very much.

Logs

Here is my log and the output of !analyze -v:

dokan log:

log.txt

!analyze -v output:

analyze.txt

minidump file:

minidump.dmp

Liryna commented 2 months ago

Hi @fcyinxunpeng ,

RequestContext->Irp->MdlAddress be filled by the program of antivirus software?

It technically could but I believe it shouldn't provide one as the current IRP major is IRP_MJ_DIRECTORY_CONTROL https://learn.microsoft.com/en-us/windows-hardware/drivers/ddi/wdm/ns-wdm-_irp

Pointer to an MDL describing a user buffer, if the driver is using direct I/O, and the IRP major function code is one of the following:

    IRP_MJ_READ

    The MDL describes an empty buffer that the device or driver fills in.

    IRP_MJ_WRITE

    The MDL describes a buffer that contains data for the device or driver.

    IRP_MJ_DEVICE_CONTROL or IRP_MJ_INTERNAL_DEVICE_CONTROL

    If the IOCTL code specifies the METHOD_IN_DIRECT transfer type, the MDL describes a buffer that contains data for the device or driver.

Normally we should allocate that MDL from the UserBuffer and own it but due to how the code is written, we try to use their. https://github.com/dokan-dev/dokany/blob/69e3d88c693da08fc1e9c540c162877880eac155/sys/directory.c#L105-L114

We could change our code to override their value by removing this check and the same one in DokanAllocateMdl to ignore their (bogus?) value. https://github.com/dokan-dev/dokany/blob/69e3d88c693da08fc1e9c540c162877880eac155/sys/directory.c#L106

fcyinxunpeng commented 2 months ago

Hi @Liryna , Thanks for your reply. I removed this check and the same one in DokanAllocateMdl() https://github.com/dokan-dev/dokany/blob/69e3d88c693da08fc1e9c540c162877880eac155/sys/directory.c#L106 https://github.com/dokan-dev/dokany/blob/69e3d88c693da08fc1e9c540c162877880eac155/sys/dokan.c#L589 But it seems like it's not working because RequestContext->Irp->UserBuffer is always NULL, while RequestContext->Irp->MdlAddress is not NULL even when I don't have antivirus software installed.

If I zero memory in DokanQueryDirecory():

 if (RequestContext->Irp->MdlAddress == NULL) {
    ...
 }else{
    // test code
    buffer = MmGetSystemAddressForMdlNormalSafe(RequestContext->Irp->MdlAddress);
    RtlZeroMemory(buffer, bufferLen);  // won't crash at this line
}

Dokany2.sys doesn't crash at this line of code, so RequestContext->Irp->MdlAddress is valid at this moment. Maybe then the antivirus software free the content of RequestContext->Irp->MdlAddress , even though dokany2.sys returns a _STATUSPENDING?

Liryna commented 2 months ago

Thanks for testing! Yeah that could be possible. Would you be able to contact Rising so they can look into this on their side ?

fcyinxunpeng commented 2 months ago

OK, I intend to ask for official help. And I found that the MmGetSystemAddressForMdlNormalSafe function returns a user-space address instead of a kernel-space address when I use antivirus software scanning.

fcyinxunpeng commented 2 months ago

This is the official reply of rising:

The problem is that when traversing the directory, Rising Anti-Virus Software V17 passes in a user-mode address, causing a blue screen when the dokany driver writes to this address. You can suggest to dokany that you add a method to determine whether the request mode is user mode, or after detecting the user mode address, use the writing method of the user mode address, such as try except and so on.

But I think the real reason is that the anti-virus software modified the address of MmGetSystemAddressForMdlNormalSafe() to point to user space, but this address is only valid in the current context. When dokany is executing DokanCompleteDirectoryControl(), this address is invalid.

Liryna commented 2 months ago

Thanks @fcyinxunpeng ! I am not sure about their answer. I believe we do everything correctly. We try catch and lock the page when we receive the request https://github.com/dokan-dev/dokany/blob/69e3d88c693da08fc1e9c540c162877880eac155/sys/dokan.c#L590-L600 and only release when we no longer need it. https://github.com/dokan-dev/dokany/blob/master/sys/directory.c#L323

I agree with you that they might not wait for our completion and do something with the buffer they gave us and things go wrong.

Liryna commented 1 month ago

@fcyinxunpeng Were you able to get more info from Rising ?

fcyinxunpeng commented 1 month ago

@Liryna I decompiled one of Rising's driver files named rsutil.sys. In the IoCompletion of IRP_MJ_DIRECTORY_CONTROL, the code releases the MDL without checking the value of IRP->PendingReturned. I have emailed this conclusion to the Rising staff. She replied that it has been forwarded to the developers to investigate the issue, but three weeks have passed, and there has been no news. Rising may not want to modify their code regarding this issue.