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

Use IoRegisterFileSystem #47

Closed
Maxhy opened this issue Aug 28, 2015 · 25 comments
Closed

Use IoRegisterFileSystem #47

Maxhy opened this issue Aug 28, 2015 · 25 comments
Labels

Comments

@Maxhy
Copy link
Member

Maxhy commented Aug 28, 2015

File system driver should be registered using IoRegisterFileSystem.
This was disabled in 2010 on svn r139 by Hiroki Asakawa, see https://code.google.com/p/dokan/source/diff?spec=svn139&r=139&format=side&path=/trunk/sys/init.c

This is bad practice ; mini filter drivers, process monitor, ... cannot be used over Dokan because of that which today result to breaking several applications.
With current code base the BSOD remains if using IoRegisterFileSystem. According to my investigation it has nothing to do with Asakawa statement FS_FILTER_CALLBACKS.PreAcquireForSectionSynchronization is not correctly called in that case but seems to be related to http://www.osronline.com/showThread.cfm?link=74100

@Maxhy
Copy link
Member Author

Maxhy commented Aug 28, 2015

After setting up a Checked Win8.1 OS, I end up to the same assertion than describe on OSROnline thread.
Assertion: (((PVOLUME_DEVICE_EXTENSION)((DeviceObject)->DeviceExtension))->Type.Signature) == 0xF107

We should focus on the workaround provided by Max.

Maxhy pushed a commit that referenced this issue Aug 30, 2015
Return fake drive layout/partition information
Maxhy pushed a commit that referenced this issue Aug 30, 2015
Return fake drive layout/partition information
@Maxhy
Copy link
Member Author

Maxhy commented Aug 30, 2015

I fixed it on temporary mountmanager branch.
Basically the problem is simple: the same device object cannot be File System Device and Volume Device anymore since Windows Vista thanks to an undocumented Filter Manager (mini-filters) behavior.
Dokan was working this way because the File System Device itself is related to a Disk Device which is dedicated per user-mode file system instance ; there was no need to distinguish the device objects.
The solution was to create a new Volume Device Object. VCB is of course allocated to VDO and I created a specific FSCB (_DokanFileSystemControlBlock) on FSDO which is a simple Device Extension just to keep track of DCB for IOCTL handling.

But this didn't resolve mini filter driver attachment. I believe it will not work as long we don't really support mount manager. I will create a specific issue for mount manager as it could require discussion.

@marinkobabic
Copy link
Contributor

@Maxhy
You are right that IoRegisterFileSystem should be executed. But I don't agree with the statement above, that the File System Device and Volume Device can't be the same device. I can't find this statement in microsoft example, documentation nor in the books. Just a post in a forum is not enough to build it that way.

Just as info, dokan has some duplicate code in the start (DokanCreateDiskDevice) and IRP_MN_MOUNT_VOLUME.

@Maxhy
Copy link
Member Author

Maxhy commented Aug 30, 2015

I agree this is not explicitly said somewhere by Microsoft. After hours of testing that's my conclusion but maybe I missed something :). I don't know any project outside Dokan that was working this way, all projects/samples that I'm aware of use a different device for file system and volume.
I isolated the issue to filter manager as I explained and I believe this is coming from an undocumented assertion the filter manager is doing about volume device object. Using a separate volume device fixed it.

But if I take a look again on the documentation https://msdn.microsoft.com/en-us/library/windows/hardware/ff543147(v=vs.85).aspx, Vpb field:
A pointer to the volume parameter block (VPB) that is associated with the device object. For file system drivers, the VPB can provide a connection to any unnamed logical device object that represents an instance of a mounted volume. This is an opaque member.
Key word here is unnamed. By using the same device object you break one rule: File System device object must be named and Volume device object shouldn't be. Not explicitly said but looking for an official answer... yeah not perfect, better if you found something else or even correct me if I'm wrong ^^.

@marinkobabic
Copy link
Contributor

The documentation is correct. So when the IRP_MN_MOUNT_VOLUME occures the command of creating device should look the following way.

    Status = IoCreateDevice(
                 DriverObject,
                 sizeof(MY_VCB),
                 NULL,
                 FILE_DEVICE_DISK_FILE_SYSTEM,
                 0,
                 FALSE,
                 &VolumeDeviceObject );

or in our case the secure method should be used. So the fix may be just to leave the name NULL like in my example.

The system checks if the Vpb has a device associated. If not or the Device is not mounted, it executes the IRP_MN_MOUNT_VOLUME on the dokan driver. Dokan creates then the device without name and makes the association to Vpb.

@Maxhy
Copy link
Member Author

Maxhy commented Aug 31, 2015

That's it. But some additional stuffs are required to handle correctly the independent file system object which will also receive IOCTL on his own.
I did it on mountmanager branch as you can see on https://github.com/dokan-dev/dokany/tree/mountmanager. Volume device is currently created on DokanCreateDiskDevice because his lifetime is the same but could have been init on IRP_MN_MOUNT_VOLUME as you suggested.

@marinkobabic
Copy link
Contributor

@Maxhy
There is no reason for an additional Volume Device like described in the workaround. All I wanted you to do is here https://github.com/dokan-dev/dokany/blob/develop/sys/init.c#L532 set to NULL. The job is done. FILE_DEVICE_DISK_FILE_SYSTEM should always be unnamed and this is the reason the FltMgr fails. Please can you try this fix before going on.

@Maxhy
Copy link
Member Author

Maxhy commented Aug 31, 2015

@marinkobabic I got your point but for me:

I will do the test later but I think file system should be named for IoRegisterFileSystem registration (not sure) and anyway it would be an issue if it is not named for network file system (see

status = FsRtlRegisterUncProvider(&(Dcb->MupHandle), Dcb->FileSystemDeviceName, FALSE);
). Isn't it?

@marinkobabic
Copy link
Contributor

@Maxhy

  • Your first point is the dokan FILE_DEVICE_DISK which should have a name. As you can see in the example the method is in DriverEntry. They are not using FILE_DEVICE_DISK, but this is the disk the IO Manager is talking to and then it realizes that VPB is NULL and it requests the CdMountVolume.
  • Then your second point comes in play, which is the FILE_DEVICE_DISK_FILE_SYSTEM and should be unnamed. As you can see in the example, we are now in the method CdMountVolume.

@Maxhy
Copy link
Member Author

Maxhy commented Aug 31, 2015

Oh... I didn't saw it that way...
So what about FsRtlRegisterUncProvider in that case, should we create a specific network redirector device then?

@kenjiuno
Copy link

Hi.
Sorry for disturbing from outside.

So what about FsRtlRegisterUncProvider in that case, should we create a specific network redirector device then?

FsRtlRegisterUncProvider makes a chance to catch UNC path access:

  • network redirector will receive IOCTL_REDIR_QUERY_PATH control code by registering to FsRtlRegisterUncProvider(Ex). We can accept or deny the subsequent access.
  • And then we will receive IRP_MJ_CREATE request with UNC path like \server\share\file.

Currently dokan seems not to implement UNC support things.

case IOCTL_REDIR_QUERY_PATH:

@marinkobabic
Copy link
Contributor

I can just confirm what kenjiuno described above. Dokan has some methods already (

DokanRegisterUncProvider(
) , but the implementation seems not to be complete.

For the network drive the FILE_DEVICE_NETWORK_FILE_SYSTEM should be used, in my opinion.

@Maxhy
Copy link
Member Author

Maxhy commented Aug 31, 2015

That's right. Maybe this should be removed from stable code and a specific branch created for this development purpose if requested.
Otherwise, using IoCreateDeviceSecure for unnamed driver object is not possible, IoCreateDevice must be used.

@Maxhy
Copy link
Member Author

Maxhy commented Aug 31, 2015

@marinkobabic after testing your assumptions I'm not sure you're right as the issue remains with your unique set of changes. Did you have a chance to test it on your side?

@marinkobabic
Copy link
Contributor

Haven't tested it yet. On which operating system are you testing? Do I need any special configuration to get it failed.

@Maxhy
Copy link
Member Author

Maxhy commented Aug 31, 2015

I'm testing on Win8.1 x64.
No special configuration required. Basically just set volume device name to NULL and call IoRegisterFileSystem / IoUnregisterFileSystem.

Or see following patch that I used for my latest test: https://gist.github.com/Maxhy/5bcfec9c4350352c45aa

@Maxhy Maxhy added the Bug label Sep 1, 2015
@marinkobabic
Copy link
Contributor

I have done some research and here the results explained:

"If somebody tries to open a file the I/O manager checks to see whether \Device
DokanVolumexx’s VPB references a file system. If it doesn’t, the I/O manager asks each registered
file system driver via a mount request whether the driver recognizes the format of the volume in
question as the driver’s own."

This means that IoRegisterFileSystem for dokan is not a must. If you want to register the filesystem, so please do it in DriverEntry like in the example of fastfat and cdfs. The name you will use there will be RtlInitUnicodeString( &UnicodeString, L"\Dokan" ); This process is totally independent of other stuff we do in the driver. The device used for the IoRegisterFileSystem does not represent the Volume nor Disk Device.

Inside of the MountVolume routine IoCreateDevice should create a file system device without a name. The pointer of the file system device, which in this case represents the Volume must be set on DeviceObject of the vpb. The vpb->RealDevice is the device created using the option FILE_DEVICE_DISK and it is not the created volume device.

@Maxhy
Copy link
Member Author

Maxhy commented Sep 1, 2015

Mhhh good that's a progress but I still have some points to discuss.

  • For what I can see IoRegisterFileSystem is required to have Filter Manager support. Just for that I believe it cannot be avoided.
  • The File System device to register cannot be created in DeviceEntry because his type (FILE_DEVICE_NETWORK_FILE_SYSTEM / FILE_DEVICE_DISK_FILE_SYSTEM) must match volume device and is not yet known at this time.

@marinkobabic
Copy link
Contributor

It makes sense to finish this task. I would suggest to go on with this task and to complete it. From my point of view the highest priority has the oplock implementation, to be able to support Windows 8 and following systems properly. Please let me know, whenever you want me to make a review of the code.

@Maxhy
Copy link
Member Author

Maxhy commented Sep 1, 2015

That's a way to do it indeed :)
I first found it conceptually better to have one file system control device per user-mode file system, but that would be overkill for what it is used for. I guess you're again right here.

I will make the change on the branch (https://github.com/dokan-dev/dokany/blob/master/sys/fscontrol.c#L326 check will need to be changed too, maybe based on disk device name through ObQueryNameString) and let you know.

EDIT: about priorities, mount manager support is really important for me, then yes maybe oplock. PR are also highly appreciated ;)

@marinkobabic
Copy link
Contributor

Will see what I can do ;-)

Maxhy pushed a commit that referenced this issue Sep 7, 2015
Return fake drive layout/partition information
Maxhy pushed a commit that referenced this issue Oct 4, 2015
Return fake drive layout/partition information
@Maxhy
Copy link
Member Author

Maxhy commented Oct 4, 2015

@marinkobabic @Liryna @kenjiuno I had the opportunity to work again on mount manager support. The part related to this specific issue is completed and I would love to have your feedback on this before going further on mount manager support (mntmgr is basically working but improvements are still required to work 100% as excepted). See current changes on https://github.com/dokan-dev/dokany/tree/mountmanager branch. Thanks.

@marinkobabic
Copy link
Contributor

@Maxhy
So far all looks good. Did not test it, did just a review of your changes. One of the following steps is to remove the dokan_mounter completelly?

@Maxhy
Copy link
Member Author

Maxhy commented Oct 6, 2015

Ok thanks for the quick review.
Yes removing dokan_mounter is one of the following step. I believe I will work on this specific step in two weeks when I get some free time again.

Maxhy pushed a commit that referenced this issue Nov 21, 2015
Return fake drive layout/partition information
@Maxhy
Copy link
Member Author

Maxhy commented Jan 8, 2016

Ok closing this here as it is part of #50, continue discussion on this last thread.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants