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

Read-only device option #72

Closed
wants to merge 1 commit into from
Closed

Read-only device option #72

wants to merge 1 commit into from

Conversation

justanotheranonymoususer
Copy link
Contributor

I haven't tested this pull request, because I cannot rebuild the driver. If you could check it, or build the driver and let me check, that would be great.

@Liryna
Copy link
Member

Liryna commented Sep 29, 2015

Thanks !
I will test it in the next days :)
Edit: I will make a release for you today

@Liryna
Copy link
Member

Liryna commented Sep 30, 2015

Sorry but I wasn't able to make it work 😢
`

mirror.exe /d /s /o /r C:\Users /l m
...
WriteFile : C:\Users\liryna\Desktop\New Text Document.txt, offset 0, length 17
write 17, offset 0`

Is there something I made wrong ?

@justanotheranonymoususer
Copy link
Contributor Author

Thanks for trying!

Is there something I made wrong ?

You, as the repo owner, should know better :)
I implemented it based on the DOKAN_EVENT_REMOVABLE flag. Don't know whether I did it correctly. We can try a couple of things, e.g. always forcing the FILE_READ_ONLY_DEVICE flag and checking whether it has any effect at all.

@marinkobabic
Copy link
Contributor

What means you was not able to get it work?

Step 1:
Ensure that the device characteristics is set properly using the new parameter.

Step 2:
Driver should check the device characteristics and respond properly to any CUD operation.

@Liryna
Copy link
Member

Liryna commented Sep 30, 2015

@marinkobabic, I was able to write in a file from the dokan device with the read only option.
I didn't look more after.
I think the Step 2 you describe is not implemented here.

@marinkobabic
Copy link
Contributor

@Liryna
You are right. Just put it into the todo list. Should in generally not be a big effort.

@justanotheranonymoususer
Copy link
Contributor Author

I found out that FILE_READ_ONLY_VOLUME can be set here:

dokany/dokan_mirror/mirror.c

Lines 1109 to 1113 in 7799f88

*FileSystemFlags = FILE_CASE_SENSITIVE_SEARCH |
FILE_CASE_PRESERVED_NAMES |
FILE_SUPPORTS_REMOTE_STORAGE |
FILE_UNICODE_ON_DISK |
FILE_PERSISTENT_ACLS;

The solution doesn't seem to be complete, e.g. files cannot be edited, but attributes can still be set, I think (haven't done a through check).

@canardos
Copy link
Contributor

canardos commented Nov 9, 2015

As pointed out above, the changes aren't achieving the desired result because the file system is not checking if the device is writable. The flags themselves are not sufficient.

NTFS and FAT etc. use a IOCTL_DISK_IS_WRITABLE device req when mounting to determine if the device is writable and set the file system flags accordingly. IO operations are then restricted as appropriate by the file system. The MS fastfat sample follows the same approach.

I've implemented the read-only option in the same style, with IOCTL_DISK_IS_WRITABLE and IOCTL_VOLUME_GET_GPT_ATTRIBUTES device requests returning read-only and dokan checking write requests at the driver level. It seems to function well and Explorer correctly indicates that the volume is write-protected during write operations.

I'll upload a pull-request tomorrow after I've tested it on Win7 (only tested on Win10 thus far).

@Liryna
Copy link
Member

Liryna commented Nov 10, 2015

I would be glad to review your PR @canardos :) !

@Liryna
Copy link
Member

Liryna commented Nov 11, 2015

I close since #105 has been merged.

@Liryna Liryna closed this Nov 11, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants