From 82bbb60c30dbff0633da34387ccab58d843379b5 Mon Sep 17 00:00:00 2001
From: Tom Lane <tgl@sss.pgh.pa.us>
Date: Fri, 16 May 2014 15:11:51 -0400
Subject: [PATCH] Fix valgrind warning for btree_gist indexes on macaddr.

The macaddr opclass stores two macaddr structs (each of size 6) in an
index column that's declared as being of type gbtreekey16, ie 16 bytes.
In the original coding this led to passing a palloc'd value of size 12
to the index insertion code, so that data would be fetched past the
end of the allocated value during index tuple construction.  This makes
valgrind unhappy.  In principle it could result in a SIGSEGV, though
with the current implementation of palloc there's no risk since
the 12-byte request size would be rounded up to 16 bytes anyway.

To fix, add a field to struct gbtree_ninfo showing the declared size of
the index datums, and use that in the palloc requests; and use palloc0
to be sure that any wasted bytes are cleanly initialized.

Per report from Andres Freund.  No back-patch since there's no current
risk of a real problem.
---
 contrib/btree_gist/btree_cash.c      | 1 +
 contrib/btree_gist/btree_date.c      | 1 +
 contrib/btree_gist/btree_float4.c    | 1 +
 contrib/btree_gist/btree_float8.c    | 1 +
 contrib/btree_gist/btree_inet.c      | 1 +
 contrib/btree_gist/btree_int2.c      | 1 +
 contrib/btree_gist/btree_int4.c      | 1 +
 contrib/btree_gist/btree_int8.c      | 1 +
 contrib/btree_gist/btree_interval.c  | 5 ++++-
 contrib/btree_gist/btree_macaddr.c   | 1 +
 contrib/btree_gist/btree_oid.c       | 1 +
 contrib/btree_gist/btree_time.c      | 1 +
 contrib/btree_gist/btree_ts.c        | 1 +
 contrib/btree_gist/btree_utils_num.c | 6 ++++--
 contrib/btree_gist/btree_utils_num.h | 3 ++-
 15 files changed, 22 insertions(+), 4 deletions(-)

diff --git a/contrib/btree_gist/btree_cash.c b/contrib/btree_gist/btree_cash.c
index 8de3716c945..63f86ebeefe 100644
--- a/contrib/btree_gist/btree_cash.c
+++ b/contrib/btree_gist/btree_cash.c
@@ -78,6 +78,7 @@ static const gbtree_ninfo tinfo =
 {
 	gbt_t_cash,
 	sizeof(Cash),
+	16,							/* sizeof(gbtreekey16) */
 	gbt_cashgt,
 	gbt_cashge,
 	gbt_casheq,
diff --git a/contrib/btree_gist/btree_date.c b/contrib/btree_gist/btree_date.c
index 9cab7ec42f5..7a4c6aa6003 100644
--- a/contrib/btree_gist/btree_date.c
+++ b/contrib/btree_gist/btree_date.c
@@ -96,6 +96,7 @@ static const gbtree_ninfo tinfo =
 {
 	gbt_t_date,
 	sizeof(DateADT),
+	8,							/* sizeof(gbtreekey8) */
 	gbt_dategt,
 	gbt_datege,
 	gbt_dateeq,
diff --git a/contrib/btree_gist/btree_float4.c b/contrib/btree_gist/btree_float4.c
index 55e1c4c1c6d..778d8dad844 100644
--- a/contrib/btree_gist/btree_float4.c
+++ b/contrib/btree_gist/btree_float4.c
@@ -77,6 +77,7 @@ static const gbtree_ninfo tinfo =
 {
 	gbt_t_float4,
 	sizeof(float4),
+	8,							/* sizeof(gbtreekey8) */
 	gbt_float4gt,
 	gbt_float4ge,
 	gbt_float4eq,
diff --git a/contrib/btree_gist/btree_float8.c b/contrib/btree_gist/btree_float8.c
index 62271dec84a..c898bf2d975 100644
--- a/contrib/btree_gist/btree_float8.c
+++ b/contrib/btree_gist/btree_float8.c
@@ -85,6 +85,7 @@ static const gbtree_ninfo tinfo =
 {
 	gbt_t_float8,
 	sizeof(float8),
+	16,							/* sizeof(gbtreekey16) */
 	gbt_float8gt,
 	gbt_float8ge,
 	gbt_float8eq,
diff --git a/contrib/btree_gist/btree_inet.c b/contrib/btree_gist/btree_inet.c
index 24ae6bf3693..822786125d9 100644
--- a/contrib/btree_gist/btree_inet.c
+++ b/contrib/btree_gist/btree_inet.c
@@ -74,6 +74,7 @@ static const gbtree_ninfo tinfo =
 {
 	gbt_t_inet,
 	sizeof(double),
+	16,							/* sizeof(gbtreekey16) */
 	gbt_inetgt,
 	gbt_inetge,
 	gbt_ineteq,
diff --git a/contrib/btree_gist/btree_int2.c b/contrib/btree_gist/btree_int2.c
index d51ad0c2ab2..a88aae64537 100644
--- a/contrib/btree_gist/btree_int2.c
+++ b/contrib/btree_gist/btree_int2.c
@@ -77,6 +77,7 @@ static const gbtree_ninfo tinfo =
 {
 	gbt_t_int2,
 	sizeof(int16),
+	4,							/* sizeof(gbtreekey4) */
 	gbt_int2gt,
 	gbt_int2ge,
 	gbt_int2eq,
diff --git a/contrib/btree_gist/btree_int4.c b/contrib/btree_gist/btree_int4.c
index e7641f22847..889a5120783 100644
--- a/contrib/btree_gist/btree_int4.c
+++ b/contrib/btree_gist/btree_int4.c
@@ -78,6 +78,7 @@ static const gbtree_ninfo tinfo =
 {
 	gbt_t_int4,
 	sizeof(int32),
+	8,							/* sizeof(gbtreekey8) */
 	gbt_int4gt,
 	gbt_int4ge,
 	gbt_int4eq,
diff --git a/contrib/btree_gist/btree_int8.c b/contrib/btree_gist/btree_int8.c
index 8bc8cb5fdf7..8685cee1760 100644
--- a/contrib/btree_gist/btree_int8.c
+++ b/contrib/btree_gist/btree_int8.c
@@ -78,6 +78,7 @@ static const gbtree_ninfo tinfo =
 {
 	gbt_t_int8,
 	sizeof(int64),
+	16,							/* sizeof(gbtreekey16) */
 	gbt_int8gt,
 	gbt_int8ge,
 	gbt_int8eq,
diff --git a/contrib/btree_gist/btree_interval.c b/contrib/btree_gist/btree_interval.c
index 93a341eb77e..68d80e8e0ae 100644
--- a/contrib/btree_gist/btree_interval.c
+++ b/contrib/btree_gist/btree_interval.c
@@ -87,7 +87,9 @@ gbt_intv_dist(const void *a, const void *b)
 /*
  * INTERVALSIZE should be the actual size-on-disk of an Interval, as shown
  * in pg_type.  This might be less than sizeof(Interval) if the compiler
- * insists on adding alignment padding at the end of the struct.
+ * insists on adding alignment padding at the end of the struct.  (Note:
+ * this concern is obsolete with the current definition of Interval, but
+ * was real before a separate "day" field was added to it.)
  */
 #define INTERVALSIZE 16
 
@@ -95,6 +97,7 @@ static const gbtree_ninfo tinfo =
 {
 	gbt_t_intv,
 	sizeof(Interval),
+	32,							/* sizeof(gbtreekey32) */
 	gbt_intvgt,
 	gbt_intvge,
 	gbt_intveq,
diff --git a/contrib/btree_gist/btree_macaddr.c b/contrib/btree_gist/btree_macaddr.c
index 6255564ac5a..244b95154bb 100644
--- a/contrib/btree_gist/btree_macaddr.c
+++ b/contrib/btree_gist/btree_macaddr.c
@@ -74,6 +74,7 @@ static const gbtree_ninfo tinfo =
 {
 	gbt_t_macad,
 	sizeof(macaddr),
+	16,							/* sizeof(gbtreekey16) */
 	gbt_macadgt,
 	gbt_macadge,
 	gbt_macadeq,
diff --git a/contrib/btree_gist/btree_oid.c b/contrib/btree_gist/btree_oid.c
index dcd0765417b..f6b7bfa05b7 100644
--- a/contrib/btree_gist/btree_oid.c
+++ b/contrib/btree_gist/btree_oid.c
@@ -84,6 +84,7 @@ static const gbtree_ninfo tinfo =
 {
 	gbt_t_oid,
 	sizeof(Oid),
+	8,							/* sizeof(gbtreekey8) */
 	gbt_oidgt,
 	gbt_oidge,
 	gbt_oideq,
diff --git a/contrib/btree_gist/btree_time.c b/contrib/btree_gist/btree_time.c
index e0e32428e22..cdf81711e76 100644
--- a/contrib/btree_gist/btree_time.c
+++ b/contrib/btree_gist/btree_time.c
@@ -124,6 +124,7 @@ static const gbtree_ninfo tinfo =
 {
 	gbt_t_time,
 	sizeof(TimeADT),
+	16,							/* sizeof(gbtreekey16) */
 	gbt_timegt,
 	gbt_timege,
 	gbt_timeeq,
diff --git a/contrib/btree_gist/btree_ts.c b/contrib/btree_gist/btree_ts.c
index 10f325d6720..a13dcc8beaa 100644
--- a/contrib/btree_gist/btree_ts.c
+++ b/contrib/btree_gist/btree_ts.c
@@ -127,6 +127,7 @@ static const gbtree_ninfo tinfo =
 {
 	gbt_t_ts,
 	sizeof(Timestamp),
+	16,							/* sizeof(gbtreekey16) */
 	gbt_tsgt,
 	gbt_tsge,
 	gbt_tseq,
diff --git a/contrib/btree_gist/btree_utils_num.c b/contrib/btree_gist/btree_utils_num.c
index 5e52ab542bf..505633c98b8 100644
--- a/contrib/btree_gist/btree_utils_num.c
+++ b/contrib/btree_gist/btree_utils_num.c
@@ -28,7 +28,7 @@ gbt_num_compress(GISTENTRY *retval, GISTENTRY *entry, const gbtree_ninfo *tinfo)
 			Cash		ch;
 		}			v;
 
-		GBT_NUMKEY *r = (GBT_NUMKEY *) palloc0(2 * tinfo->size);
+		GBT_NUMKEY *r = (GBT_NUMKEY *) palloc0(tinfo->indexsize);
 		void	   *leaf = NULL;
 
 		switch (tinfo->t)
@@ -77,6 +77,8 @@ gbt_num_compress(GISTENTRY *retval, GISTENTRY *entry, const gbtree_ninfo *tinfo)
 				leaf = DatumGetPointer(entry->key);
 		}
 
+		Assert(tinfo->indexsize >= 2 * tinfo->size);
+
 		memcpy((void *) &r[0], leaf, tinfo->size);
 		memcpy((void *) &r[tinfo->size], leaf, tinfo->size);
 		retval = palloc(sizeof(GISTENTRY));
@@ -165,7 +167,7 @@ gbt_num_bin_union(Datum *u, GBT_NUMKEY *e, const gbtree_ninfo *tinfo)
 
 	if (!DatumGetPointer(*u))
 	{
-		*u = PointerGetDatum(palloc(2 * tinfo->size));
+		*u = PointerGetDatum(palloc0(tinfo->indexsize));
 		memcpy((void *) &(((GBT_NUMKEY *) DatumGetPointer(*u))[0]), (void *) rd.lower, tinfo->size);
 		memcpy((void *) &(((GBT_NUMKEY *) DatumGetPointer(*u))[tinfo->size]), (void *) rd.upper, tinfo->size);
 	}
diff --git a/contrib/btree_gist/btree_utils_num.h b/contrib/btree_gist/btree_utils_num.h
index d7a61d22426..0d79cd2a7f7 100644
--- a/contrib/btree_gist/btree_utils_num.h
+++ b/contrib/btree_gist/btree_utils_num.h
@@ -37,7 +37,8 @@ typedef struct
 	/* Attribs */
 
 	enum gbtree_type t;			/* data type */
-	int32		size;			/* size of type , 0 means variable */
+	int32		size;			/* size of type, 0 means variable */
+	int32		indexsize;		/* size of datums stored in index */
 
 	/* Methods */
 
-- 
GitLab