Race condition with io cancel

[ylgybbz]

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: Windows
• Processor architecture: X86-64
• Dokany version: 2.0.3.2000
• Library type (Dokany/FUSE): Dokany

Check List

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

Description

i think there are some race condition may cause BSOD

1. when IoSetCancelRoutine in DokanEventWrite return null, it Initialize irpEntry->ListEntry ; when DokanIrpCancelRoutine, it will call RemoveEntryList(&irpEntry->ListEntry), system may crash. Maybe it should call RemoveEntryList(&irpEntry->ListEntry) in DokanEventWrite?
2. when process completeList in DokanCompleteIrp, the irpEntry may free/initialized by cancel routine. I think the irp should set cancel-routine NULL before insert into completeList?

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]

Hi @ylgybbz ,

Thanks for the report and the analysis.

1. Let me take a look
2. I actually had a pending commit that I forgot to push that should fix this https://github.com/dokan-dev/dokany/commit/18bae97350434ed2319abe9a4b41b10799cf4b5c Let me know if you are ok with the change or not.

[Liryna]

1. This is happening while we hold KeAcquireSpinLock. It should not be possible that the IRP be canceled, no ?

[ylgybbz]

1. This is happening while we hold KeAcquireSpinLock. It should not be possible that the IRP be canceled, no ?

suppose that : 1、cancel enters，DokanCompleteIrp is waiting for pending-list lock, 2、DokanEventWrite hold lock，but meets IoSetCancelRoutine return null，then initial the list entry , this makes the list broken，other functions use the list may got wrong/no list entry

Although holding KeAcquireSpinLock in DokanEventWrite, but it break the list

[ylgybbz]

2. I actually had a pending commit that I forgot to push that should fix this 18bae97 Let me know if you are ok with the change or not.

yes , This is how I modified it，but i think offset += GetEventInfoSize(irpEntry->RequestContext.IrpSp->MajorFunction, eventInfo); should be done before continue next loop

[Liryna]

@ylgybbz Thanks a lot for pointing out this and describing the Write case!

Those two commits should take care of that https://github.com/dokan-dev/dokany/commit/18bae97350434ed2319abe9a4b41b10799cf4b5c https://github.com/dokan-dev/dokany/commit/5a372e7a076daea45910f5e1ca36429ee5ff6bbc

Let me know otherwise 💪