From 902fcfa569bbc1dbed18c64fb4864df7b1078e86 Mon Sep 17 00:00:00 2001 From: "X.organic" Date: Fri, 2 Sep 2022 13:20:31 +0000 Subject: [PATCH 1/2] Miscellaneous memory bug fixes to make AddressSanitizer happy # Conflicts: # src/d_clisrv.c # src/dehacked.c # src/hardware/hw_cache.c # src/hardware/hw_md2.c # src/http-mserv.c # src/lua_hudlib_drawlist.c --- src/d_clisrv.c | 2 +- src/d_net.c | 2 +- src/g_input.c | 3 +++ src/hardware/hw_md2.c | 16 +++++++++---- src/http-mserv.c | 8 ++++--- src/i_net.h | 2 +- src/i_tcp.c | 4 ++-- src/lua_hudlib_drawlist.c | 48 +++++++++++++++++++++++---------------- 8 files changed, 53 insertions(+), 32 deletions(-) diff --git a/src/d_clisrv.c b/src/d_clisrv.c index f8934c2a8..99a5e5fe6 100644 --- a/src/d_clisrv.c +++ b/src/d_clisrv.c @@ -1351,7 +1351,7 @@ static void SendAskInfo(INT32 node) if (node != 0 && node != BROADCASTADDR && cv_rendezvousserver.string[0]) { - I_NetRequestHolePunch(); + I_NetRequestHolePunch(node); } asktime = I_GetTime(); diff --git a/src/d_net.c b/src/d_net.c index 1c81fe3f3..48e97aa31 100644 --- a/src/d_net.c +++ b/src/d_net.c @@ -75,7 +75,7 @@ boolean (*I_NetCanGet)(void) = NULL; void (*I_NetCloseSocket)(void) = NULL; void (*I_NetFreeNodenum)(INT32 nodenum) = NULL; SINT8 (*I_NetMakeNodewPort)(const char *address, const char* port) = NULL; -void (*I_NetRequestHolePunch)(void) = NULL; +void (*I_NetRequestHolePunch)(INT32 node) = NULL; void (*I_NetRegisterHolePunch)(void) = NULL; boolean (*I_NetOpenSocket)(void) = NULL; boolean (*I_Ban) (INT32 node) = NULL; diff --git a/src/g_input.c b/src/g_input.c index ead8cab27..9fbf89bf0 100644 --- a/src/g_input.c +++ b/src/g_input.c @@ -499,6 +499,9 @@ INT32 G_KeyStringtoNum(const char *keystr) { UINT32 j; + if (!keystr[0]) + return 0; + if (!keystr[1] && keystr[0] > ' ' && keystr[0] <= 'z') return keystr[0]; diff --git a/src/hardware/hw_md2.c b/src/hardware/hw_md2.c index 32b7ff7f4..746f094e2 100644 --- a/src/hardware/hw_md2.c +++ b/src/hardware/hw_md2.c @@ -713,11 +713,11 @@ static void HWR_CreateBlendedTexture(patch_t *gpatch, patch_t *blendgpatch, GLMi UINT16 w = gpatch->width, h = gpatch->height; UINT32 size = w*h; RGBA_t *image, *blendimage, *cur, blendcolor; - UINT8 translation[16]; // First the color index - UINT8 cutoff[16]; // Brightness cutoff before using the next color + UINT8 translation[17]; // First the color index + UINT8 cutoff[17]; // Brightness cutoff before using the next color UINT8 translen = 0; UINT8 i; - UINT8 colorbrightnesses[16]; + UINT8 colorbrightnesses[17]; UINT8 color_match_lookup[256]; // optimization attempt blendcolor = V_GetColor(0); // initialize @@ -788,6 +788,11 @@ static void HWR_CreateBlendedTexture(patch_t *gpatch, patch_t *blendgpatch, GLMi translen++; } + if (translen > 0) + translation[translen] = translation[translen-1]; // extended to accomodate secondi if firsti equal to translen-1 + if (translen > 1) + cutoff[translen] = cutoff[translen-1] = 0; // as above + if (skinnum == TC_RAINBOW && translen > 0) { UINT16 b; @@ -803,7 +808,7 @@ static void HWR_CreateBlendedTexture(patch_t *gpatch, patch_t *blendgpatch, GLMi { UINT16 brightdif = 256; - color_match_lookup[i] = 0; + color_match_lookup[b] = 0; for (i = 0; i < translen; i++) { if (b > colorbrightnesses[i]) // don't allow greater matches (because calculating a makeshift gradient for this is already a huge mess as is) @@ -820,6 +825,9 @@ static void HWR_CreateBlendedTexture(patch_t *gpatch, patch_t *blendgpatch, GLMi } } + if (translen > 0) + colorbrightnesses[translen] = colorbrightnesses[translen-1]; + while (size--) { if (skinnum == TC_HITLAG) diff --git a/src/http-mserv.c b/src/http-mserv.c index b5e3ba6c7..d7d65d871 100644 --- a/src/http-mserv.c +++ b/src/http-mserv.c @@ -55,6 +55,8 @@ consvar_t cv_masterserver_token = CVAR_INIT NULL ); +#define HMS_QUERY_VERSION "?v=2.2" + #ifdef MASTERSERVER static int hms_started; @@ -174,7 +176,7 @@ HMS_connect (const char *format, ...) va_start (ap, format); url = malloc(seek + vsnprintf(0, 0, format, ap) + - sizeof "?v=2" - 1 + + sizeof HMS_QUERY_VERSION - 1 + token_length + 1); va_end (ap); @@ -188,8 +190,8 @@ HMS_connect (const char *format, ...) seek += vsprintf(&url[seek], format, ap); va_end (ap); - strcpy(&url[seek], "?v=2"); - seek += sizeof "?v=2" - 1; + strcpy(&url[seek], HMS_QUERY_VERSION); + seek += sizeof HMS_QUERY_VERSION - 1; if (quack_token) sprintf(&url[seek], "&token=%s", quack_token); diff --git a/src/i_net.h b/src/i_net.h index 8caa0edcc..6efa22c69 100644 --- a/src/i_net.h +++ b/src/i_net.h @@ -150,7 +150,7 @@ extern void (*I_NetCloseSocket)(void); /** \brief send a hole punching request */ -extern void (*I_NetRequestHolePunch)(void); +extern void (*I_NetRequestHolePunch)(INT32 node); /** \brief register this machine on the hole punching server */ diff --git a/src/i_tcp.c b/src/i_tcp.c index fef81d05c..55667d252 100644 --- a/src/i_tcp.c +++ b/src/i_tcp.c @@ -1382,9 +1382,9 @@ static void rendezvous(int size) free(addrs); } -static void SOCK_RequestHolePunch(void) +static void SOCK_RequestHolePunch(INT32 node) { - mysockaddr_t * addr = &clientaddress[doomcom->remotenode]; + mysockaddr_t * addr = &clientaddress[node]; holepunchpacket->addr = addr->ip4.sin_addr.s_addr; holepunchpacket->port = addr->ip4.sin_port; diff --git a/src/lua_hudlib_drawlist.c b/src/lua_hudlib_drawlist.c index 9b318ee91..ce5b9d49f 100644 --- a/src/lua_hudlib_drawlist.c +++ b/src/lua_hudlib_drawlist.c @@ -54,7 +54,7 @@ typedef struct drawitem_s { fixed_t sy; INT32 num; INT32 digits; - const char *str; + size_t stroffset; // offset into strbuf to get str UINT16 color; UINT8 strength; INT32 align; @@ -129,6 +129,10 @@ void LUA_HUD_DestroyDrawList(huddrawlist_h list) { Z_Free(list->items); } + if (list->strbuf) + { + Z_Free(list->strbuf); + } Z_Free(list); } @@ -156,7 +160,7 @@ static size_t AllocateDrawItem(huddrawlist_h list) // copy string to list's internal string buffer // lua can deallocate the string before we get to use it, so it's important to // keep our own copy -static const char *CopyString(huddrawlist_h list, const char* str) +static size_t CopyString(huddrawlist_h list, const char* str) { size_t lenstr; @@ -168,10 +172,13 @@ static const char *CopyString(huddrawlist_h list, const char* str) else list->strbuf_capacity *= 2; list->strbuf = (char*) Z_ReallocAlign(list->strbuf, sizeof(char) * list->strbuf_capacity, PU_STATIC, NULL, 8); } - const char *result = (const char *) &list->strbuf[list->strbuf_len]; - strncpy(&list->strbuf[list->strbuf_len], str, lenstr + 1); - list->strbuf_len += lenstr + 1; - return result; + + { + size_t old_len = list->strbuf_len; + strncpy(&list->strbuf[old_len], str, lenstr + 1); + list->strbuf_len += lenstr + 1; + return old_len; + } } void LUA_HUD_AddDraw( @@ -325,7 +332,7 @@ void LUA_HUD_AddDrawString( item->type = DI_DrawString; item->x = x; item->y = y; - item->str = CopyString(list, str); + item->stroffset = CopyString(list, str); item->flags = flags; item->align = align; } @@ -360,7 +367,7 @@ void LUA_HUD_AddDrawTitleCardString( item->x = x; item->y = y; item->flags = flags; - item->str = CopyString(list, str); + item->stroffset = CopyString(list, str); item->bossmode = bossmode; item->timer = timer; item->threshold = threshold; @@ -379,8 +386,8 @@ void LUA_HUD_AddDrawKartString( item->type = DI_DrawKartString; item->x = x; item->y = y; + item->stroffset = CopyString(list, str); item->flags = flags; - item->str = CopyString(list, str); } void LUA_HUD_DrawList(huddrawlist_h list) @@ -394,6 +401,7 @@ void LUA_HUD_DrawList(huddrawlist_h list) for (i = 0; i < list->items_len; i++) { drawitem_t *item = &list->items[i]; + const char *itemstr = &list->strbuf[item->stroffset]; switch (item->type) { @@ -423,33 +431,33 @@ void LUA_HUD_DrawList(huddrawlist_h list) { // hu_font case align_left: - V_DrawString(item->x, item->y, item->flags, item->str); + V_DrawString(item->x, item->y, item->flags, itemstr); break; case align_center: - V_DrawCenteredString(item->x, item->y, item->flags, item->str); + V_DrawCenteredString(item->x, item->y, item->flags, itemstr); break; case align_right: - V_DrawRightAlignedString(item->x, item->y, item->flags, item->str); + V_DrawRightAlignedString(item->x, item->y, item->flags, itemstr); break; // hu_font, 0.5x scale case align_small: - V_DrawSmallString(item->x, item->y, item->flags, item->str); + V_DrawSmallString(item->x, item->y, item->flags, itemstr); break; case align_smallcenter: - V_DrawCenteredSmallString(item->x, item->y, item->flags, item->str); + V_DrawCenteredSmallString(item->x, item->y, item->flags, itemstr); break; case align_smallright: - V_DrawRightAlignedSmallString(item->x, item->y, item->flags, item->str); + V_DrawRightAlignedSmallString(item->x, item->y, item->flags, itemstr); break; // tny_font case align_thin: - V_DrawThinString(item->x, item->y, item->flags, item->str); + V_DrawThinString(item->x, item->y, item->flags, itemstr); break; case align_thincenter: - V_DrawCenteredThinString(item->x, item->y, item->flags, item->str); + V_DrawCenteredThinString(item->x, item->y, item->flags, itemstr); break; case align_thinright: - V_DrawRightAlignedThinString(item->x, item->y, item->flags, item->str); + V_DrawRightAlignedThinString(item->x, item->y, item->flags, itemstr); break; } break; @@ -457,10 +465,10 @@ void LUA_HUD_DrawList(huddrawlist_h list) V_DrawFadeScreen(item->color, item->strength); break; case DI_DrawTitleCardString: - V_DrawTitleCardString(item->x, item->y, item->str, item->flags, item->bossmode, item->timer, item->threshold); + V_DrawTitleCardString(item->x, item->y, itemstr, item->flags, item->bossmode, item->timer, item->threshold); break; case DI_DrawKartString: - V_DrawKartString(item->x, item->y, item->flags, item->str); + V_DrawKartString(item->x, item->y, item->flags, itemstr); break; default: I_Error("can't draw draw list item: invalid draw list item type"); From 6dfd49a3b96161c18c95cf29c6d03ae8f607a592 Mon Sep 17 00:00:00 2001 From: "X.organic" Date: Sat, 3 Sep 2022 02:58:47 +0000 Subject: [PATCH 2/2] Fix use-after-frees around mobjs # Conflicts: # src/p_enemy.c # src/p_mobj.c # src/p_saveg.c # src/p_tick.c --- src/k_kart.c | 2 +- src/p_mobj.c | 3 ++- src/p_saveg.c | 6 +++--- 3 files changed, 6 insertions(+), 5 deletions(-) diff --git a/src/k_kart.c b/src/k_kart.c index 269d45d06..5683baa6f 100644 --- a/src/k_kart.c +++ b/src/k_kart.c @@ -6701,7 +6701,7 @@ static void K_MoveHeldObjects(player_t *player) targz += (player->mo->height/2 - 32*player->mo->scale)*6; } - if (cur->tracer) + if (cur->tracer && !P_MobjWasRemoved(cur->tracer)) { fixed_t diffx, diffy, diffz; diff --git a/src/p_mobj.c b/src/p_mobj.c index c4c9b7962..b541744b3 100644 --- a/src/p_mobj.c +++ b/src/p_mobj.c @@ -10345,7 +10345,7 @@ mobj_t *P_SpawnMobj(fixed_t x, fixed_t y, fixed_t z, mobjtype_t type) mobj_t *side = P_SpawnMobj(mobj->x + FINECOSINE((ang>>ANGLETOFINESHIFT) & FINEMASK), mobj->y + FINESINE((ang>>ANGLETOFINESHIFT) & FINEMASK), mobj->z, MT_DAYTONAPINETREE_SIDE); P_InitAngle(side, ang); - side->target = mobj; + P_SetTarget(&side->target, mobj); side->threshold = i; } break; @@ -10762,6 +10762,7 @@ void P_RemoveSavegameMobj(mobj_t *mobj) // free block P_RemoveThinker((thinker_t *)mobj); + R_RemoveMobjInterpolator(mobj); } static CV_PossibleValue_t respawnitemtime_cons_t[] = {{1, "MIN"}, {300, "MAX"}, {0, NULL}}; diff --git a/src/p_saveg.c b/src/p_saveg.c index 72101fc9c..e657da154 100644 --- a/src/p_saveg.c +++ b/src/p_saveg.c @@ -4193,21 +4193,21 @@ static void P_RelinkPointers(void) { temp = (UINT32)(size_t)mobj->hnext; mobj->hnext = NULL; - if (!(mobj->hnext = P_FindNewPosition(temp))) + if (!P_SetTarget(&mobj->hnext, P_FindNewPosition(temp))) CONS_Debug(DBG_GAMELOGIC, "hnext not found on %d\n", mobj->type); } if (mobj->hprev) { temp = (UINT32)(size_t)mobj->hprev; mobj->hprev = NULL; - if (!(mobj->hprev = P_FindNewPosition(temp))) + if (!P_SetTarget(&mobj->hprev, P_FindNewPosition(temp))) CONS_Debug(DBG_GAMELOGIC, "hprev not found on %d\n", mobj->type); } if (mobj->itnext) { temp = (UINT32)(size_t)mobj->itnext; mobj->itnext = NULL; - if (!(mobj->itnext = P_FindNewPosition(temp))) + if (!P_SetTarget(&mobj->itnext, P_FindNewPosition(temp))) CONS_Debug(DBG_GAMELOGIC, "itnext not found on %d\n", mobj->type); } if (mobj->terrain)