Open LTRData opened 2 years ago
Liryna
commented
2 years ago Thanks @LTRData for the report! The device must have been partially unmounting while the library pulled events. The fix is clearly not enough but might fix it. As said in the commit, the Todo in that function should be implemented and the VCB usage to be removed.
LTRData
commented
6 months ago Got another report now with exact same BSOD (but now with latest driver version). I think I realize now how this could still happen. The fix that calls IsUnmountPendingVcb(RequestContext->Vcb) before the crashing code does not help in this case. In IsUnmountPendingVcb function, it checks whether Vcb is NULL and in that case, it returns FALSE. This means that it will continue beyond this if statement when RequestContext->Vcb is NULL, and then crash at RequestContext->Vcb->HasEventWait = TRUE; because RequestContext->Vcb is NULL.
The question here is what exactly is wrong. Either IsUnmountPendingVcb function needs to be changed to return TRUE when Vcb is NULL, or that if statement needs to be changed to something like if (RequestContext->Vcb == NULL || IsUnmountPendingVcb(RequestContext->Vcb)). Could there be other unexpected side effects with changing the IsUnmountPendingVcb function?
Liryna
commented
6 months ago I would agree to do if (RequestContext->Vcb == NULL || IsUnmountPendingVcb(RequestContext->Vcb)) but I would like to understand why we have a Dcb without Vcb here. We don't have more context when this happens ?
LTRData
commented
6 months ago I would agree to do
if (RequestContext->Vcb == NULL || IsUnmountPendingVcb(RequestContext->Vcb))but I would like to understand why we have aDcbwithoutVcbhere. We don't have more context when this happens ?
I do not know much about the actual origin of the request that ends up like this. In both cases, I only got minidumps with very limited user process information available. But I am certain that it happens very early in the lifetime of the file system device. Right after mount, I would say.
Liryna
commented
6 months ago That can somewhat make sense. The Vcb is only set to the Dcb after it was mounted. We start to pull when DokanEventStart returns from the kernel. During that call IoVerifyVolume should be forcing the Dcb to get its Vcb through IRP_MN_MOUNT_VOLUME but maybe here it is not happening and we are still wrongly? returning a success to userland which then starts to pull the events on a null Vcb.
What I expect is that https://github.com/dokan-dev/dokany/commit/b74b6484b3e6f872d7a7eeac2b9ecf683b653b71 will stop the crash but will make the mount fail for the user due to pull event failing. Then we will be able to better understand what is going on from the logs.
Does that sound correct ?
LTRData
commented
6 months ago That sounds good! 👍
Liryna
commented
6 months ago Dokan v1 which had event wait to pull the event, also had that check with a comment that was lost https://github.com/dokan-dev/dokany/blob/v1.5.1.1000/sys/event.c#L347-L351
Feature request can skip this form. Bug report must complete it.
Check Listmust be 100% match or it will be automatically closed without further discussion. Please remove this line.Environment
Description
Got a dump file from a BSOD from a machine from a person who currently tests an application that I am working on that uses Dokany. She has not been able to replicate the same crash but since the crash dump seems interesting in that I think it might show a possible logic flaw of some kind, I share it here anyway.
The crash happened in
DokanProcessAndPullEventswhere it tries to deference a NULLRequestContext->Vcb. I see there is code to workaround similar cases inDokanDiskUserFsRequestwith the linerequestContext.Vcb = requestContext.Dcb->Vcb;, but the problem in this case is thatrequestContext.Dcb->Vcbis also NULL. The FSCTL code isFSCTL_EVENT_PROCESS_N_PULLandIrpSp->Parameters.FileSystemhas OutputBufferLength = 0x20000 and InputBufferLength = 0.