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

Compatibility with Rising antivirus software #1211

Open
4 of 5 tasks
fcyinxunpeng opened this issue Apr 22, 2024 · 8 comments
Open
4 of 5 tasks

Compatibility with Rising antivirus software #1211

fcyinxunpeng opened this issue Apr 22, 2024 · 8 comments

Comments

@fcyinxunpeng
Copy link

fcyinxunpeng commented Apr 22, 2024

Environment

  • Windows version: win7, win10
  • Processor architecture: x64
  • Dokany version: 2.1.0.1000
  • 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

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
Copy link
Member

Liryna commented Apr 22, 2024

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.

dokany/sys/directory.c

Lines 105 to 114 in 69e3d88

// make a MDL for UserBuffer that can be used later on another thread context
if (RequestContext->Irp->MdlAddress == NULL) {
status = DokanAllocateMdl(
RequestContext,
RequestContext->IrpSp->Parameters.QueryDirectory.Length);
if (!NT_SUCCESS(status)) {
return status;
}
RequestContext->Flags = DOKAN_MDL_ALLOCATED;
}

We could change our code to override their value by removing this check and the same one in DokanAllocateMdl to ignore their (bogus?) value.

if (RequestContext->Irp->MdlAddress == NULL) {

@fcyinxunpeng
Copy link
Author

Hi @Liryna ,
Thanks for your reply.
I removed this check and the same one in DokanAllocateMdl()

if (RequestContext->Irp->MdlAddress == NULL) {

if (RequestContext->Irp->MdlAddress == NULL) {

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 STATUS_PENDING?

@Liryna
Copy link
Member

Liryna commented Apr 24, 2024

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
Copy link
Author

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
Copy link
Author

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
Copy link
Member

Liryna commented Apr 27, 2024

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

dokany/sys/dokan.c

Lines 590 to 600 in 69e3d88

RequestContext->Irp->MdlAddress =
IoAllocateMdl(RequestContext->Irp->UserBuffer, Length, FALSE, FALSE,
RequestContext->Irp);
if (RequestContext->Irp->MdlAddress == NULL) {
DOKAN_LOG_FINE_IRP(RequestContext, "IoAllocateMdl returned NULL");
return STATUS_INSUFFICIENT_RESOURCES;
}
__try {
MmProbeAndLockPages(RequestContext->Irp->MdlAddress,
RequestContext->Irp->RequestorMode, IoWriteAccess);

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 Liryna changed the title Compatibility with antivirus software Compatibility with Rising antivirus software May 6, 2024
@Liryna
Copy link
Member

Liryna commented Jun 12, 2024

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

@fcyinxunpeng
Copy link
Author

@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.

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