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

Do not forcefully free all VCBs upon IRP_MJ_SHUTDOWN request #103

Open
wants to merge 1 commit into
base: master
from

Conversation

Projects
None yet
2 participants
@Extravert-ir
Contributor

Extravert-ir commented Jul 31, 2018

I'm not sure if I've done this the right way, so it is probably more like an issue than a PR.
There is the thing:
According to MSDN, upon receiving IRP_MJ_SHUTDOWN request, a driver should flush its caches and complete any ongoing operations, and not clean up its internal structures (if I understand correctly).
But drv_shutdown function, which is a handler of IRP_MJ_SHUTDOWN literally dismounts the volume by calling uninit() function in it, disallowing any further interaction with Vcb.
Looks like it's ok on "secondary" disks, but when using WinBtrfs as a boot driver, this causes issues.
For example, during shutdown ReactOS sends IRP_MJ_SHUTDOWN to all FSDs, but there are still running processes left (with open handles ofc). WinBtrfs frees all its memory, and when that processes are going to close their handles, we are getting use-after-free and BSOD.

If I'm right, Microsoft's fastfat driver dismounts the volume only if it can do it: https://github.com/Microsoft/Windows-driver-samples/blob/a49f6121e1976979af7d6d8fc1f9af7acec9bcb2/filesys/fastfat/shutdown.c#L278

I tried to do the same in drv_shutdown

Do not forcefully free all VCBs upon IRP_MJ_SHUTDOWN request. Set
Vcb->removing instead, and it will be freed only when no open handles
left.
@Extravert-ir

This comment has been minimized.

Contributor

Extravert-ir commented Jul 31, 2018

Oh, maybe just call dismount_volume for each Vcb?
It will properly treat volumes with "disallow_dismount" variable set, and for all other ones it will set "removing" flag.

@HBelusca HBelusca referenced this pull request Aug 14, 2018

Open

Boot Windows? #16

@ale5000-git

This comment has been minimized.

ale5000-git commented Nov 10, 2018

@maharmstone
Hi,
any update on this?

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