From 970a0395421581dd097cca9707f72a8bc9faec53 Mon Sep 17 00:00:00 2001
From: Daniel Gustafsson <dgustafsson@pivotal.io>
Date: Thu, 24 Jan 2019 14:00:17 +0100
Subject: [PATCH] Fix memory management in gp_sparse_vector

palloc() is guaranteed to only return on successful allocation, so there
is no need to check it. ereport(ERROR..) is guaranteed never to return,
and to clean up on it's way out, so pfree()ing after an ereport() is not
just unreachable code, it would be a double-free if it was reached.

Also add proper checks on the malloc() and strdup() calls as those are
subject to the usual memory pressure controls by the programmer.

Reviewed-by: Heikki Linnakangas <hlinnakangas@pivotal.io>
---
 gpcontrib/gp_sparse_vector/SparseData.c |  8 +-------
 gpcontrib/gp_sparse_vector/gp_sfv.c     | 15 +++++++++++++++
 2 files changed, 16 insertions(+), 7 deletions(-)

diff --git a/gpcontrib/gp_sparse_vector/SparseData.c b/gpcontrib/gp_sparse_vector/SparseData.c
index cf16a3241a..7f142c47af 100644
--- a/gpcontrib/gp_sparse_vector/SparseData.c
+++ b/gpcontrib/gp_sparse_vector/SparseData.c
@@ -226,13 +226,7 @@ double *sdata_to_float8arr(SparseData sdata) {
 			errmsg("data type of SparseData is not FLOAT64")));
 	}
 
-	if ((array = (double *)palloc(sizeof(double)*(sdata->total_value_count)))
-			== NULL)
-	{
-		ereport(ERROR,(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
-			errmsg("Error allocating memory for array\n")));
-	}
-
+	array = (double *) palloc(sizeof(double) * sdata->total_value_count);
 	iptr = sdata->index->data;
 	aptr = 0;
 	for (int i=0; i<sdata->unique_value_count; i++) {
diff --git a/gpcontrib/gp_sparse_vector/gp_sfv.c b/gpcontrib/gp_sparse_vector/gp_sfv.c
index 82d083819b..b0d6087399 100644
--- a/gpcontrib/gp_sparse_vector/gp_sfv.c
+++ b/gpcontrib/gp_sparse_vector/gp_sfv.c
@@ -196,7 +196,18 @@ SvecType *classify_document(char **features, int num_features, char **document,
 #endif
 		(void) hdestroy();
 		ordinals    = (int *)malloc(sizeof(int)*num_features); //Need to use malloc so that hdestroy() can be called.
+		if (!ordinals)
+			ereport(ERROR,
+					(errcode(ERRCODE_OUT_OF_MEMORY),
+					 errmsg("out of memory")));
+
 		histogram = (float8 *)malloc(sizeof(float8)*num_features); //Use malloc because pallocs are cleaned up between queries
+		if (!histogram) {
+			free(ordinals);
+			ereport(ERROR,
+					(errcode(ERRCODE_OUT_OF_MEMORY),
+					 errmsg("out of memory")));
+		}
 
 		for (i=0; i<num_features; i++) {
 			ordinals[i] = i;
@@ -206,6 +217,10 @@ SvecType *classify_document(char **features, int num_features, char **document,
 		for (i=0; i<num_features; i++) {
 			if (features[i] != NULL) {
 		  		item.key = strdup(features[i]);
+				if (!item.key)
+					ereport(ERROR,
+							(errcode(ERRCODE_OUT_OF_MEMORY),
+							 errmsg("out of memory")));
 		  		item.data = (void *)(&(ordinals[i]));
 		  		(void) hsearch(item,ENTER);
 			}
-- 
GitLab