Another batch of memory management improvement

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]>
babelfish-for-postgresql · Oct 17, 2023 · a165efe · a165efe
1 parent 293e2e9
commit a165efe
Show file tree

Hide file tree

Showing 7 changed files with 44 additions and 32 deletions.
diff --git a/contrib/babelfishpg_tds/src/backend/tds/tdstypeio.c b/contrib/babelfishpg_tds/src/backend/tds/tdstypeio.c
@@ -2700,10 +2700,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)

diff --git a/contrib/babelfishpg_tsql/runtime/functions.c b/contrib/babelfishpg_tsql/runtime/functions.c
@@ -1520,7 +1520,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();
@@ -1552,9 +1552,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 */
@@ -1565,8 +1563,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);
@@ -1583,8 +1580,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);
@@ -1602,7 +1598,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;
@@ -1630,8 +1626,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;
@@ -1651,26 +1646,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();
 }

diff --git a/contrib/babelfishpg_tsql/src/hooks.c b/contrib/babelfishpg_tsql/src/hooks.c
@@ -1759,15 +1759,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);
 			}
@@ -2636,7 +2637,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;

diff --git a/contrib/babelfishpg_tsql/src/pl_comp.c b/contrib/babelfishpg_tsql/src/pl_comp.c
@@ -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);
@@ -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);
 

diff --git a/contrib/babelfishpg_tsql/src/pl_handler.c b/contrib/babelfishpg_tsql/src/pl_handler.c
@@ -3498,7 +3498,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,

diff --git a/contrib/babelfishpg_tsql/src/rolecmds.c b/contrib/babelfishpg_tsql/src/rolecmds.c
@@ -2136,8 +2136,6 @@ babelfish_add_domain_mapping_entry_internal(PG_FUNCTION_ARGS)
 	new_record = palloc0(sizeof(Datum) * BBF_DOMAIN_MAPPING_NUM_COLS);
 	new_record_nulls = palloc0(sizeof(bool) * BBF_DOMAIN_MAPPING_NUM_COLS);
 
-	MemSet(new_record_nulls, false, sizeof(new_record_nulls));
-
 	new_record[0] = PG_GETARG_DATUM(0);
 	new_record[1] = PG_GETARG_DATUM(1);
 

diff --git a/contrib/babelfishpg_tsql/src/string.c b/contrib/babelfishpg_tsql/src/string.c
@@ -407,15 +407,14 @@ prepare_format_string(StringInfo buf, char *msg_string, int nargs,
 			strncpy(fmt_seg, seg_start, seg_len);
 			prev_fmt_seg_sz = seg_len;
 
-			arg = args[i];
-
 			if (i >= nargs || argisnull[i])
 			{
 				appendStringInfo(buf, "(null)");
 				appendStringInfoString(buf, fmt_seg + 2);
 				continue;
 			}
 
+			arg = args[i];
 			typid = argtypes[i];
 			type = TypeCategory(typid);