Location via proxy:   [ UP ]  
[Report a bug]   [Manage cookies]                
Skip to content

Commit 6462238

Browse files
committed
Fix uninitialized memory propagation mistakes
Valgrind complains that some uninitialized bytes are being passed around by the extended statistics code since commit 7b504eb, as reported by Andres Freund. Silence it. Tomas Vondra submitted a patch which he verified to fix the complaints in his machine; however I messed with it a bit before pushing, so any remaining problems are likely my (Álvaro's) fault. Author: Tomas Vondra Discussion: https://postgr.es/m/20170325211031.4xxoptigqxm2emn2@alap3.anarazel.de
1 parent 6e31c3e commit 6462238

File tree

2 files changed

+61
-35
lines changed

2 files changed

+61
-35
lines changed

src/backend/statistics/mvdistinct.c

+54-35
Original file line numberDiff line numberDiff line change
@@ -161,10 +161,10 @@ statext_ndistinct_serialize(MVNDistinct *ndistinct)
161161
Assert(ndistinct->type == STATS_NDISTINCT_TYPE_BASIC);
162162

163163
/*
164-
* Base size is base struct size, plus one base struct for each items,
165-
* including number of items for each.
164+
* Base size is size of scalar fields in the struct, plus one base struct
165+
* for each item, including number of items for each.
166166
*/
167-
len = VARHDRSZ + offsetof(MVNDistinct, items) +
167+
len = VARHDRSZ + SizeOfMVNDistinct +
168168
ndistinct->nitems * (offsetof(MVNDistinctItem, attrs) + sizeof(int));
169169

170170
/* and also include space for the actual attribute numbers */
@@ -182,9 +182,13 @@ statext_ndistinct_serialize(MVNDistinct *ndistinct)
182182

183183
tmp = VARDATA(output);
184184

185-
/* Store the base struct values */
186-
memcpy(tmp, ndistinct, offsetof(MVNDistinct, items));
187-
tmp += offsetof(MVNDistinct, items);
185+
/* Store the base struct values (magic, type, nitems) */
186+
memcpy(tmp, &ndistinct->magic, sizeof(uint32));
187+
tmp += sizeof(uint32);
188+
memcpy(tmp, &ndistinct->type, sizeof(uint32));
189+
tmp += sizeof(uint32);
190+
memcpy(tmp, &ndistinct->nitems, sizeof(uint32));
191+
tmp += sizeof(uint32);
188192

189193
/*
190194
* store number of attributes and attribute numbers for each ndistinct
@@ -224,49 +228,64 @@ MVNDistinct *
224228
statext_ndistinct_deserialize(bytea *data)
225229
{
226230
int i;
227-
Size expected_size;
231+
Size minimum_size;
232+
MVNDistinct ndist;
228233
MVNDistinct *ndistinct;
229234
char *tmp;
230235

231236
if (data == NULL)
232237
return NULL;
233238

234-
if (VARSIZE_ANY_EXHDR(data) < offsetof(MVNDistinct, items))
239+
/* we expect at least the basic fields of MVNDistinct struct */
240+
if (VARSIZE_ANY_EXHDR(data) < SizeOfMVNDistinct)
235241
elog(ERROR, "invalid MVNDistinct size %ld (expected at least %ld)",
236-
VARSIZE_ANY_EXHDR(data), offsetof(MVNDistinct, items));
237-
238-
/* read the MVNDistinct header */
239-
ndistinct = (MVNDistinct *) palloc(sizeof(MVNDistinct));
242+
VARSIZE_ANY_EXHDR(data), SizeOfMVNDistinct);
240243

241244
/* initialize pointer to the data part (skip the varlena header) */
242245
tmp = VARDATA_ANY(data);
243246

244-
/* get the header and perform basic sanity checks */
245-
memcpy(ndistinct, tmp, offsetof(MVNDistinct, items));
246-
tmp += offsetof(MVNDistinct, items);
247-
248-
if (ndistinct->magic != STATS_NDISTINCT_MAGIC)
249-
elog(ERROR, "invalid ndistinct magic %d (expected %d)",
250-
ndistinct->magic, STATS_NDISTINCT_MAGIC);
251-
252-
if (ndistinct->type != STATS_NDISTINCT_TYPE_BASIC)
253-
elog(ERROR, "invalid ndistinct type %d (expected %d)",
254-
ndistinct->type, STATS_NDISTINCT_TYPE_BASIC);
255-
256-
Assert(ndistinct->nitems > 0);
247+
/* read the header fields and perform basic sanity checks */
248+
memcpy(&ndist.magic, tmp, sizeof(uint32));
249+
tmp += sizeof(uint32);
250+
memcpy(&ndist.type, tmp, sizeof(uint32));
251+
tmp += sizeof(uint32);
252+
memcpy(&ndist.nitems, tmp, sizeof(uint32));
253+
tmp += sizeof(uint32);
254+
255+
if (ndist.magic != STATS_NDISTINCT_MAGIC)
256+
ereport(ERROR,
257+
(errcode(ERRCODE_DATA_CORRUPTED),
258+
errmsg("invalid ndistinct magic %08x (expected %08x)",
259+
ndist.magic, STATS_NDISTINCT_MAGIC)));
260+
if (ndist.type != STATS_NDISTINCT_TYPE_BASIC)
261+
ereport(ERROR,
262+
(errcode(ERRCODE_DATA_CORRUPTED),
263+
errmsg("invalid ndistinct type %d (expected %d)",
264+
ndist.type, STATS_NDISTINCT_TYPE_BASIC)));
265+
if (ndist.nitems == 0)
266+
ereport(ERROR,
267+
(errcode(ERRCODE_DATA_CORRUPTED),
268+
errmsg("invalid zero-length item array in MVNDistinct")));
257269

258270
/* what minimum bytea size do we expect for those parameters */
259-
expected_size = offsetof(MVNDistinct, items) +
260-
ndistinct->nitems * (offsetof(MVNDistinctItem, attrs) +
261-
sizeof(AttrNumber) * 2);
271+
minimum_size = (SizeOfMVNDistinct +
272+
ndist.nitems * (SizeOfMVNDistinctItem +
273+
sizeof(AttrNumber) * 2));
274+
if (VARSIZE_ANY_EXHDR(data) < minimum_size)
275+
ereport(ERROR,
276+
(errcode(ERRCODE_DATA_CORRUPTED),
277+
errmsg("invalid MVNDistinct size %ld (expected at least %ld)",
278+
VARSIZE_ANY_EXHDR(data), minimum_size)));
262279

263-
if (VARSIZE_ANY_EXHDR(data) < expected_size)
264-
elog(ERROR, "invalid dependencies size %ld (expected at least %ld)",
265-
VARSIZE_ANY_EXHDR(data), expected_size);
266-
267-
/* allocate space for the ndistinct items */
268-
ndistinct = repalloc(ndistinct, offsetof(MVNDistinct, items) +
269-
(ndistinct->nitems * sizeof(MVNDistinctItem)));
280+
/*
281+
* Allocate space for the ndistinct items (no space for each item's attnos:
282+
* those live in bitmapsets allocated separately)
283+
*/
284+
ndistinct = palloc0(MAXALIGN(SizeOfMVNDistinct) +
285+
(ndist.nitems * sizeof(MVNDistinctItem)));
286+
ndistinct->magic = ndist.magic;
287+
ndistinct->type = ndist.type;
288+
ndistinct->nitems = ndist.nitems;
270289

271290
for (i = 0; i < ndistinct->nitems; i++)
272291
{

src/include/statistics/statistics.h

+7
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,9 @@ typedef struct MVNDistinctItem
2727
double ndistinct; /* ndistinct value for this combination */
2828
Bitmapset *attrs; /* attr numbers of items */
2929
} MVNDistinctItem;
30+
/* size of the struct, excluding attribute list */
31+
#define SizeOfMVNDistinctItem \
32+
(offsetof(MVNDistinctItem, ndistinct) + sizeof(double))
3033

3134
/* A MVNDistinct object, comprising all possible combinations of columns */
3235
typedef struct MVNDistinct
@@ -37,6 +40,10 @@ typedef struct MVNDistinct
3740
MVNDistinctItem items[FLEXIBLE_ARRAY_MEMBER];
3841
} MVNDistinct;
3942

43+
/* size of the struct excluding the items array */
44+
#define SizeOfMVNDistinct (offsetof(MVNDistinct, nitems) + sizeof(uint32))
45+
46+
4047
extern MVNDistinct *statext_ndistinct_load(Oid mvoid);
4148

4249
extern void BuildRelationExtStatistics(Relation onerel, double totalrows,

0 commit comments

Comments
 (0)