public inbox for cygwin-apps@cygwin.com
 help / color / mirror / Atom feed
From: Corinna Vinschen <corinna-cygwin@cygwin.com>
To: Christian Franke <Christian.Franke@t-online.de>
Cc: cygwin-apps@cygwin.com
Subject: Re: [PATCH rebase 1/2] Always update the file checksum in the PE header
Date: Tue, 15 Aug 2023 10:23:43 +0200	[thread overview]
Message-ID: <ZNs2D7RnVRILLJt0@calimero.vinschen.de> (raw)
In-Reply-To: <b8e6aff4-9db2-b6ce-ee71-91955c19d8cc@t-online.de>

On Aug 12 14:26, Christian Franke via Cygwin-apps wrote:
> This patch is still experimental, but tested with all /bin/cyg*.dll from my
> installation.

Does that mean I shouldn't apply it for now, or do you want me to
apply it but not create a new release yet?


Thanks,
Corinna


> 
> -- 
> Regards,
> Christian
> 

> From 53663d46c2c989e665143b33c0614b416fd1c666 Mon Sep 17 00:00:00 2001
> From: Christian Franke <christian.franke@t-online.de>
> Date: Sat, 12 Aug 2023 14:13:43 +0200
> Subject: [PATCH] Always update the file checksum in the PE header
> 
> Both peflags and rebase now update the checksum without reading
> the whole file under the assumption that the original checksum
> was correct.  Calculation is done by new module
> 'imagehelper/pechecksum_update.c'.
> Slightly change the peflags -k, --checksum option accordingly.
> 
> Signed-off-by: Christian Franke <christian.franke@t-online.de>
> ---
>  Makefile.in                     |   6 +-
>  imagehelper/Makefile.in         |   7 ++-
>  imagehelper/imagehelper.h       |   4 ++
>  imagehelper/objectfile.cc       |  62 +++++++++++++++---
>  imagehelper/objectfile.h        |   7 +++
>  imagehelper/pechecksum_update.c |  56 +++++++++++++++++
>  imagehelper/pechecksum_update.h |  52 ++++++++++++++++
>  imagehelper/rebaseimage.cc      |   4 ++
>  imagehelper/sections.cc         |  20 +++++-
>  imagehelper/sections.h          |   8 ++-
>  peflags.c                       | 107 +++++++++++++++++++++-----------
>  rebase.c                        |  39 ++++++++----
>  12 files changed, 305 insertions(+), 67 deletions(-)
>  create mode 100644 imagehelper/pechecksum_update.c
>  create mode 100644 imagehelper/pechecksum_update.h
> 
> diff --git a/Makefile.in b/Makefile.in
> index e7b7f6a..edc9eda 100644
> --- a/Makefile.in
> +++ b/Makefile.in
> @@ -81,7 +81,7 @@ REBASE_DUMP_OBJS = rebase-dump.$(O) rebase-db.$(O) $(LIBOBJS)
>  REBASE_DUMP_LIBS =
>  
>  PEFLAGS_OBJS = peflags.$(O) pechecksum.$(O) $(LIBOBJS)
> -PEFLAGS_LIBS =
> +PEFLAGS_LIBS = $(LIBIMAGEHELPER)
>  
>  SRC_DISTFILES = configure.ac configure Makefile.in \
>  	peflagsall.in rebaseall.in \
> @@ -109,8 +109,8 @@ rebase-dump$(EXEEXT): $(REBASE_DUMP_OBJS)
>  
>  rebase-dump.$(O):: rebase-dump.c rebase-db.h Makefile
>  
> -peflags$(EXEEXT): $(PEFLAGS_OBJS)
> -	$(CC) $(CFLAGS) $(LDFLAGS) -o $@ $(PEFLAGS_OBJS)
> +peflags$(EXEEXT): $(PEFLAGS_OBJS) $(PEFLAGS_LIBS)
> +	$(CXX) $(CXXFLAGS) $(LDFLAGS) $(CXX_LDFLAGS) -o $@ $(PEFLAGS_OBJS) $(PEFLAGS_LIBS)
>  
>  peflags.$(O):: peflags.c pechecksum.h Makefile
>  
> diff --git a/imagehelper/Makefile.in b/imagehelper/Makefile.in
> index 0ed8e6c..a40cd8c 100755
> --- a/imagehelper/Makefile.in
> +++ b/imagehelper/Makefile.in
> @@ -75,11 +75,12 @@ LIB_TARGET=imagehelper
>  LIB_TARGET_FILE=libimagehelper.a
>  LIB_OBJS = objectfile.$(O) objectfilelist.$(O) sections.$(O) debug.$(O) \
>  	rebaseimage.$(O) checkimage.$(O) fiximage.$(O) getimageinfos.$(O) \
> -	bindimage.$(O)
> +	bindimage.$(O) pechecksum_update.$(O)
>  LIB_SRCS = objectfile.cc objectfilelist.cc sections.cc debug.cc \
>  	rebaseimage.cc checkimage.cc fiximage.cc getimageinfos.cc \
> -	bindimage.cc
> -LIB_HDRS = objectfilelist.h imagehelper.h sections.h objectfile.h
> +	bindimage.cc pechecksum_update.c
> +LIB_HDRS = objectfilelist.h imagehelper.h sections.h objectfile.h \
> +	pechecksum_update.h
>  
>  #
>  # (obsolete) applications
> diff --git a/imagehelper/imagehelper.h b/imagehelper/imagehelper.h
> index bafc106..8535311 100755
> --- a/imagehelper/imagehelper.h
> +++ b/imagehelper/imagehelper.h
> @@ -33,6 +33,10 @@ extern BOOL ReBaseChangeFileTime;
>  /* Set to TRUE, if rebasing should also drop the /DYNAMICBASE flag
>     from the PE flags. */
>  extern BOOL ReBaseDropDynamicbaseFlag;
> +/* Set to TRUE if ReBaseImage{64} should update the PE checksum.
> +   Cleared if the update failed due to inconsistent original
> +   checksum. */
> +extern BOOL ReBaseUpdateCheckSum;
>  
>  BOOL ReBaseImage64(
>    LPCSTR CurrentImageName,
> diff --git a/imagehelper/objectfile.cc b/imagehelper/objectfile.cc
> index 4dafa6a..651fcce 100755
> --- a/imagehelper/objectfile.cc
> +++ b/imagehelper/objectfile.cc
> @@ -24,6 +24,7 @@
>  #include <time.h>
>  
>  #include "objectfile.h"
> +#include "pechecksum_update.h"
>  
>  // read a dll into the cache
>  
> @@ -109,6 +110,15 @@ ObjectFile::ObjectFile(const char *aFileName, bool writeable)
>        FileName = strdup(name);
>      }
>  
> +  LARGE_INTEGER fs;
> +  if (!GetFileSizeEx(hfile, &fs))
> +    {
> +      CloseHandle(hfile);
> +      Error = 2;
> +      return;
> +    }
> +  FileSize = fs.QuadPart;
> +
>    hfilemapping = CreateFileMapping(hfile, NULL, writeable ? PAGE_READWRITE : PAGE_READONLY , 0, 0,  NULL);
>    if (hfilemapping == 0)
>      {
> @@ -139,8 +149,7 @@ ObjectFile::ObjectFile(const char *aFileName, bool writeable)
>        return;
>      }
>    // filesize big enough to allow at least reading the NT header?
> -  if (GetFileSize (hfile, NULL)
> -      < (char *) ntheader - (char *) dosheader + sizeof *ntheader)
> +  if (FileSize < (char *) ntheader - (char *) dosheader + sizeof *ntheader)
>      {
>        Error = 4;
>        return;
> @@ -178,9 +187,6 @@ ObjectFile::~ObjectFile()
>      CloseHandle(hfile);
>  }
>  
> -
> -
> -
>  int LinkedObjectFile::level = 0;
>  
>  LinkedObjectFile::LinkedObjectFile(const char *aFileName, bool writable) : ObjectFile(aFileName,writable)
> @@ -204,6 +210,12 @@ LinkedObjectFile::LinkedObjectFile(const char *aFileName, bool writable) : Objec
>    PIMAGE_NT_HEADERS32 ntheader32 = getNTHeader32 ();
>    PIMAGE_NT_HEADERS64 ntheader64 = getNTHeader64 ();
>  
> +  // Save original header for checksum update
> +  if (is64bit ())
> +    old_ntheader64 = *ntheader64;
> +  else
> +    old_ntheader32 = *ntheader32;
> +
>    Section *edata = sections->find(".edata");
>    if (edata)
>      exports = new Exports(*edata);
> @@ -231,9 +243,13 @@ LinkedObjectFile::LinkedObjectFile(const char *aFileName, bool writable) : Objec
>        imports = new Imports(*sections, dir);
>      }
>  
> +  uint32_t checksum;
> +  if (is64bit ())
> +    checksum = getNTHeader64 ()->OptionalHeader.CheckSum;
> +  else
> +    checksum = getNTHeader32 ()->OptionalHeader.CheckSum;
>  
> -
> -  relocs = new Relocations(*sections,".reloc");
> +  relocs = new Relocations(*sections, ".reloc", pe32_checksum_update_begin(checksum, FileSize));
>  
>  }
>  
> @@ -474,6 +490,38 @@ bool LinkedObjectFile::unbind(void)
>    return true;
>  }
>  
> +BOOL LinkedObjectFile::updateCheckSum()
> +{
> +  ULONG *p_checksum;
> +  if (is64bit ())
> +    p_checksum = &getNTHeader64 ()->OptionalHeader.CheckSum;
> +  else
> +    p_checksum = &getNTHeader32 ()->OptionalHeader.CheckSum;
> +
> +  // 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);
> +
> +  if (is64bit ())
> +    checksum16 = pe32_checksum_update_add(checksum16, &old_ntheader64,
> +					  getNTHeader64 (),
> +					  sizeof(old_ntheader64));
> +  else
> +    checksum16 = pe32_checksum_update_add(checksum16, &old_ntheader32,
> +					 getNTHeader32 (),
> +					 sizeof(old_ntheader32));
> +
> +  uint32_t checksum = pe32_checksum_update_end(checksum16, FileSize);
> +  if (!checksum)
> +    return FALSE;
> +
> +  // Update the PE header
> +  *p_checksum = checksum;
> +  return TRUE;
> +}
>  
>  LinkedObjectFile::~LinkedObjectFile()
>  {
> diff --git a/imagehelper/objectfile.h b/imagehelper/objectfile.h
> index cd9509c..9deda1f 100755
> --- a/imagehelper/objectfile.h
> +++ b/imagehelper/objectfile.h
> @@ -92,6 +92,7 @@ class ObjectFile : public Base
>  
>    protected:
>      char *FileName;
> +    ULONG64 FileSize;
>      HANDLE hfile;
>      HANDLE hfilemapping;
>      LPVOID lpFileBase;
> @@ -140,12 +141,18 @@ class LinkedObjectFile : public ObjectFile
>        return exports;
>      }
>  
> +    BOOL updateCheckSum();
> +
>    protected:
>      Imports *imports;
>      Exports *exports;
>      Relocations *relocs;
>      static int level;
>      bool isPrinted;
> +    union {
> +      IMAGE_NT_HEADERS32 old_ntheader32;
> +      IMAGE_NT_HEADERS64 old_ntheader64;
> +    };
>    };
>  
>  #include "objectfilelist.h"
> diff --git a/imagehelper/pechecksum_update.c b/imagehelper/pechecksum_update.c
> new file mode 100644
> index 0000000..c15579d
> --- /dev/null
> +++ b/imagehelper/pechecksum_update.c
> @@ -0,0 +1,56 @@
> +/*
> + * PE32 checksum update
> + *
> + * Copyright (C) 2023 Christian Franke
> + *
> + * SPDX-License-Identifier: BSD-3-Clause
> + */
> +
> +#include "pechecksum_update.h"
> +
> +uint16_t pe32_checksum_update_begin(uint32_t checksum, unsigned long long filesize)
> +{
> +  if (!(0 < checksum && filesize < checksum && checksum - filesize <= 0xffffULL))
> +    return 0;
> +  return (uint16_t)(checksum - filesize);
> +}
> +
> +static inline uint16_t get_word(const void * p, unsigned i)
> +{
> +  const uint8_t * bp = p;
> +  return bp[i] | (bp[i + 1] << 8);
> +}
> +
> +uint16_t pe32_checksum_update_add(uint16_t checksum16, const void * old_block,
> +                                  const void * new_block, unsigned size)
> +{
> +  if (!(checksum16 && !((uintptr_t)old_block & 1)
> +        && !((uintptr_t)new_block & 1) && !(size & 1)))
> +    return 0;
> +  uint32_t sum = checksum16;
> +
> +  for (unsigned i = 0; i < size; i += 2) {
> +    uint16_t old_word = get_word(old_block, i);
> +    uint16_t new_word = get_word(new_block, i);
> +    if (new_word == old_word)
> +      continue;
> +    // Note: This does not work for updates of the first nonzero word of the file.
> +    // This does not matter because the first word "MZ" is never changed.
> +    if (old_word < sum) // Never return to 0, so no '<='.
> +      sum -= old_word;
> +    else // Revert previous 16-bit overflow, skip 0.
> +      sum += 0xffffU - old_word;
> +    sum += new_word;
> +    // If 16-bit overflow, skip 0.
> +    sum = (sum + (sum >> 16)) & 0xffffU;
> +  }
> +
> +  return (uint16_t)sum;
> +}
> +
> +uint32_t pe32_checksum_update_end(uint16_t checksum16, unsigned long long filesize)
> +{
> +  if (!(checksum16 && checksum16 + filesize < 0xffffffffULL))
> +    return 0;
> +  return (uint32_t)(checksum16 + filesize);
> +}
> diff --git a/imagehelper/pechecksum_update.h b/imagehelper/pechecksum_update.h
> new file mode 100644
> index 0000000..7a2c911
> --- /dev/null
> +++ b/imagehelper/pechecksum_update.h
> @@ -0,0 +1,52 @@
> +/*
> + * PE32 checksum update
> + *
> + * Copyright (C) 2023 Christian Franke
> + *
> + * SPDX-License-Identifier: BSD-3-Clause
> + */
> +
> +#ifndef PE_CHECKSUM_UPDATE_H
> +#define PE_CHECKSUM_UPDATE_H
> +
> +#include <stdint.h>
> +
> +#ifdef __cplusplus
> +extern "C" {
> +#endif
> +
> +/*
> + * Begin checksum update sequence.
> + * checksum: Old 32-bit checksum from PE header.
> + * filesize: Size of PE file.
> + * return value: 16-bit checksum for p32_checksum_update_add()
> + * or 0 on error (the checksum itself is never 0).
> + */
> +uint16_t pe32_checksum_update_begin(uint32_t checksum, unsigned long long filesize);
> +
> +/*
> + * Update checksum.
> + * checksum16: Current 16-bit checksum from p32_checksum_update_begin()
> + * or this function.
> + * old_block: Address of old (original) data block, must be word aligned.
> + * new_block: Address of new (changed) data block, must be word aligned.
> + * size: Number of bytes of both data blocks, must be even.
> + * The PE checksum field may be included if identical in both blocks.
> + * return value: Updated 16-bit checksum or 0 on error.
> + */
> +uint16_t pe32_checksum_update_add(uint16_t checksum16, const void * old_block,
> +                                  const void * new_block, unsigned size);
> +
> +/*
> + * End checksum update sequence.
> + * checksum16: New 16-bit checksum from pe32_checksum_update_add().
> + * filesize: Size of PE file.
> + * return value: New 32-bit checksum for PE header or 0 on error.
> + */
> +uint32_t pe32_checksum_update_end(uint16_t checksum16, unsigned long long filesize);
> +
> +#ifdef __cplusplus
> +}
> +#endif
> +
> +#endif /* PE_CHECKSUM_UPDATE_H */
> diff --git a/imagehelper/rebaseimage.cc b/imagehelper/rebaseimage.cc
> index 2f1fb03..8827a14 100755
> --- a/imagehelper/rebaseimage.cc
> +++ b/imagehelper/rebaseimage.cc
> @@ -32,6 +32,7 @@
>  
>  BOOL ReBaseChangeFileTime = FALSE;
>  BOOL ReBaseDropDynamicbaseFlag = FALSE;
> +BOOL ReBaseUpdateCheckSum = FALSE;
>  
>  BOOL ReBaseImage64 (
>    LPCSTR CurrentImageName,
> @@ -141,6 +142,9 @@ BOOL ReBaseImage64 (
>  	  &= ~IMAGE_DLLCHARACTERISTICS_DYNAMIC_BASE;
>      }
>  
> +  if (ReBaseUpdateCheckSum && !dll.updateCheckSum())
> +    ReBaseUpdateCheckSum = FALSE;
> +
>    if (!fGoingDown)
>      *NewImageBase += *NewImageSize;
>  
> diff --git a/imagehelper/sections.cc b/imagehelper/sections.cc
> index dbf7da0..76f46ee 100755
> --- a/imagehelper/sections.cc
> +++ b/imagehelper/sections.cc
> @@ -22,8 +22,10 @@
>  
>  #include <iostream>
>  #include <iomanip>
> +#include <string.h>
>  
>  #include "sections.h"
> +#include "pechecksum_update.h"
>  
>  int Base::debug = 0;
>  
> @@ -270,10 +272,11 @@ void Imports::dump(const char *title)
>  //----- class Imports --------------------------------------------------
>  
>  
> -Relocations::Relocations(SectionList &sectionList, const char *sectionName)
> +Relocations::Relocations(SectionList &sectionList, const char *sectionName, uint16_t checksum16_)
>  {
>    sections = &sectionList;
>    Section *asection = sections->find(sectionName);
> +  checksum16 = checksum16_;
>    if (asection)
>      {
>        relocs = PIMAGE_BASE_RELOCATION (asection->getStartAddress());
> @@ -363,8 +366,11 @@ bool Relocations::fix(void)
>          std::cerr << "no errors found" << std::endl;
>      }
>    else
> -    if (debug)
> -      std::cerr << "corrupted relocation records fixed" << std::endl;
> +    {
> +      checksum16 = 0; // Checksum update not implemented.
> +      if (debug)
> +	std::cerr << "corrupted relocation records fixed" << std::endl;
> +    }
>    return true;
>  }
>  
> @@ -425,7 +431,12 @@ bool Relocations::relocate(int64_t difference)
>  		    << std::endl;
>  		  }
>  		int32_t *patch_adr = (int *)cursec->rva2real(location);
> +		unsigned cs_offs = (uintptr_t)patch_adr & 1; // 32-bit relocations may be not word aligned.
> +		const uint8_t *cs_adr = (const uint8_t *)patch_adr - cs_offs;
> +		unsigned cs_size = sizeof(uint32_t) + cs_offs * 2;
> +		uint8_t old_val[cs_size]; memcpy(old_val, cs_adr, cs_size);
>  		*patch_adr += difference;
> +		checksum16 = pe32_checksum_update_add(checksum16, old_val, cs_adr, cs_size);
>  	      }
>  	      break;
>  	    case IMAGE_REL_BASED_DIR64:
> @@ -440,7 +451,10 @@ bool Relocations::relocate(int64_t difference)
>  		    << std::endl;
>  		  }
>  		int64_t *patch_adr = (int64_t *)cursec->rva2real(location);
> +		int64_t old_val = *patch_adr;
>  		*patch_adr += difference;
> +		// Assume that 64-bit relocations are always word aligned.
> +		checksum16 = pe32_checksum_update_add(checksum16, &old_val, patch_adr, sizeof(old_val));
>  	      }
>  	      break;
>  	    default:
> diff --git a/imagehelper/sections.h b/imagehelper/sections.h
> index b317fa1..20dd24c 100755
> --- a/imagehelper/sections.h
> +++ b/imagehelper/sections.h
> @@ -199,7 +199,7 @@ class Relocations : SectionBase
>    {
>    public:
>      // create a relocation object using section named "section" from  the section list "sections"
> -    Relocations(SectionList &sectionList, const char *sectionName);
> +    Relocations(SectionList &sectionList, const char *sectionName, uint16_t checksum16_);
>  
>      // check for bad relocations
>      bool check(void);
> @@ -210,10 +210,16 @@ class Relocations : SectionBase
>      // precondition: fixed dll
>      bool relocate(int64_t difference);
>  
> +    uint16_t getCheckSum16() const
> +      {
> +	return checksum16;
> +      }
> +
>    private:
>      PIMAGE_BASE_RELOCATION relocs;
>      SectionList *sections;
>      int size;   // section size
> +    uint16_t checksum16;
>    };
>  
>  #endif
> diff --git a/peflags.c b/peflags.c
> index 1a61da7..917088b 100644
> --- a/peflags.c
> +++ b/peflags.c
> @@ -44,6 +44,7 @@
>  #include <windows.h>
>  
>  #include "pechecksum.h"
> +#include "imagehelper/pechecksum_update.h"
>  
>  #if defined(__MSYS__)
>  /* MSYS has no strtoull */
> @@ -67,12 +68,12 @@ static WORD coff_characteristics_show;
>  static WORD pe_characteristics_set;
>  static WORD pe_characteristics_clr;
>  static WORD pe_characteristics_show;
> -static short checksum_show;
> -static short checksum_update; /* 1 = update after changes, 2 = fix always */
> +static short checksum_option; /* 1 = show, 2 = verify, 3 = fix */
>  
>  typedef struct
>  {
>    const char *pathname;
> +  unsigned long long filesize;
>    PIMAGE_DOS_HEADER dosheader;
>    union
>      {
> @@ -507,24 +508,48 @@ do_mark (const char *pathname)
>  
>      }
>  
> -  /* Any actual changes? */
> -  if (pep->is_64bit)
> -    {
> -      changed = memcmp(&old_ntheader.ntheader64, pep->ntheader64,
> -		       sizeof(old_ntheader.ntheader64));
> -      hdr_checksum = pep->ntheader64->OptionalHeader.CheckSum;
> -    }
> -  else
> -    {
> -      changed = memcmp(&old_ntheader.ntheader32, pep->ntheader64,
> -		       sizeof(old_ntheader.ntheader32));
> -      hdr_checksum = pep->ntheader32->OptionalHeader.CheckSum;
> -    }
> +  /* Update the checksum. */
> +  changed = FALSE;
> +  {
> +    const void *p_oldheader, *p_newheader;
> +    unsigned headersize;
> +    ULONG *p_checksum;
> +    if (pep->is_64bit)
> +      {
> +	p_oldheader = &old_ntheader.ntheader64;
> +	p_newheader = pep->ntheader64;
> +	headersize = sizeof(old_ntheader.ntheader64);
> +	p_checksum = &pep->ntheader64->OptionalHeader.CheckSum;
> +      }
> +    else
> +      {
> +	p_oldheader = &old_ntheader.ntheader32;
> +	p_newheader = pep->ntheader32;
> +	headersize = sizeof(old_ntheader.ntheader32);
> +	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)
> +      {
> +	changed = TRUE;
> +	*p_checksum = hdr_checksum;
> +      }
> +  }
>  
>    pe_close (pep);
>  
> -  /* Checksum calculation requires re-open as a regular file. */
> -  if (checksum_show || checksum_update)
> +  /* Checksum re-calculation requires re-open as a regular file. */
> +  if (checksum_option)
>      do_checksum (pathname, indent, changed, hdr_checksum);
>    return 0;
>  }
> @@ -625,16 +650,16 @@ static void
>  do_checksum (const char *pathname, int indent, int changed, ULONG hdr_checksum)
>  {
>    const char name[] = "PE file checksum";
> -  if (checksum_show || (checksum_update && changed) || checksum_update > 1)
> +  if (checksum_option > 1)
>      {
>        int fd;
>        unsigned new_checksum, old_checksum;
> -      if ((fd = open (pathname, (checksum_update ? O_RDWR : O_RDONLY) | O_BINARY)) == -1)
> +      if ((fd = open (pathname, (checksum_option > 2 ? O_RDWR : O_RDONLY) | O_BINARY)) == -1)
>  	{
>  	  fprintf (stderr, "%s: Unable to reopen, errno=%d\n", pathname, errno);
>  	  return;
>  	}
> -      new_checksum = pe32_checksum (fd, checksum_update, &old_checksum);
> +      new_checksum = pe32_checksum (fd, checksum_option > 2, &old_checksum);
>        close(fd);
>        if (!new_checksum)
>  	{
> @@ -650,19 +675,19 @@ do_checksum (const char *pathname, int indent, int changed, ULONG hdr_checksum)
>  	}
>  
>        if (new_checksum == old_checksum)
> -	  printf ("%*s%-24s: 0x%08x (%sverified)\n", indent, "", name,
> -		  old_checksum, (checksum_update ? "unchanged, " : ""));
> -      else if (checksum_update)
> +	  printf ("%*s%-24s: 0x%08x (%schanged, verified)\n", indent, "", name,
> +	          old_checksum, (changed ? "" : "un"));
> +      else if (checksum_option > 2)
>  	  printf ("%*s%-24s: 0x%08x (updated from 0x%08x)\n", indent, "", name,
>  		  new_checksum, old_checksum);
>        else
>  	  printf ("%*s%-24s: 0x%08x (*needs update to 0x%08x*)\n", indent, "", name,
>  		  old_checksum, new_checksum);
>      }
> -  else /* (!checksum_show && checksum_update == 1 && !changed) */
> +  else /* (checksum_option == 1) */
>      {
> -      printf ("%*s%-24s: 0x%08x (unchanged, not verified)\n", indent, "", name,
> -	      hdr_checksum);
> +      printf ("%*s%-24s: 0x%08x (%schanged, not verified)\n", indent, "", name,
> +	      hdr_checksum, (changed ? "" : "un"));
>      }
>  }
>  
> @@ -767,11 +792,7 @@ handle_checksum_option (const char *option_name,
>  {
>    int bool_value;
>    if (!option_arg)
> -    {
> -      checksum_show = 1;
> -      if (handle_any_sizeof == DONT_HANDLE)
> -	handle_any_sizeof = DO_READ;
> -    }
> +    checksum_option = 1;
>    else
>      {
>        if (string_to_bool (option_arg, &bool_value) != 0)
> @@ -781,9 +802,12 @@ handle_checksum_option (const char *option_name,
>            short_usage (stderr);
>            exit (1);
>          }
> -      checksum_update = (bool_value ? 2 : 1);
> -      handle_any_sizeof = DO_WRITE;
> +      checksum_option = (!bool_value ? 2 : 3);
>      }
> +  if (checksum_option > 2)
> +    handle_any_sizeof = DO_WRITE;
> +  else if (handle_any_sizeof == DONT_HANDLE)
> +    handle_any_sizeof = DO_READ;
>  }
>  
>  void
> @@ -1042,6 +1066,11 @@ pe_open (const char *path, BOOL writing)
>    int fd;
>    void *map;
>    
> +  /* Get filesize for checksum update. */
> +  struct stat st;
> +  if (stat (path, &st))
> +    return NULL;
> +
>    fd = open (path, O_BINARY | (writing ? O_RDWR : O_RDONLY));
>    if (fd == -1)
>      return NULL;
> @@ -1053,6 +1082,7 @@ pe_open (const char *path, BOOL writing)
>        return NULL;
>      }
>    pef.pathname = path;
> +  pef.filesize = st.st_size;
>    pef.dosheader = (PIMAGE_DOS_HEADER) map;
>    pef.ntheader32 = (PIMAGE_NT_HEADERS32)
>  		   ((PBYTE) pef.dosheader + pef.dosheader->e_lfanew);
> @@ -1233,9 +1263,12 @@ help (FILE *f)
>  "                              than 2 GB.\n"
>  "  -S, --sepdbg       [BOOL]   Debugging information was removed and stored\n"
>  "                              separately in another file.\n"
> -"  -k, --checksum     [BOOL]   Displays (no argument), updates after changes\n"
> -"                              only (false) or always fixes (true) the file\n"
> -"                              checksum in the PE header.\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"
> +"                              assumption that the original checksum was\n"
> +"                              correct.\n"
>  "  -x, --stack-reserve [NUM]   Reserved stack size of the process in bytes.\n"
>  "  -X, --stack-commit  [NUM]   Initial commited portion of the process stack\n"
>  "                              in bytes.\n"
> @@ -1272,7 +1305,7 @@ help (FILE *f)
>  "For flag values, to set a value, and display the results symbolic, repeat the\n"
>  "option:  --tsaware=true --tsaware -d0 -d\n"
>  "The time of last modification of each file is preserved unless the checksum\n"
> -"has been updated.\n"
> +"has been fixed by option --checksum=true\n"
>  "\n", f);
>  }
>  
> diff --git a/rebase.c b/rebase.c
> index 50cc79f..20a9902 100644
> --- a/rebase.c
> +++ b/rebase.c
> @@ -1190,7 +1190,7 @@ print_overlapped ()
>      }
>  }
>  
> -static BOOL
> +static int
>  update_checksum (const char *pathname)
>  {
>    int fd, err;
> @@ -1200,7 +1200,7 @@ update_checksum (const char *pathname)
>      {
>        fprintf (stderr, "%s: failed to reopen for checksum update: %s\n",
>  	       pathname, strerror (errno));
> -      return FALSE;
> +      return -1;
>      }
>    new_checksum = pe32_checksum (fd, 1, &old_checksum);
>    err = errno;
> @@ -1210,10 +1210,10 @@ update_checksum (const char *pathname)
>        fprintf (stderr, "%s: checksum update failed: %s\n", pathname,
>  	       strerror (err));
>        /* Assume file is unchanged. */
> -      return FALSE;
> +      return -1;
>      }
>  
> -  return (new_checksum != old_checksum);
> +  return (new_checksum != old_checksum ? 1 : 0);
>  }
>  
>  BOOL
> @@ -1222,6 +1222,7 @@ rebase (const char *pathname, ULONG64 *new_image_base, BOOL down_flag)
>    ULONG64 old_image_base, prev_new_image_base;
>    ULONG old_image_size, new_image_size;
>    BOOL status;
> +  const char *checksum_msg;
>  
>    /* Skip if not writable. */
>    if (access (pathname, W_OK) == -1)
> @@ -1231,6 +1232,9 @@ rebase (const char *pathname, ULONG64 *new_image_base, BOOL down_flag)
>        return TRUE;
>      }
>  
> +  /* Try to update the checksum. */
> +  ReBaseUpdateCheckSum = TRUE;
> +
>    /* Calculate next base address, if rebasing down. */
>    if (down_flag)
>      *new_image_base -= offset;
> @@ -1307,8 +1311,17 @@ retry:
>      }
>  #endif
>  
> -  /* Update checksum, if requested. */
> -  status = (checksum_flag ? update_checksum (pathname) : FALSE);
> +
> +  /* Fix checksum, if requested or incremental update failed. */
> +  if (checksum_flag || !ReBaseUpdateCheckSum)
> +    switch (update_checksum (pathname))
> +      {
> +	case 0:  checksum_msg = ", checksum verified"; break;
> +	case 1:  checksum_msg = ", checksum fixed"; break;
> +	default: checksum_msg = ", checksum update failed"; break;
> +      }
> +  else
> +    checksum_msg = "";
>  
>    /* Display rebase results, if verbose. */
>    if (verbose)
> @@ -1316,9 +1329,7 @@ retry:
>        printf ("%s: new base = %" PRIx64 ", new size = %x%s\n",
>  	      pathname,
>  	      (uint64_t) ((down_flag) ? *new_image_base : prev_new_image_base),
> -	      (uint32_t) (new_image_size + offset),
> -	      (checksum_flag ? (status ? ", checksum updated" :
> -	      ", checksum unchanged") : ""));
> +	      (uint32_t) (new_image_size + offset), checksum_msg);
>      }
>  
>    /* Calculate next base address, if rebasing up. */
> @@ -1662,10 +1673,12 @@ Rebase PE files, usually DLLs, to a specified address or address range.\n\
>    One of the options -b, -s or -i is mandatory.  If no rebase database exists\n\
>    yet, -b is required together with -s.\n\
>  \n\
> -  -c, --checksum          Update the file's checksum in the PE header if the\n\
> -                          file has been successfully rebased.  This also bumps\n\
> -                          the file's modification time (like -t) if the\n\
> -                          checksum has been changed.\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\
>    -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
> 


  reply	other threads:[~2023-08-15  8:23 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-08-12 12:26 Christian Franke
2023-08-15  8:23 ` Corinna Vinschen [this message]
2023-08-15  9:29   ` Christian Franke
2023-08-15 10:02     ` Corinna Vinschen

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=ZNs2D7RnVRILLJt0@calimero.vinschen.de \
    --to=corinna-cygwin@cygwin.com \
    --cc=Christian.Franke@t-online.de \
    --cc=cygwin-apps@cygwin.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).