Skip to content

Commit

Permalink
Another batch of memory management improvement (#1925)
Browse files Browse the repository at this point in the history 
Some codepaths access arrays out of bounds or reference
uninitialize variables.

do_compile()
prepare_format_string()
pre_transform_target_entry()
bbf_ProcessUtility()
babelfish_add_domain_mapping_entry_internal()
object_name()
TdsSendTypeBinary()
modify_insert_stmt()

Tasks: BABEL-4446, BABEL-4457, BABEL-4456, BABEL-4471, BABEL-4472, BABEL-4483

Signed-off-by: Kristian Lejao <[email protected]>
  • Loading branch information
@lejaokri
lejaokri authored Oct 18, 2023
1 parent b9ceb87 commit ec6a8e5
Show file tree
Hide file tree
Showing 6 changed files with 66 additions and 41 deletions.
38 changes: 26 additions & 12 deletions contrib/babelfishpg_tds/src/backend/tds/tdstypeio.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2605,10 +2605,13 @@ TdsSendTypeBinary(FmgrInfo *finfo, Datum value, void *vMetaData)
maxLen = 0;
bytea *vlena = DatumGetByteaPCopy(value);
bytea *buf;
int copyLen = 0;
TdsColumnMetaData *col = (TdsColumnMetaData *) vMetaData;

maxLen = col->metaEntry.type7.maxSize;
buf = (bytea *) palloc0(sizeof(bytea) * maxLen);
copyLen = Max((sizeof(bytea) * maxLen), VARSIZE_ANY_EXHDR(vlena));

buf = (bytea *) palloc0(copyLen);
memcpy(buf, VARDATA_ANY(vlena), VARSIZE_ANY_EXHDR(vlena));

if ((rc = TdsPutUInt16LE(maxLen)) == 0)
Expand Down Expand Up @@ -3044,6 +3047,7 @@ TdsSendTypeNumeric(FmgrInfo *finfo, Datum value, void *vMetaData)
TdsColumnMetaData *col = (TdsColumnMetaData *) vMetaData;
uint8_t max_scale = col->metaEntry.type5.scale;
uint8_t max_precision = col->metaEntry.type5.precision;
int target_precision = 0;

out = OutputFunctionCall(finfo, value);
if (out[0] == '-')
Expand Down Expand Up @@ -3080,24 +3084,34 @@ TdsSendTypeNumeric(FmgrInfo *finfo, Datum value, void *vMetaData)
if (scale == -1)
scale = 0;

/* Perform the overflow check before scribbling on to decString. */
target_precision = precision + (max_scale - scale);
if (target_precision > TDS_MAX_NUM_PRECISION ||
target_precision > max_precision)
ereport(ERROR, (errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE),
errmsg("Arithmetic overflow error for data type numeric.")));

/*
* Fill in the remaining 0's if the processed scale from out is less than
* max_scale This is needed because the output generated by engine may not
* always produce the same precision/scale as calculated by
* resolve_numeric_typmod_from_exp, which is the precision/scale we have
* sent to the client with column metadata.
*/
* Fill in the remaining 0's if the processed scale from out is less than
* max_scale This is needed because the output generated by engine may not
* always produce the same precision/scale as calculated by
* resolve_numeric_typmod_from_exp, which is the precision/scale we have
* sent to the client with column metadata.
*/
while (scale++ < max_scale)
{
decString[precision++] = '0';
}
decString[precision] = '\0';
Assert(precision <= outlen);
Assert(precision == target_precision);

if (precision > TDS_MAX_NUM_PRECISION ||
precision > max_precision)
ereport(ERROR, (errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE),
errmsg("Arithmetic overflow error for data type numeric.")));
/*
* Verify that we did not go beyond the memory allocated.
* We allow precision < outlen. Consider the case when
* out="123.456", max_scale=8. Then by the end, precision=11
* but outlen=15.
*/
Assert(precision <= outlen);

if (precision >= 1 && precision < 10)
length = 4;
Expand Down
32 changes: 15 additions & 17 deletions contrib/babelfishpg_tsql/runtime/functions.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1344,7 +1344,7 @@ object_name(PG_FUNCTION_ARGS)
SysScanDesc tgscan;
EphemeralNamedRelation enr;
bool found = false;
char *result = NULL;
text *result_text = NULL;

if (input1 < 0)
PG_RETURN_NULL();
Expand Down Expand Up @@ -1376,9 +1376,7 @@ object_name(PG_FUNCTION_ARGS)
enr = get_ENR_withoid(currentQueryEnv, object_id, ENR_TSQL_TEMP);
if (enr != NULL && enr->md.enrtype == ENR_TSQL_TEMP)
{
result = enr->md.name;

PG_RETURN_VARCHAR_P((VarChar *) cstring_to_text(result));
PG_RETURN_VARCHAR_P((VarChar *) cstring_to_text(enr->md.name));
}

/* search in pg_class by object_id */
Expand All @@ -1389,8 +1387,7 @@ object_name(PG_FUNCTION_ARGS)
if (pg_class_aclcheck(object_id, user_id, ACL_SELECT) == ACLCHECK_OK)
{
Form_pg_class pg_class = (Form_pg_class) GETSTRUCT(tuple);
result = NameStr(pg_class->relname);

result_text = cstring_to_text(NameStr(pg_class->relname)); // make a copy before releasing syscache
schema_id = pg_class->relnamespace;
}
ReleaseSysCache(tuple);
Expand All @@ -1407,8 +1404,7 @@ object_name(PG_FUNCTION_ARGS)
if (pg_proc_aclcheck(object_id, user_id, ACL_EXECUTE) == ACLCHECK_OK)
{
Form_pg_proc procform = (Form_pg_proc) GETSTRUCT(tuple);
result = NameStr(procform->proname);

result_text = cstring_to_text(NameStr(procform->proname));
schema_id = procform->pronamespace;
}
ReleaseSysCache(tuple);
Expand All @@ -1426,7 +1422,7 @@ object_name(PG_FUNCTION_ARGS)
if (pg_type_aclcheck(object_id, user_id, ACL_USAGE) == ACLCHECK_OK)
{
Form_pg_type pg_type = (Form_pg_type) GETSTRUCT(tuple);
result = NameStr(pg_type->typname);
result_text = cstring_to_text(NameStr(pg_type->typname));
}
ReleaseSysCache(tuple);
found = true;
Expand Down Expand Up @@ -1454,8 +1450,7 @@ object_name(PG_FUNCTION_ARGS)
if (OidIsValid(pg_trigger->tgrelid) &&
pg_class_aclcheck(pg_trigger->tgrelid, user_id, ACL_SELECT) == ACLCHECK_OK)
{
result = NameStr(pg_trigger->tgname);

result_text = cstring_to_text(NameStr(pg_trigger->tgname));
schema_id = get_rel_namespace(pg_trigger->tgrelid);
}
found = true;
Expand All @@ -1475,26 +1470,29 @@ object_name(PG_FUNCTION_ARGS)
/* check if user have right permission on object */
if (OidIsValid(con->conrelid) && (pg_class_aclcheck(con->conrelid, user_id, ACL_SELECT) == ACLCHECK_OK))
{
result = NameStr(con->conname);

result_text = cstring_to_text(NameStr(con->conname));
schema_id = con->connamespace;
}
ReleaseSysCache(tuple);
found = true;
}
}

if (result)
if (result_text)
{
/*
* Check if schema corresponding to found object belongs to specified
* database, schema also can be shared schema like "sys" or
* "information_schema_tsql". In case of pg_type schema_id will be
* invalid.
*/
if (!OidIsValid(schema_id) || is_schema_from_db(schema_id, database_id)
|| (schema_id == get_namespace_oid("sys", true)) || (schema_id == get_namespace_oid("information_schema_tsql", true)))
PG_RETURN_VARCHAR_P((VarChar *) cstring_to_text(result));
if (!OidIsValid(schema_id) ||
is_schema_from_db(schema_id, database_id) ||
(schema_id == get_namespace_oid("sys", true)) ||
(schema_id == get_namespace_oid("information_schema_tsql", true)))
{
PG_RETURN_VARCHAR_P((VarChar *) result_text);
}
}
PG_RETURN_NULL();
}
Expand Down
21 changes: 14 additions & 7 deletions contrib/babelfishpg_tsql/src/hooks.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1398,15 +1398,16 @@ pre_transform_target_entry(ResTarget *res, ParseState *pstate,
if(actual_alias_len > alias_len)
{
/* First 32 characters of original_name are assigned to alias. */
memcpy(alias, original_name, (alias_len - 32) );
memcpy(alias, original_name, (alias_len - 32));

/* Last 32 characters of identifier_name are assigned to alias, as actual alias is truncated. */
memcpy(alias + (alias_len) - 32,
identifier_name + (alias_len) - 32,
32);
memcpy(alias + (alias_len - 32),
identifier_name + (alias_len - 32),
32);

alias[alias_len] = '\0';
}
/* Identifier is not truncated. */
else
else /* Identifier is not truncated. */
{
memcpy(alias, original_name, actual_alias_len);
}
Expand Down Expand Up @@ -2274,7 +2275,13 @@ modify_insert_stmt(InsertStmt *stmt, Oid relid)

if (att->attnum > 0)
{
col->name = NameStr(att->attname);
/*
* Do a deep copy of attname because tuple is a pointer
* to a shared_buffer page which is released when scan
* is ended.
*/
col->name = pstrdup(NameStr(att->attname));

col->indirection = NIL;
col->val = NULL;
col->location = 1;
Expand Down
11 changes: 9 additions & 2 deletions contrib/babelfishpg_tsql/src/pl_comp.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -465,6 +465,7 @@ do_compile(FunctionCallInfo fcinfo,
PLtsql_type *argdtype;
PLtsql_variable *argvariable;
PLtsql_nsitem_type argitemtype;
int typmod = 0;

/* Create $n name for variable */
snprintf(buf, sizeof(buf), "$%d", i + 1);
Expand Down Expand Up @@ -496,9 +497,15 @@ do_compile(FunctionCallInfo fcinfo,
function->is_mstvf = true;
}

/* Create datatype info */
/*
* Create datatype info.
* typmods array has length procStruct->pronargs and numargs >= procStruct->pronargs,
* (See Assert in get_func_arg_info())
* Pass in typmods[i] while we are not out of bounds, otherwise pass in -1.
*/
typmod = (typmods && i < procStruct->pronargs) ? typmods[i] : -1;
argdtype = pltsql_build_datatype(argtypeid,
(typmods ? typmods[i] : -1),
typmod,
function->fn_input_collation,
NULL);

Expand Down
2 changes: 1 addition & 1 deletion contrib/babelfishpg_tsql/src/pl_handler.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3172,7 +3172,7 @@ bbf_ProcessUtility(PlannedStmt *pstmt,
Constraint *con;

con = get_rowversion_default_constraint(makeTypeNameFromOid(attr->atttypid, attr->atttypmod));
rawEnt = (RawColumnDefault *) palloc(sizeof(RawColumnDefault));
rawEnt = (RawColumnDefault *) palloc0(sizeof(RawColumnDefault));
rawEnt->attnum = attr_num + 1;
rawEnt->raw_default = (Node *) con->raw_expr;
AddRelationNewConstraints(rel, list_make1(rawEnt), NIL,
Expand Down
3 changes: 1 addition & 2 deletions contrib/babelfishpg_tsql/src/string.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -387,8 +387,6 @@ prepare_format_string(StringInfo buf, char *msg_string, int nargs,
memset(fmt_seg, 0, prev_fmt_seg_sz);
strncpy(fmt_seg, seg_start, seg_len);
prev_fmt_seg_sz = seg_len;

arg = args[i];

if (i >= nargs || argisnull[i])
{
Expand All @@ -397,6 +395,7 @@ prepare_format_string(StringInfo buf, char *msg_string, int nargs,
continue;
}

arg = args[i];
typid = argtypes[i];
type = TypeCategory(typid);

Expand Down

0 comments on commit ec6a8e5

Please sign in to comment.