Fix potential use-after-free in error handling.

- Store PQresultStatus() result in a local variable before calling cdbdisp_clearCdbPgResults() - Prevent accessing PQresultStatus() after clearing CDB results - Improve consistency in error handling across different functions - Avoid redundant function calls by caching result of PQresultStatus() Enhances safety and readability in segment result processing
apache · Jan 27, 2025 · 9150751 · 9150751
1 parent ccac620
commit 9150751
Show file tree

Hide file tree

Showing 3 changed files with 25 additions and 21 deletions.
diff --git a/src/backend/cdb/cdbsreh.c b/src/backend/cdb/cdbsreh.c
@@ -1098,13 +1098,13 @@ TruncateErrorLog(text *relname, bool persistent)
 			Datum		value;
 			bool		isnull;
 			struct pg_result *pgresult = cdb_pgresults.pg_results[i];
-
-			if (PQresultStatus(pgresult) != PGRES_TUPLES_OK)
+			ExecStatusType status = PQresultStatus(pgresult);
+			if (status != PGRES_TUPLES_OK)
 			{
 				cdbdisp_clearCdbPgResults(&cdb_pgresults);
 				ereport(ERROR,
 						(errmsg("unexpected result from segment: %d",
-								PQresultStatus(pgresult))));
+								status)));
 			}
 			value = ResultToDatum(pgresult, 0, 0, boolin, &isnull);
 			allResults &= (!isnull && DatumGetBool(value));

diff --git a/src/backend/commands/analyze.c b/src/backend/commands/analyze.c
@@ -5109,29 +5109,30 @@ calculate_correlation_use_weighted_mean(CdbPgResults *cdb_pgresults,
 
 	for (int segno = 0; segno < segmentNum; segno++)
 	{
-		int			rows;
+		int			rows, nfields;
 		float4		correlationValue;
 		int			attno;
 		struct pg_result *pgresult = cdb_pgresults->pg_results[segno];
 		int			index;
-
-		if (PQresultStatus(pgresult) != PGRES_TUPLES_OK)
+		ExecStatusType status = PQresultStatus(pgresult);
+		if (status != PGRES_TUPLES_OK)
 		{
 			cdbdisp_clearCdbPgResults(cdb_pgresults);
 			ereport(ERROR,
 					(errmsg("unexpected result from segment: %d",
-							PQresultStatus(pgresult))));
+							status)));
 		}
 		/*
 		 * gp_acquire_correlations returns a result for each alive columns.
 		 */
 		rows = PQntuples(pgresult);
-		if (rows != live_natts || PQnfields(pgresult) != 1)
+		nfields = PQnfields(pgresult);
+		if (rows != live_natts || nfields != 1)
 		{
 			cdbdisp_clearCdbPgResults(cdb_pgresults);
 			ereport(ERROR,
 					(errmsg("unexpected shape of result from segment (%d rows, %d cols)",
-							rows, PQnfields(pgresult))));
+							rows, nfields)));
 		}
 		for (int j = 0; j < rows; j++)
 		{
@@ -5232,28 +5233,29 @@ calculate_correlation_use_mean(CdbPgResults *cdb_pgresults,
 
 	for (int segno = 0; segno < segmentNum; segno++)
 	{
-		int			ntuples;
+		int			ntuples, nfields;
 		float4		correlationValue;
 		int			attno;
 		struct pg_result *pgresult = cdb_pgresults->pg_results[segno];
-
-		if (PQresultStatus(pgresult) != PGRES_TUPLES_OK)
+		ExecStatusType  status = PQresultStatus(pgresult);
+		if (status != PGRES_TUPLES_OK)
 		{
 			cdbdisp_clearCdbPgResults(cdb_pgresults);
 			ereport(ERROR,
 					(errmsg("unexpected result from segment: %d",
-							PQresultStatus(pgresult))));
+							status)));
 		}
 		/*
 		 * gp_acquire_correlations returns a result for each alive columns.
 		 */
 		ntuples = PQntuples(pgresult);
-		if (ntuples != live_natts || PQnfields(pgresult) != 1)
+		nfields = PQnfields(pgresult);
+		if (ntuples != live_natts || nfields != 1)
 		{
 			cdbdisp_clearCdbPgResults(cdb_pgresults);
 			ereport(ERROR,
 					(errmsg("unexpected shape of result from segment (%d rows, %d cols)",
-							ntuples, PQnfields(pgresult))));
+							ntuples, nfields)));
 		}
 		for (int j = 0; j < ntuples; j++)
 		{

diff --git a/src/backend/commands/dirtablecmds.c b/src/backend/commands/dirtablecmds.c
@@ -478,22 +478,24 @@ remove_file(PG_FUNCTION_ARGS)
 	numDeletes = 0;
 	for (i = 0; i < cdbPgresults.numResults; i++)
 	{
+		int ntuples, nfields;
 		Datum value;
 		struct pg_result *pgresult = cdbPgresults.pg_results[i];
-
-		if (PQresultStatus(pgresult) != PGRES_TUPLES_OK)
+		ExecStatusType status = PQresultStatus(pgresult);
+		if (status != PGRES_TUPLES_OK)
 		{
 			cdbdisp_clearCdbPgResults(&cdbPgresults);
 			ereport(ERROR,
-					(errmsg("unexpected result from segment: %d", PQresultStatus(pgresult))));
+					(errmsg("unexpected result from segment: %d", status)));
 		}
-
-		if (PQntuples(pgresult) != 1 || PQnfields(pgresult) != 1)
+		ntuples = PQntuples(pgresult);
+		nfields = PQnfields(pgresult);
+		if (ntuples != 1 || nfields != 1)
 		{
 			cdbdisp_clearCdbPgResults(&cdbPgresults);
 			ereport(ERROR,
 					(errmsg("unexpected shape of result from segment (%d rows, %d cols)",
-							PQntuples(pgresult), PQnfields(pgresult))));
+							ntuples, nfields)));
 		}
 		if (PQgetisnull(pgresult, 0, 0))
 			value = 0;