Merge pull request #3740 from citusdata/avoid-freeconn-segfault

GetConnParams: Set runtimeParamStart before setting keywords/values to avoid out of bounds access
pull/3489/head
Philip Dubé 2020-04-10 13:25:32 +00:00 committed by GitHub
commit b054911466
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 21 additions and 13 deletions

View File

@ -261,6 +261,13 @@ GetConnParams(ConnectionHashKey *key, char ***keywords, char ***values,
GetDatabaseEncodingName()
};
/*
* remember where global/GUC params end and runtime ones start, all entries after this
* point should be allocated in context and will be freed upon
* FreeConnParamsHashEntryFields
*/
*runtimeParamStart = ConnParams.size;
/*
* Declare local params for readability;
*
@ -279,7 +286,6 @@ GetConnParams(ConnectionHashKey *key, char ***keywords, char ***values,
/* auth keywords will begin after global and runtime ones are appended */
Index authParamsIdx = ConnParams.size + lengthof(runtimeKeywords);
if (ConnParams.size + lengthof(runtimeKeywords) >= ConnParams.maxSize)
{
/* hopefully this error is only seen by developers */
@ -297,13 +303,6 @@ GetConnParams(ConnectionHashKey *key, char ***keywords, char ***values,
connValues[paramIndex] = ConnParams.values[paramIndex];
}
/*
* remember where global/GUC params end and runtime ones start, all entries after this
* point should be allocated in context and will be freed upon
* FreeConnParamsHashEntryFields
*/
*runtimeParamStart = ConnParams.size;
/* second step: begin after global params and copy runtime params into our context */
for (Index runtimeParamIndex = 0;
runtimeParamIndex < lengthof(runtimeKeywords);

View File

@ -995,6 +995,16 @@ StartConnectionEstablishment(ConnectionHashKey *key)
ConnParamsHashEntry *entry = hash_search(ConnParamsHash, key, HASH_ENTER, &found);
if (!found || !entry->isValid)
{
if (!found)
{
/*
* Zero out entry, but not the key part.
* Avoids leaving invalid pointers in hash table if GetConnParam throws with MemoryContextAllocZero.
*/
memset(((char *) entry) + sizeof(ConnectionHashKey), 0,
sizeof(ConnParamsHashEntry) - sizeof(ConnectionHashKey));
}
/* avoid leaking memory in the keys and values arrays */
if (found && !entry->isValid)
{
@ -1043,9 +1053,6 @@ StartConnectionEstablishment(ConnectionHashKey *key)
static void
FreeConnParamsHashEntryFields(ConnParamsHashEntry *entry)
{
char **keyword = NULL;
char **value = NULL;
/*
* if there was a memory error during the initialization of ConnParamHashEntry in
* GetConnParams the keywords or values might not have been initialized completely.
@ -1058,7 +1065,7 @@ FreeConnParamsHashEntryFields(ConnParamsHashEntry *entry)
if (entry->keywords != NULL)
{
keyword = &entry->keywords[entry->runtimeParamStart];
char **keyword = &entry->keywords[entry->runtimeParamStart];
while (*keyword != NULL)
{
pfree(*keyword);
@ -1070,7 +1077,7 @@ FreeConnParamsHashEntryFields(ConnParamsHashEntry *entry)
if (entry->values != NULL)
{
value = &entry->values[entry->runtimeParamStart];
char **value = &entry->values[entry->runtimeParamStart];
while (*value != NULL)
{
pfree(*value);
@ -1079,6 +1086,8 @@ FreeConnParamsHashEntryFields(ConnParamsHashEntry *entry)
pfree(entry->values);
entry->values = NULL;
}
entry->runtimeParamStart = 0;
}