The current check for overlapping non-rebaseable DLLs uses a possible outdated base address. It also could not detect multiple overlaps by one non-rebaseable DLL.

Patch size may be misleading due to indentation change.

--
Regards,
Christian

From ee8a966cb6da48b45dfb6de3e732dade81ce7fb9 Mon Sep 17 00:00:00 2001
From: Christian Franke <christian.fra...@t-online.de>
Date: Sat, 16 Jul 2022 17:55:58 +0200
Subject: [PATCH] Ensure that no rebaseable DLL overlaps a non-rebaseable DLL

Restart rebase decision loop with newly sorted list if a DLL is not
rebaseable.  Search for all DLLs which overlap such a DLL.
---
 rebase.c | 203 +++++++++++++++++++++++++++++++++----------------------
 1 file changed, 121 insertions(+), 82 deletions(-)

diff --git a/rebase.c b/rebase.c
index 5cda123..39759a9 100644
--- a/rebase.c
+++ b/rebase.c
@@ -625,6 +625,7 @@ merge_image_info ()
 {
   int i, end;
   img_info_t *match;
+  BOOL sorted;
   ULONG64 floating_image_base;
 
   /* Sort new files from command line by name. */
@@ -725,91 +726,129 @@ merge_image_info ()
       img_info_rebase_start = 0;
     }
 
-  /* Now sort the old part of the list by base address. */
-  if (img_info_rebase_start)
-    qsort (img_info_list, img_info_rebase_start, sizeof (img_info_t),
-          img_info_cmp);
-  /* Perform several tests on the information fetched from the database
-     to match with reality. */
-  for (i = 0; i < img_info_rebase_start; ++i)
+  for (sorted = FALSE; img_info_rebase_start && !sorted; )
     {
-      ULONG64 cur_base, cur_base_orig;
-      ULONG cur_size, slot_size;
-
-      /* Files with the needs_rebasing or cannot_rebase flags set have been
-        checked already. */
-      if (img_info_list[i].flag.needs_rebasing
-         || img_info_list[i].flag.cannot_rebase)
-       continue;
-      /* Check if the files in the old list still exist.  Drop non-existant
-        or unaccessible files. */
-      if (access (img_info_list[i].name, F_OK) == -1
-         || !GetImageInfos64 (img_info_list[i].name, NULL,
-                              &cur_base, &cur_size))
-       {
-         free (img_info_list[i].name);
-         memmove (img_info_list + i, img_info_list + i + 1,
-                  (img_info_size - i - 1) * sizeof (img_info_t));
-         --img_info_rebase_start;
-         --img_info_size;
-         continue;
-       }
-      slot_size = roundup2 (cur_size, ALLOCATION_SLOT);
-      cur_base_orig = cur_base;
-      /* If the file has been reinstalled, try to rebase to the same address
-        in the first place. */
-      if (cur_base != img_info_list[i].base)
+      char overlaps[img_info_rebase_start];
+      memset(overlaps, 0, img_info_rebase_start);
+      /* Now sort the old part of the list by base address. */
+      qsort (img_info_list, img_info_rebase_start, sizeof (img_info_t),
+            img_info_cmp);
+      /* Perform several tests on the information fetched from the database
+        to match with reality. */
+      for (sorted = TRUE, i = 0; sorted && i < img_info_rebase_start; ++i)
        {
-         img_info_list[i].flag.needs_rebasing = 1;
-         if (verbose)
-           fprintf (stderr, "rebasing %s because it's base has changed (due to 
being reinstalled?)\n", img_info_list[i].name);
-         /* Set cur_base to the old base to simplify subsequent tests. */
-         cur_base = img_info_list[i].base;
-       }
-      /* However, if the DLL got bigger and doesn't fit into its slot
-        anymore, rebase this DLL from scratch. */
-      if (i + 1 < img_info_rebase_start
-         && cur_base + slot_size + offset > img_info_list[i + 1].base)
-       {
-         img_info_list[i].base = 0;
-         if (verbose)
-           fprintf (stderr, "rebasing %s because it won't fit in it's old slot 
without overlapping next DLL\n", img_info_list[i].name);
-       }
-      /* Does the previous DLL reach into the address space of this
-        DLL?  This happens if the previous DLL is not rebaseable. */
-      else if (i > 0 && cur_base < img_info_list[i - 1].base
-                                  + img_info_list[i - 1].slot_size)
-       {
-         img_info_list[i].base = 0;
-         if (verbose)
-           fprintf (stderr, "rebasing %s because previous DLL now overlaps\n", 
img_info_list[i].name);
-       }
-      /* Does the file match the base address requirements?  If not,
-        rebase from scratch. */
-      else if ((down_flag && cur_base + slot_size + offset > image_base)
-              || (!down_flag && cur_base < image_base))
-       {
-         img_info_list[i].base = 0;
-         if (verbose)
-           fprintf (stderr, "rebasing %s because it's base address is outside 
the expected area\n", img_info_list[i].name);
-       }
-      /* Make sure all DLLs with base address 0 have the needs_rebasing
-        flag set. */
-      if (img_info_list[i].base == 0)
-       img_info_list[i].flag.needs_rebasing = 1;
-      /* Only check for writability if file needs rebasing to keep
-        Compact OS compression of unchanged files.  Revert rebase
-        decision if not writeable. */
-      if (img_info_list[i].flag.needs_rebasing && set_cannot_rebase 
(&img_info_list[i]))
-       {
-         img_info_list[i].flag.needs_rebasing = 0;
-         img_info_list[i].base = cur_base_orig;
-         if (verbose)
-           fprintf (stderr, "rebasing %s deferred because file is not 
writable\n", img_info_list[i].name);
+         ULONG64 cur_base, cur_base_orig;
+         ULONG cur_size, slot_size;
+
+         /* If file is not rebaseable and neither --oblivious nor --merge-files
+            is active, search forward for possible overlaps. */
+         if (img_info_list[i].flag.cannot_rebase == 1)
+           {
+             int j;
+             /* If cannot_rebase is set, base and size are already valid. */
+             cur_base = img_info_list[i].base;
+             slot_size = img_info_list[i].slot_size;
+             for (j = i + 1; j < img_info_rebase_start; ++j)
+               {
+                 if (img_info_list[j].flag.cannot_rebase)
+                   continue; /* Also not rebaseable. */
+                 if (img_info_list[j].base == 0)
+                   continue; /* Will already be rebased from scratch. */
+                 if (cur_base + slot_size + offset <= img_info_list[j].base)
+                   break; /* This and any further DLLs do not overlap. */
+                 /* Select rebase from scratch when outer loop arrives at j. */
+                 overlaps[j] = 1;
+               }
+           }
+
+         /* Files with the needs_rebasing or cannot_rebase flags set have been
+            checked already. */
+         if (img_info_list[i].flag.needs_rebasing
+             || img_info_list[i].flag.cannot_rebase)
+           continue;
+         /* Check if the files in the old list still exist.  Drop non-existant
+            or unaccessible files. */
+         if (access (img_info_list[i].name, F_OK) == -1
+             || !GetImageInfos64 (img_info_list[i].name, NULL,
+                                  &cur_base, &cur_size))
+           {
+             free (img_info_list[i].name);
+             memmove (img_info_list + i, img_info_list + i + 1,
+                      (img_info_size - i - 1) * sizeof (img_info_t));
+             memmove (overlaps + i, overlaps + i + 1, img_info_size - i - 1);
+             --img_info_rebase_start;
+             --img_info_size;
+             continue;
+           }
+         slot_size = roundup2 (cur_size, ALLOCATION_SLOT);
+         cur_base_orig = cur_base;
+         /* If the file has been reinstalled, try to rebase to the same address
+            in the first place. */
+         if (cur_base != img_info_list[i].base)
+           {
+             img_info_list[i].flag.needs_rebasing = 1;
+             if (verbose)
+               fprintf (stderr, "rebasing %s because it's base has changed "
+                        "(due to being reinstalled?)\n", 
img_info_list[i].name);
+             /* Set cur_base to the old base to simplify subsequent tests. */
+             cur_base = img_info_list[i].base;
+           }
+         /* However, if the DLL got bigger and doesn't fit into its slot
+            anymore, rebase this DLL from scratch. */
+         if (i + 1 < img_info_rebase_start
+             && cur_base + slot_size + offset > img_info_list[i + 1].base)
+           {
+             img_info_list[i].base = 0;
+             if (verbose)
+               fprintf (stderr, "rebasing %s because it won't fit in it's old 
slot "
+                        "without overlapping next DLL\n", 
img_info_list[i].name);
+           }
+         /* Does a previous DLL reach into the address space of this
+            DLL?  This happens if the previous DLL is not rebaseable. */
+         else if (overlaps[i])
+           {
+             img_info_list[i].base = 0;
+             if (verbose)
+               fprintf (stderr, "rebasing %s because a previous non-writable 
DLL "
+                        "overlaps\n", img_info_list[i].name);
+           }
+         /* Does the file match the base address requirements?  If not,
+            rebase from scratch. */
+         else if ((down_flag && cur_base + slot_size + offset > image_base)
+                  || (!down_flag && cur_base < image_base))
+           {
+             img_info_list[i].base = 0;
+             if (verbose)
+               fprintf (stderr, "rebasing %s because it's base address is 
outside the "
+                        "expected area\n", img_info_list[i].name);
+           }
+         /* Make sure all DLLs with base address 0 have the needs_rebasing
+            flag set. */
+         if (img_info_list[i].base == 0)
+           img_info_list[i].flag.needs_rebasing = 1;
+         /* Only check for writability if file needs rebasing to keep
+            Compact OS compression of unchanged files.  Revert rebase
+            decision if not writeable. */
+         if (img_info_list[i].flag.needs_rebasing
+             && set_cannot_rebase (&img_info_list[i]))
+           {
+             img_info_list[i].flag.needs_rebasing = 0;
+             img_info_list[i].base = cur_base_orig;
+             if (verbose)
+               fprintf (stderr, "rebasing %s deferred because file is not 
writable\n",
+                        img_info_list[i].name);
+             /* List is possibly no longer sorted which could result in false
+                negative rebase decisions.  Exit loop and restart it with
+                newly sorted list.  This will also set the overlaps[] array
+                for this DLL when the restarted loop visits the (possibly new)
+                index of this DLL.  Infinite retries could not occur because
+                each turn sets another cannot_rebase flag. */
+             sorted = FALSE;
+           }
+         /* Unconditionally overwrite old with new size. */
+         img_info_list[i].size = cur_size;
+         img_info_list[i].slot_size = slot_size;
        }
-      /* Unconditionally overwrite old with new size. */
-      img_info_list[i].size = cur_size;
-      img_info_list[i].slot_size = slot_size;
     }
   /* The remainder of the function expects img_info_size to be > 0. */
   if (img_info_size == 0)
-- 
2.37.1

Reply via email to