Tests with non-Cygwin executables show that other toolchains (LLVM LLD,
MS link without /RELEASE option) leave the PE checksum field as zero.
Peflags+rebase 4.6.6 still work correctly then, but rebase prints a
misleading warning 'Checksum update failed' in this case. With the
attached patch, a zero checksum is left as is and no message is printed.
This currently does not affect typical Cygwin use cases, so there is IMO
no urgent need for a new package.
BTW, there was possibly no announcement for the rebase 4.6.6 package.
--
Regards,
Christian
From 855be8d72a1f96d0a3cfcb0a97fbe12a9a912748 Mon Sep 17 00:00:00 2001
From: Christian Franke <christian.fra...@t-online.de>
Date: Mon, 9 Oct 2023 09:56:05 +0200
Subject: [PATCH] Fix handling of unset PE checksums
Leave unset (zero) checksums as is unless checksum update is
explicitly requested. Remove misleading peflags warning
'Checksum update failed'.
Signed-off-by: Christian Franke <christian.fra...@t-online.de>
---
imagehelper/objectfile.cc | 9 ++++++--
peflags.c | 45 ++++++++++++++++++++++-----------------
rebase.c | 7 +++---
3 files changed, 36 insertions(+), 25 deletions(-)
diff --git a/imagehelper/objectfile.cc b/imagehelper/objectfile.cc
index 651fcce..ce4b444 100755
--- a/imagehelper/objectfile.cc
+++ b/imagehelper/objectfile.cc
@@ -497,13 +497,18 @@ BOOL LinkedObjectFile::updateCheckSum()
p_checksum = &getNTHeader64 ()->OptionalHeader.CheckSum;
else
p_checksum = &getNTHeader32 ()->OptionalHeader.CheckSum;
+ uint32_t checksum = *p_checksum;
+
+ // Leave unset checksum as is
+ if (!checksum)
+ return TRUE;
// Add changes from NT header to checksum updated by relocs
uint16_t checksum16;
if (relocs)
checksum16 = relocs->getCheckSum16();
else
- checksum16 = pe32_checksum_update_begin(*p_checksum, FileSize);
+ checksum16 = pe32_checksum_update_begin(checksum, FileSize);
if (is64bit ())
checksum16 = pe32_checksum_update_add(checksum16, &old_ntheader64,
@@ -514,7 +519,7 @@ BOOL LinkedObjectFile::updateCheckSum()
getNTHeader32 (),
sizeof(old_ntheader32));
- uint32_t checksum = pe32_checksum_update_end(checksum16, FileSize);
+ checksum = pe32_checksum_update_end(checksum16, FileSize);
if (!checksum)
return FALSE;
diff --git a/peflags.c b/peflags.c
index 917088b..14ea78d 100644
--- a/peflags.c
+++ b/peflags.c
@@ -508,7 +508,7 @@ do_mark (const char *pathname)
}
- /* Update the checksum. */
+ /* Update the checksum if set. */
changed = FALSE;
{
const void *p_oldheader, *p_newheader;
@@ -529,20 +529,24 @@ do_mark (const char *pathname)
p_checksum = &pep->ntheader32->OptionalHeader.CheckSum;
}
- hdr_checksum = pe32_checksum_update_end(
- pe32_checksum_update_add(
- pe32_checksum_update_begin(*p_checksum, pep->filesize),
- p_oldheader, p_newheader, headersize
- ),
- pep->filesize
- );
-
- if (!hdr_checksum)
- fprintf(stderr, "%s: Checksum update failed\n", pathname);
- else if (*p_checksum != hdr_checksum)
+ hdr_checksum = *p_checksum;
+ if (hdr_checksum)
{
- changed = TRUE;
- *p_checksum = hdr_checksum;
+ ULONG new_hdr_checksum = pe32_checksum_update_end (
+ pe32_checksum_update_add (
+ pe32_checksum_update_begin (hdr_checksum, pep->filesize),
+ p_oldheader, p_newheader, headersize
+ ),
+ pep->filesize
+ );
+
+ /* Update the checksum or set it to 0 if the old value is invalid. */
+ if (new_hdr_checksum != hdr_checksum
+ && (mark_any || handle_any_sizeof == DO_WRITE))
+ {
+ changed = TRUE;
+ *p_checksum = hdr_checksum = new_hdr_checksum;
+ }
}
}
@@ -650,7 +654,7 @@ static void
do_checksum (const char *pathname, int indent, int changed, ULONG hdr_checksum)
{
const char name[] = "PE file checksum";
- if (checksum_option > 1)
+ if (checksum_option > 1 && (checksum_option > 2 || hdr_checksum))
{
int fd;
unsigned new_checksum, old_checksum;
@@ -684,10 +688,11 @@ do_checksum (const char *pathname, int indent, int
changed, ULONG hdr_checksum)
printf ("%*s%-24s: 0x%08x (*needs update to 0x%08x*)\n", indent, "",
name,
old_checksum, new_checksum);
}
- else /* (checksum_option == 1) */
+ else /* (checksum_option == 1 || (checksum_option == 2 && !hdr_checksum)) */
{
- printf ("%*s%-24s: 0x%08x (%schanged, not verified)\n", indent, "", name,
- hdr_checksum, (changed ? "" : "un"));
+ printf ("%*s%-24s: 0x%08x (%schanged%s)\n", indent, "", name,
+ hdr_checksum, (changed ? "" : "un"),
+ (hdr_checksum ? ", not verified" : ""));
}
}
@@ -1265,8 +1270,8 @@ help (FILE *f)
" separately in another file.\n"
" -k, --checksum [BOOL] Displays (no argument), verifies (false) or\n"
" always fixes (true) the file checksum in the
PE\n"
-" header. By default, the checksum is updated\n"
-" without reading the whole file under the\n"
+" header. By default, a set (nonzero) checksum
is\n"
+" updated without reading the whole file under
the\n"
" assumption that the original checksum was\n"
" correct.\n"
" -x, --stack-reserve [NUM] Reserved stack size of the process in bytes.\n"
diff --git a/rebase.c b/rebase.c
index 6a531d0..3325c5e 100644
--- a/rebase.c
+++ b/rebase.c
@@ -1676,9 +1676,10 @@ Rebase PE files, usually DLLs, to a specified address or
address range.\n\
-c, --checksum Fix the file's checksum in the PE header if the
file\n\
has been successfully rebased. This also bumps
the\n\
file's modification time (like -t) if the checksum\n\
- has been changed. By default, the checksum is\n\
- updated without reading the whole file under the\n\
- assumption that the original checksum was correct.\n\
+ has been changed. By default, a set (nonzero)\n\
+ checksum is updated without reading the whole file\n\
+ under the assumption that the original checksum
was\n\
+ correct.\n\
-d, --down Treat the BaseAddress as upper ceiling and rebase\n\
files top-down from there. Without this option
the\n\
files are rebased from BaseAddress bottom-up.\n\
--
2.39.0
