From d5b6bb70919f91f94b035b12ff706ef9ab08689c Mon Sep 17 00:00:00 2001
From: Simon Howard
Date: Thu, 16 Oct 2014 23:25:52 +0000
Subject: Fix crash caused by adding a new WAD file.

This fixes #442, a crash caused by adding a new WAD file after a
lump has been loaded (and cached) from a previous WAD. This
manifested when playing using the Freedoom IWADs and also loading
a PWAD at the same time. The Freedoom IWADs have DEHACKED lumps
that are loaded from within the IWAD file.

The ultimate cause (thanks to Fabian Greffrath for uncovering it)
is that lumpinfo is realloc()ed after each new WAD load to store
the lumpinfo_t structures from the new WAD. If a lump has been read
and cached from a previous WAD file, it may end up having an invalid
'user' pointer that points to somewhere in the old lumpinfo[] array,
not the new one.

I think this bug was masked because realloc() will often not move
data if the previous location can simply be extended. The bug was
discovered when loading BTSX as a PWAD, probably because it's a
large WAD that contains a lot of lumps, and forced a move during
realloc.
---
 src/w_wad.c    | 67 +++++++++++++++++++++++++++++++++++++++++++++-------------
 src/z_native.c | 15 +++++++++++++
 src/z_zone.c   | 15 +++++++++++++
 src/z_zone.h   |  1 +
 4 files changed, 83 insertions(+), 15 deletions(-)

diff --git a/src/w_wad.c b/src/w_wad.c
index 1d8142c8..aa0646af 100644
--- a/src/w_wad.c
+++ b/src/w_wad.c
@@ -83,6 +83,46 @@ unsigned int W_LumpNameHash(const char *s)
     return result;
 }
 
+// Increase the size of the lumpinfo[] array to the specified size.
+static void ExtendLumpInfo(int newnumlumps)
+{
+    lumpinfo_t *newlumpinfo;
+    unsigned int i;
+
+    newlumpinfo = calloc(newnumlumps, sizeof(lumpinfo_t));
+
+    if (newlumpinfo == NULL)
+    {
+	I_Error ("Couldn't realloc lumpinfo");
+    }
+
+    // Copy over lumpinfo_t structures from the old array. If any of
+    // these lumps have been cached, we need to update the user
+    // pointers to the new location.
+    for (i = 0; i < numlumps && i < newnumlumps; ++i)
+    {
+        memcpy(&newlumpinfo[i], &lumpinfo[i], sizeof(lumpinfo_t));
+
+        if (newlumpinfo[i].cache != NULL)
+        {
+            Z_ChangeUser(newlumpinfo[i].cache, &newlumpinfo[i].cache);
+        }
+
+        // We shouldn't be generating a hash table until after all WADs have
+        // been loaded, but just in case...
+        if (lumpinfo[i].next != NULL)
+        {
+            int nextlumpnum = lumpinfo[i].next - lumpinfo;
+            newlumpinfo[i].next = &newlumpinfo[nextlumpnum];
+        }
+    }
+
+    // All done.
+    free(lumpinfo);
+    lumpinfo = newlumpinfo;
+    numlumps = newnumlumps;
+}
+
 //
 // LUMP BASED ROUTINES.
 //
@@ -106,19 +146,20 @@ wad_file_t *W_AddFile (char *filename)
     int startlump;
     filelump_t *fileinfo;
     filelump_t *filerover;
-    
+    int newnumlumps;
+
     // open the file and add to directory
 
     wad_file = W_OpenFile(filename);
-		
+
     if (wad_file == NULL)
     {
 	printf (" couldn't open %s\n", filename);
 	return NULL;
     }
 
-    startlump = numlumps;
-	
+    newnumlumps = numlumps;
+
     if (strcasecmp(filename+strlen(filename)-3 , "wad" ) )
     {
 	// single lump file
@@ -136,7 +177,7 @@ wad_file_t *W_AddFile (char *filename)
         // extension).
 
 	M_ExtractFileBase (filename, fileinfo->name);
-	numlumps++;
+	newnumlumps++;
     }
     else 
     {
@@ -161,19 +202,15 @@ wad_file_t *W_AddFile (char *filename)
 	fileinfo = Z_Malloc(length, PU_STATIC, 0);
 
         W_Read(wad_file, header.infotableofs, fileinfo, length);
-	numlumps += header.numlumps;
+	newnumlumps += header.numlumps;
     }
 
-    // Fill in lumpinfo
-    lumpinfo = realloc(lumpinfo, numlumps * sizeof(lumpinfo_t));
-
-    if (lumpinfo == NULL)
-    {
-	I_Error ("Couldn't realloc lumpinfo");
-    }
+    // Increase size of numlumps array to accomodate the new file.
+    startlump = numlumps;
+    ExtendLumpInfo(newnumlumps);
 
     lump_p = &lumpinfo[startlump];
-	
+
     filerover = fileinfo;
 
     for (i=startlump; i<numlumps; ++i)
@@ -187,7 +224,7 @@ wad_file_t *W_AddFile (char *filename)
         ++lump_p;
         ++filerover;
     }
-	
+
     Z_Free(fileinfo);
 
     if (lumphash != NULL)
diff --git a/src/z_native.c b/src/z_native.c
index c56c5b3a..26e12a06 100644
--- a/src/z_native.c
+++ b/src/z_native.c
@@ -464,6 +464,21 @@ void Z_ChangeTag2(void *ptr, int tag, char *file, int line)
     Z_InsertBlock(block);
 }
 
+void Z_ChangeUser(void *ptr, void **user)
+{
+    memblock_t*	block;
+
+    block = (memblock_t *) ((byte *)ptr - sizeof(memblock_t));
+
+    if (block->id != ZONEID)
+    {
+        I_Error("Z_ChangeUser: Tried to change user for invalid block!");
+    }
+
+    block->user = user;
+    *user = ptr;
+}
+
 
 //
 // Z_FreeMemory
diff --git a/src/z_zone.c b/src/z_zone.c
index c9dd8315..16da22b9 100644
--- a/src/z_zone.c
+++ b/src/z_zone.c
@@ -443,6 +443,21 @@ void Z_ChangeTag2(void *ptr, int tag, char *file, int line)
     block->tag = tag;
 }
 
+void Z_ChangeUser(void *ptr, void **user)
+{
+    memblock_t*	block;
+
+    block = (memblock_t *) ((byte *)ptr - sizeof(memblock_t));
+
+    if (block->id != ZONEID)
+    {
+        I_Error("Z_ChangeUser: Tried to change user for invalid block!");
+    }
+
+    block->user = user;
+    *user = ptr;
+}
+
 
 
 //
diff --git a/src/z_zone.h b/src/z_zone.h
index b15fb168..526f30d3 100644
--- a/src/z_zone.h
+++ b/src/z_zone.h
@@ -58,6 +58,7 @@ void    Z_DumpHeap (int lowtag, int hightag);
 void    Z_FileDumpHeap (FILE *f);
 void    Z_CheckHeap (void);
 void    Z_ChangeTag2 (void *ptr, int tag, char *file, int line);
+void    Z_ChangeUser(void *ptr, void **user);
 int     Z_FreeMemory (void);
 unsigned int Z_ZoneSize(void);
 
-- 
cgit v1.2.3