From e405cdb40b32d9a2d004314cc2699ceac892ee44 Mon Sep 17 00:00:00 2001
From: bones_was_here <bones_was_here@xonotic.au>
Date: Fri, 26 Jan 2024 16:58:50 +1000
Subject: [PATCH] MD3: fix misaligned memory access
The memory subdivision was ordered such that this format likely had no
issues on a 32-bit machine but on 64-bit the alignment requirement of
texture_t increases to 8 bytes, invoking undefined behaviour.
Signed-off-by: bones_was_here <bones_was_here@xonotic.au>
---
model_alias.c | 26 ++++++++++++++++++--------
1 file changed, 18 insertions(+), 8 deletions(-)
diff --git a/model_alias.c b/model_alias.c
index cd235cf0..fc9c9313 100644
--- a/model_alias.c
+++ b/model_alias.c
@@ -1674,21 +1674,31 @@ void Mod_IDP3_Load(model_t *mod, void *buffer, void *bufferend)
loadmodel->submodelsurfaces_end = loadmodel->num_surfaces;
loadmodel->num_textures = loadmodel->num_surfaces * loadmodel->numskins;
loadmodel->num_texturesperskin = loadmodel->num_surfaces;
- data = (unsigned char *)Mem_Alloc(loadmodel->mempool, loadmodel->num_surfaces * sizeof(msurface_t) + loadmodel->num_surfaces * sizeof(int) + loadmodel->num_surfaces * loadmodel->numskins * sizeof(texture_t) + meshtriangles * sizeof(int[3]) + (meshvertices <= 65536 ? meshtriangles * sizeof(unsigned short[3]) : 0) + meshvertices * sizeof(float[2]) + meshvertices * loadmodel->numframes * sizeof(md3vertex_t));
- loadmodel->data_surfaces = (msurface_t *)data;data += loadmodel->num_surfaces * sizeof(msurface_t);
- loadmodel->modelsurfaces_sorted = (int *)data;data += loadmodel->num_surfaces * sizeof(int);
- loadmodel->data_textures = (texture_t *)data;data += loadmodel->num_surfaces * loadmodel->numskins * sizeof(texture_t);
loadmodel->surfmesh.num_vertices = meshvertices;
loadmodel->surfmesh.num_triangles = meshtriangles;
loadmodel->surfmesh.num_morphframes = loadmodel->numframes; // TODO: remove?
loadmodel->num_poses = loadmodel->surfmesh.num_morphframes;
- loadmodel->surfmesh.data_element3i = (int *)data;data += meshtriangles * sizeof(int[3]);
- loadmodel->surfmesh.data_texcoordtexture2f = (float *)data;data += meshvertices * sizeof(float[2]);
- loadmodel->surfmesh.data_morphmd3vertex = (md3vertex_t *)data;data += meshvertices * loadmodel->numframes * sizeof(md3vertex_t);
+
+ // do most allocations as one merged chunk
+ // This is only robust for C standard types!
+ data = (unsigned char *)Mem_Alloc(loadmodel->mempool,
+ meshvertices * sizeof(float[2])
+ + loadmodel->num_surfaces * sizeof(int)
+ + meshtriangles * sizeof(int[3])
+ + (meshvertices <= 65536 ? meshtriangles * sizeof(unsigned short[3]) : 0));
+ // Pointers must be taken in descending order of alignment requirement!
+ loadmodel->surfmesh.data_texcoordtexture2f = (float *)data; data += meshvertices * sizeof(float[2]);
+ loadmodel->modelsurfaces_sorted = (int *)data; data += loadmodel->num_surfaces * sizeof(int);
+ loadmodel->surfmesh.data_element3i = (int *)data; data += meshtriangles * sizeof(int[3]);
if (meshvertices <= 65536)
{
- loadmodel->surfmesh.data_element3s = (unsigned short *)data;data += meshtriangles * sizeof(unsigned short[3]);
+ loadmodel->surfmesh.data_element3s = (unsigned short *)data; data += meshtriangles * sizeof(unsigned short[3]);
}
+ // Struct alignment requirements could change so we can't assume them here
+ // otherwise a safe-looking commit could introduce undefined behaviour!
+ loadmodel->data_surfaces = Mem_AllocType(loadmodel->mempool, msurface_t, loadmodel->num_surfaces * sizeof(msurface_t));
+ loadmodel->data_textures = Mem_AllocType(loadmodel->mempool, texture_t, loadmodel->num_surfaces * loadmodel->numskins * sizeof(texture_t));
+ loadmodel->surfmesh.data_morphmd3vertex = Mem_AllocType(loadmodel->mempool, md3vertex_t, meshvertices * loadmodel->numframes * sizeof(md3vertex_t));
meshvertices = 0;
meshtriangles = 0;
--
2.39.5