From 82bbb60c30dbff0633da34387ccab58d843379b5 Mon Sep 17 00:00:00 2001 From: Tom Lane 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 8de3716c94..63f86ebeef 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 9cab7ec42f..7a4c6aa600 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 55e1c4c1c6..778d8dad84 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 62271dec84..c898bf2d97 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 24ae6bf369..822786125d 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 d51ad0c2ab..a88aae6453 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 e7641f2284..889a512078 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 8bc8cb5fdf..8685cee176 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 93a341eb77..68d80e8e0a 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 6255564ac5..244b95154b 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 dcd0765417..f6b7bfa05b 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 e0e32428e2..cdf81711e7 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 10f325d672..a13dcc8bea 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 5e52ab542b..505633c98b 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 d7a61d2242..0d79cd2a7f 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 */ -- 2.40.0