[util] Plug a thread-safety hole in plists

This commit is contained in:
Bill Currie 2021-02-09 09:57:07 +09:00
parent 84dc73da2c
commit dfa7af03c6
11 changed files with 41 additions and 28 deletions

View file

@ -298,13 +298,15 @@ void ED_PrintNum (progs_t *pr, pr_int_t ent);
// pr_parse.c // pr_parse.c
struct script_s; struct script_s;
struct plitem_s; struct plitem_s;
struct hashlink_s;
qboolean ED_ParseEpair (progs_t *pr, pr_type_t *base, pr_def_t *key, qboolean ED_ParseEpair (progs_t *pr, pr_type_t *base, pr_def_t *key,
const char *s); const char *s);
struct plitem_s *ED_EntityDict (progs_t *pr, edict_t *ed); struct plitem_s *ED_EntityDict (progs_t *pr, edict_t *ed);
struct plitem_s *ED_GlobalsDict (progs_t *pr); struct plitem_s *ED_GlobalsDict (progs_t *pr);
void ED_InitGlobals (progs_t *pr, struct plitem_s *globals); void ED_InitGlobals (progs_t *pr, struct plitem_s *globals);
void ED_InitEntity (progs_t *pr, struct plitem_s *entity, edict_t *ent); void ED_InitEntity (progs_t *pr, struct plitem_s *entity, edict_t *ent);
struct plitem_s *ED_ConvertToPlist (struct script_s *script, int nohack); struct plitem_s *ED_ConvertToPlist (struct script_s *script, int nohack,
struct hashlink_s **hashlinks);
struct plitem_s *ED_Parse (progs_t *pr, const char *data); struct plitem_s *ED_Parse (progs_t *pr, const char *data);
void ED_LoadFromFile (progs_t *pr, const char *data); void ED_LoadFromFile (progs_t *pr, const char *data);
void ED_EntityParseFunction (progs_t *pr); void ED_EntityParseFunction (progs_t *pr);

View file

@ -28,6 +28,8 @@
#ifndef __QF_qfplist_h #ifndef __QF_qfplist_h
#define __QF_qfplist_h #define __QF_qfplist_h
struct hashlink_s;
/** \defgroup qfplist Property lists /** \defgroup qfplist Property lists
\ingroup utils \ingroup utils
*/ */
@ -115,11 +117,14 @@ typedef struct plelement_s {
/** Create an in-memory representation of the contents of a property list. /** Create an in-memory representation of the contents of a property list.
\param string the saved plist, as read from a file. \param string the saved plist, as read from a file.
\param hashlinks Hashlink chain to use when creating dictionaries (see
Hash_NewTable()). May be null.
\return Returns an object equivalent to the passed-in string. \return Returns an object equivalent to the passed-in string.
\note You are responsible for freeing the returned object. \note You are responsible for freeing the returned object.
*/ */
plitem_t *PL_GetPropertyList (const char *string); plitem_t *PL_GetPropertyList (const char *string,
struct hashlink_s **hashlinks);
/** Create a property list string from the in-memory representation. /** Create a property list string from the in-memory representation.
@ -275,9 +280,11 @@ plitem_t *PL_RemoveObjectAtIndex (plitem_t *array, int index);
/** Create a new dictionary object. /** Create a new dictionary object.
The dictionary will be empty. The dictionary will be empty.
\param hashlinks Hashlink chain to use when creating dictionaries (see
Hash_NewTable()). May be null.
\return the new dictionary object \return the new dictionary object
*/ */
plitem_t *PL_NewDictionary (void); plitem_t *PL_NewDictionary (struct hashlink_s **hashlinks);
/** Create a new array object. /** Create a new array object.
The array will be empty. The array will be empty.

View file

@ -172,7 +172,7 @@ Load_Tracklist (void)
buffile = calloc (size+10, sizeof (char)); buffile = calloc (size+10, sizeof (char));
Qread (oggfile, buffile, size); Qread (oggfile, buffile, size);
tracklist = PL_GetPropertyList (buffile); tracklist = PL_GetPropertyList (buffile, 0);
if (!tracklist || PL_Type (tracklist) != QFDictionary) { if (!tracklist || PL_Type (tracklist) != QFDictionary) {
Sys_Printf ("Malformed or empty tracklist file. check mus_ogglist\n"); Sys_Printf ("Malformed or empty tracklist file. check mus_ogglist\n");
return -1; return -1;

View file

@ -109,7 +109,7 @@ VISIBLE plitem_t *
ED_EntityDict (progs_t *pr, edict_t *ed) ED_EntityDict (progs_t *pr, edict_t *ed)
{ {
dstring_t *dstr = dstring_newstr (); dstring_t *dstr = dstring_newstr ();
plitem_t *entity = PL_NewDictionary (); plitem_t *entity = PL_NewDictionary (pr->hashlink_freelist);
pr_uint_t i; pr_uint_t i;
int j; int j;
int type; int type;
@ -155,7 +155,7 @@ VISIBLE plitem_t *
ED_GlobalsDict (progs_t *pr) ED_GlobalsDict (progs_t *pr)
{ {
dstring_t *dstr = dstring_newstr (); dstring_t *dstr = dstring_newstr ();
plitem_t *globals = PL_NewDictionary (); plitem_t *globals = PL_NewDictionary (pr->hashlink_freelist);
pr_uint_t i; pr_uint_t i;
const char *name; const char *name;
const char *value; const char *value;
@ -290,7 +290,7 @@ ED_ParseEpair (progs_t *pr, pr_type_t *base, pr_def_t *key, const char *s)
*/ */
VISIBLE plitem_t * VISIBLE plitem_t *
ED_ConvertToPlist (script_t *script, int nohack) ED_ConvertToPlist (script_t *script, int nohack, struct hashlink_s **hashlinks)
{ {
dstring_t *dstr = dstring_newstr (); dstring_t *dstr = dstring_newstr ();
plitem_t *plist = PL_NewArray (); plitem_t *plist = PL_NewArray ();
@ -304,7 +304,7 @@ ED_ConvertToPlist (script_t *script, int nohack)
token = script->token->str; token = script->token->str;
if (!strequal (token, "{")) if (!strequal (token, "{"))
Sys_Error ("ED_ConvertToPlist: EOF without closing brace"); Sys_Error ("ED_ConvertToPlist: EOF without closing brace");
ent = PL_NewDictionary (); ent = PL_NewDictionary (hashlinks);
while (1) { while (1) {
int n; int n;
@ -499,11 +499,11 @@ ED_Parse (progs_t *pr, const char *data)
if (Script_GetToken (script, 1)) { if (Script_GetToken (script, 1)) {
if (strequal (script->token->str, "(")) { if (strequal (script->token->str, "(")) {
// new style (plist) entity data // new style (plist) entity data
entity_list = PL_GetPropertyList (data); entity_list = PL_GetPropertyList (data, pr->hashlink_freelist);
} else { } else {
// oldstyle entity data // oldstyle entity data
Script_UngetToken (script); Script_UngetToken (script);
entity_list = ED_ConvertToPlist (script, 0); entity_list = ED_ConvertToPlist (script, 0, pr->hashlink_freelist);
} }
} }
Script_Delete (script); Script_Delete (script);

View file

@ -195,7 +195,7 @@ bi_PL_GetFromFile (progs_t *pr)
Qread (file, buf, len); Qread (file, buf, len);
buf[len] = 0; buf[len] = 0;
plitem = PL_GetPropertyList (buf); plitem = PL_GetPropertyList (buf, pr->hashlink_freelist);
R_INT (pr) = plist_retain (res, plitem); R_INT (pr) = plist_retain (res, plitem);
} }
@ -204,7 +204,8 @@ static void
bi_PL_GetPropertyList (progs_t *pr) bi_PL_GetPropertyList (progs_t *pr)
{ {
plist_resources_t *res = PR_Resources_Find (pr, "plist"); plist_resources_t *res = PR_Resources_Find (pr, "plist");
plitem_t *plitem = PL_GetPropertyList (P_GSTRING (pr, 0)); plitem_t *plitem = PL_GetPropertyList (P_GSTRING (pr, 0),
pr->hashlink_freelist);
R_INT (pr) = plist_retain (res, plitem); R_INT (pr) = plist_retain (res, plitem);
} }
@ -373,7 +374,7 @@ static void
bi_PL_NewDictionary (progs_t *pr) bi_PL_NewDictionary (progs_t *pr)
{ {
plist_resources_t *res = PR_Resources_Find (pr, "plist"); plist_resources_t *res = PR_Resources_Find (pr, "plist");
plitem_t *plitem = PL_NewDictionary (); plitem_t *plitem = PL_NewDictionary (pr->hashlink_freelist);
R_INT (pr) = plist_retain (res, plitem); R_INT (pr) = plist_retain (res, plitem);
} }

View file

@ -91,6 +91,7 @@ typedef struct pldata_s { // Unparsed property list string
unsigned line; unsigned line;
plitem_t *error; plitem_t *error;
va_ctx_t *va_ctx; va_ctx_t *va_ctx;
hashlink_t **hashlinks;
} pldata_t; } pldata_t;
// Ugly defines for fast checking and conversion from char to number // Ugly defines for fast checking and conversion from char to number
@ -157,11 +158,11 @@ PL_NewItem (pltype_t type)
} }
VISIBLE plitem_t * VISIBLE plitem_t *
PL_NewDictionary (void) PL_NewDictionary (hashlink_t **hashlinks)
{ {
plitem_t *item = PL_NewItem (QFDictionary); plitem_t *item = PL_NewItem (QFDictionary);
//FIXME need a per-thread hashlink freelist for plist to be thread-safe hashtab_t *dict = Hash_NewTable (1021, dict_get_key, dict_free, NULL,
hashtab_t *dict = Hash_NewTable (1021, dict_get_key, dict_free, NULL, 0); hashlinks);
item->data = dict; item->data = dict;
return item; return item;
} }
@ -722,7 +723,7 @@ PL_ParsePropertyListItem (pldata_t *pl)
switch (pl->ptr[pl->pos]) { switch (pl->ptr[pl->pos]) {
case '{': case '{':
{ {
item = PL_NewDictionary (); item = PL_NewDictionary (pl->hashlinks);
item->line = pl->line; item->line = pl->line;
pl->pos++; pl->pos++;
@ -880,7 +881,7 @@ PL_ParsePropertyListItem (pldata_t *pl)
} }
VISIBLE plitem_t * VISIBLE plitem_t *
PL_GetPropertyList (const char *string) PL_GetPropertyList (const char *string, hashlink_t **hashlinks)
{ {
pldata_t *pl = calloc (1, sizeof (pldata_t)); pldata_t *pl = calloc (1, sizeof (pldata_t));
plitem_t *newpl = NULL; plitem_t *newpl = NULL;
@ -894,6 +895,7 @@ PL_GetPropertyList (const char *string)
pl->error = NULL; pl->error = NULL;
pl->line = 1; pl->line = 1;
pl->va_ctx = va_create_context (4); pl->va_ctx = va_create_context (4);
pl->hashlinks = hashlinks;
if ((newpl = PL_ParsePropertyListItem (pl))) { if ((newpl = PL_ParsePropertyListItem (pl))) {
va_destroy_context (pl->va_ctx); va_destroy_context (pl->va_ctx);

View file

@ -672,7 +672,7 @@ qfs_load_config (void)
buf[len + 2] = 0; buf[len + 2] = 0;
if (qfs_gd_plist) if (qfs_gd_plist)
PL_Free (qfs_gd_plist); PL_Free (qfs_gd_plist);
qfs_gd_plist = PL_GetPropertyList (buf); qfs_gd_plist = PL_GetPropertyList (buf, 0);
free (buf); free (buf);
if (qfs_gd_plist && PL_Type (qfs_gd_plist) == QFDictionary) if (qfs_gd_plist && PL_Type (qfs_gd_plist) == QFDictionary)
return; // done return; // done
@ -680,7 +680,7 @@ qfs_load_config (void)
no_config: no_config:
if (qfs_gd_plist) if (qfs_gd_plist)
PL_Free (qfs_gd_plist); PL_Free (qfs_gd_plist);
qfs_gd_plist = PL_GetPropertyList (qfs_default_dirconf); qfs_gd_plist = PL_GetPropertyList (qfs_default_dirconf, 0);
} }
/* /*

View file

@ -244,7 +244,8 @@ static plitem_t *
qfv_load_pipeline (vulkan_ctx_t *ctx, const char *name) qfv_load_pipeline (vulkan_ctx_t *ctx, const char *name)
{ {
if (!ctx->pipelineDef) { if (!ctx->pipelineDef) {
ctx->pipelineDef = PL_GetPropertyList (quakeforge_pipeline); ctx->pipelineDef = PL_GetPropertyList (quakeforge_pipeline,
&ctx->hashlinks);
} }
plitem_t *item = ctx->pipelineDef; plitem_t *item = ctx->pipelineDef;

View file

@ -434,7 +434,7 @@ entities_array (void)
static plitem_t * static plitem_t *
game_dict (void) game_dict (void)
{ {
plitem_t *game = PL_NewDictionary (); plitem_t *game = PL_NewDictionary (0);
PL_D_AddObject (game, "comment", PL_D_AddObject (game, "comment",
PL_NewString (va (0, "%-21s kills:%3i/%3i", cl.levelname, PL_NewString (va (0, "%-21s kills:%3i/%3i", cl.levelname,
@ -455,7 +455,7 @@ game_dict (void)
static plitem_t * static plitem_t *
convert_to_game_dict (script_t *script) convert_to_game_dict (script_t *script)
{ {
plitem_t *game = PL_NewDictionary (); plitem_t *game = PL_NewDictionary (0);
plitem_t *item; plitem_t *item;
plitem_t *list; plitem_t *list;
int skill; int skill;
@ -500,7 +500,7 @@ convert_to_game_dict (script_t *script)
PL_D_AddObject (game, "lightstyles", item); PL_D_AddObject (game, "lightstyles", item);
// load the edicts out of the savegame file // load the edicts out of the savegame file
list = ED_ConvertToPlist (script, 0); list = ED_ConvertToPlist (script, 0, 0);
item = PL_RemoveObjectAtIndex (list, 0); item = PL_RemoveObjectAtIndex (list, 0);
PL_D_AddObject (game, "globals", item); PL_D_AddObject (game, "globals", item);
PL_D_AddObject (game, "entities", list); PL_D_AddObject (game, "entities", list);
@ -647,7 +647,7 @@ Host_Loadgame_f (void)
Sys_Printf ("Unexpected EOF reading %s\n", name->str); Sys_Printf ("Unexpected EOF reading %s\n", name->str);
goto end; goto end;
} }
game = PL_GetPropertyList (script->p); game = PL_GetPropertyList (script->p, 0);
} else { } else {
sscanf (script->token->str, "%i", &version); sscanf (script->token->str, "%i", &version);
if (version != SAVEGAME_VERSION) { if (version != SAVEGAME_VERSION) {

View file

@ -179,7 +179,7 @@ LoadEntities (void)
script = Script_New (); script = Script_New ();
Script_Start (script, "ent data", bsp->entdata); Script_Start (script, "ent data", bsp->entdata);
entity_list = ED_ConvertToPlist (script, 1); entity_list = ED_ConvertToPlist (script, 1, 0);
Script_Delete (script); Script_Delete (script);
// start parsing // start parsing
@ -208,7 +208,7 @@ LoadEntities (void)
} }
entity->dict = PL_ObjectAtIndex (entity_list, i); entity->dict = PL_ObjectAtIndex (entity_list, i);
dict = PL_NewDictionary (); dict = PL_NewDictionary (0);
// go through all the keys in this entity // go through all the keys in this entity
keys = PL_D_AllKeys (entity->dict); keys = PL_D_AllKeys (entity->dict);

View file

@ -334,6 +334,6 @@ LoadProperties (const char *filename)
Qread (f, buf, len); Qread (f, buf, len);
Qclose (f); Qclose (f);
buf[len] = 0; buf[len] = 0;
properties = PL_GetPropertyList (buf); properties = PL_GetPropertyList (buf, 0);
free (buf); free (buf);
} }