Skip to content

Commit 540a437

Browse files
committed
dllinject: Track actual attribute writes instead of the requesting flag
Opening a file with the `FILE_WRITE_ATTRIBUTES` access right merely *requests* write access to attributes, and does not constitute a write in itself. Previously, this caused issues with tools that need to specify this flag for whatever reason, but didn't actually write or change the attributes of the files they opened with this flag. If those tools lie outside the source tree, Tup will in fact delete all of these files, forcing you to either reinstall the tool or recover the missing files from a backup. (Thankfully, write-protecting the compiler's directory prevents this from succeeding!) This change should still address the node.js use case that initially prompted the check for `FILE_WRITE_ATTRIBUTES` in c7160c8. I couldn't find a definitive list of everything that counts as "attributes", but it does seem to be limited to the timestamps and attribute bits covered by the `FILE_BASIC_INFORMATION` structure.
1 parent 748b7c7 commit 540a437

File tree

1 file changed

+25
-2
lines changed

1 file changed

+25
-2
lines changed

src/dllinject/dllinject.c

Lines changed: 25 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -422,7 +422,7 @@ static NtSetInformationFile_t NtSetInformationFile_orig;
422422
static access_t _access_orig;
423423
static rename_t rename_orig;
424424

425-
#define TUP_CREATE_WRITE_FLAGS (GENERIC_WRITE | FILE_APPEND_DATA | FILE_WRITE_DATA | FILE_WRITE_ATTRIBUTES)
425+
#define TUP_CREATE_WRITE_FLAGS (GENERIC_WRITE | FILE_APPEND_DATA | FILE_WRITE_DATA)
426426
/* Including ddk/wdm.h causes other issues, and this is all we need... */
427427
#define FILE_OPEN_FOR_BACKUP_INTENT 0x00004000
428428

@@ -787,10 +787,30 @@ NTSTATUS WINAPI NtSetInformationFile_hook(
787787
DWORD len = 0;
788788
int failed = 0;
789789

790+
/* * A value of 0 in any field preserves the file's current value for this
791+
* specific attribute.
792+
* * -1 or -2 in the timestamp fields disable or enable the default
793+
* timestamp updates for file I/O:
794+
*
795+
* https://learn.microsoft.com/en-us/windows-hardware/drivers/ddi/wdm/ns-wdm-_file_basic_information
796+
*
797+
* * SetFileInformationByHandle() returns ERROR_INVALID_PARAMETER for any
798+
* other negative number.
799+
*/
800+
FILE_BASIC_INFORMATION* basic = FileInformation;
801+
int relevant_basic_change = (FileInformationClass == FileBasicInformation) && (
802+
(int64_t)(basic->CreationTime.QuadPart) >= 1 ||
803+
(int64_t)(basic->LastAccessTime.QuadPart) >= 1 ||
804+
(int64_t)(basic->LastWriteTime.QuadPart) >= 1 ||
805+
(int64_t)(basic->ChangeTime.QuadPart) >= 1 ||
806+
basic->FileAttributes != 0
807+
);
808+
790809
/* We need to get the old path before calling NtSetInformationFile in
791810
* case of a rename, in which case that information is lost.
792811
*/
793-
if(FileInformationClass == FileRenameInformation ||
812+
if(relevant_basic_change ||
813+
FileInformationClass == FileRenameInformation ||
794814
FileInformationClass == FileRenameInformationEx ||
795815
FileInformationClass == FileDispositionInformation) {
796816
len = GetFinalPathNameByHandleW(FileHandle, widepath, WIDE_PATH_MAX, FILE_NAME_NORMALIZED);
@@ -831,6 +851,9 @@ NTSTATUS WINAPI NtSetInformationFile_hook(
831851
DEBUG_HOOK("NtSetInformationFile[%i]: dont delete on close '%S'\n", FileInformationClass, widepath);
832852
handle_file_w(widepath, len, NULL, ACCESS_WRITE);
833853
}
854+
} else if(relevant_basic_change) {
855+
DEBUG_HOOK("NtSetInformationFile[%i]: set basic information '%S'\n", FileInformationClass, widepath);
856+
handle_file_w(widepath, len, NULL, ACCESS_WRITE);
834857
}
835858

836859
out_exit:

0 commit comments

Comments
 (0)