On Sat, Sep 08, 2018 at 10:45:16AM +0800, Dmitry Timoshkov wrote:
Brock York twunknown@gmail.com wrote:
attr.ObjectName = &nt_replaced_name;
- status = NtOpenFile(&hReplaced, GENERIC_READ|GENERIC_WRITE|DELETE|SYNCHRONIZE,
- status = NtOpenFile(&hReplaced, GENERIC_READ|DELETE|SYNCHRONIZE, &attr, &io, FILE_SHARE_READ|FILE_SHARE_WRITE|FILE_SHARE_DELETE, FILE_SYNCHRONOUS_IO_NONALERT|FILE_NON_DIRECTORY_FILE);
@@ -1788,6 +1789,19 @@ BOOL WINAPI ReplaceFileW(LPCWSTR lpReplacedFileName, LPCWSTR lpReplacementFileNa goto fail; }
- /* Replacement should fail if replaced is READ_ONLY */
- if (!GetFileInformationByHandle(hReplaced, &file_info))
- {
error = GetLastError();goto fail;- }
- if (file_info.dwFileAttributes & FILE_ATTRIBUTE_READONLY)
- {
error = ERROR_ACCESS_DENIED;goto fail;- }
There is an existing test for NtCreateFile that it's supposed to fail if the file has read-only attribute. Probably NtOpenFile with WRITE|DELETE access supposed to behave in a similar way. Adding such workarounds to high level API like ReplaceFile only clutters the code with not appropriate hacks.
-- Dmitry.
Hello Dmitry
I had written a test to check that exact case before I implemented this patch but it turns out that I had screwed up my test program and was trying to NtOpenFile the wrong file... Which made me then think that the ACCESS_DENIED was coming from the ReplaceFileW call itself.
The MSDN documentation for ReplaceFileW states that the replaced file is opened with GENERIC_READ, DELETE and SYNCHRONISE. But the next sentence after mentions that the user must have write access, is this implying that the GENERIC_WRITE flag in passed in the like original author of ReplaceFileW for wine had implemented or is the MSDN probably wrong? ? If so the fix is easy modifying the check from NtOpenFile to handle the access_denied case which you were correct that when WRITE | DELETE is passed as the access flags to open a read only file NtOpenFile correctly returns ACCESS_DENIED
Thanks for the good pickup and feedback.
Warm regards Brock