On Wed, Nov 27, 2013 at 9:10 AM, Noah Misch <n...@leadboat.com> wrote: > The threat is that rounding the read size up to the next MAXALIGN would cross > into an unreadable memory page, resulting in a SIGSEGV. Every palloc chunk > has MAXALIGN'd size under the hood, so the excess read of "toDelete" cannot > cause a SIGSEGV. For a stack variable, it depends on the ABI. I'm not aware > of an ABI where the four bytes past the end of this stack variable could be > unreadable, which is not to claim I'm well-read on the topic. We should fix > this in due course on code hygiene grounds, but I would not back-patch it.
Attached patch silences the "Invalid read of size n" complaints of Valgrind. I agree with your general thoughts around backpatching. Note that the patch addresses a distinct complaint from Kevin's, as Valgrind doesn't take issue with the invalid reads past the end of spgxlogPickSplit variables on the stack. -- Peter Geoghegan
*** a/src/backend/access/spgist/spgdoinsert.c --- b/src/backend/access/spgist/spgdoinsert.c *************** moveLeafs(Relation index, SpGistState *s *** 412,419 **** /* Locate the tuples to be moved, and count up the space needed */ i = PageGetMaxOffsetNumber(current->page); ! toDelete = (OffsetNumber *) palloc(sizeof(OffsetNumber) * i); ! toInsert = (OffsetNumber *) palloc(sizeof(OffsetNumber) * (i + 1)); size = newLeafTuple->size + sizeof(ItemIdData); --- 412,419 ---- /* Locate the tuples to be moved, and count up the space needed */ i = PageGetMaxOffsetNumber(current->page); ! toDelete = (OffsetNumber *) palloc0(MAXALIGN(sizeof(OffsetNumber) * i)); ! toInsert = (OffsetNumber *) palloc0(MAXALIGN(sizeof(OffsetNumber) * (i + 1))); size = newLeafTuple->size + sizeof(ItemIdData); *************** doPickSplit(Relation index, SpGistState *** 722,731 **** n = max + 1; in.datums = (Datum *) palloc(sizeof(Datum) * n); heapPtrs = (ItemPointerData *) palloc(sizeof(ItemPointerData) * n); ! toDelete = (OffsetNumber *) palloc(sizeof(OffsetNumber) * n); ! toInsert = (OffsetNumber *) palloc(sizeof(OffsetNumber) * n); newLeafs = (SpGistLeafTuple *) palloc(sizeof(SpGistLeafTuple) * n); ! leafPageSelect = (uint8 *) palloc(sizeof(uint8) * n); xlrec.node = index->rd_node; STORE_STATE(state, xlrec.stateSrc); --- 722,731 ---- n = max + 1; in.datums = (Datum *) palloc(sizeof(Datum) * n); heapPtrs = (ItemPointerData *) palloc(sizeof(ItemPointerData) * n); ! toDelete = (OffsetNumber *) palloc0(MAXALIGN(sizeof(OffsetNumber) * n)); ! toInsert = (OffsetNumber *) palloc0(MAXALIGN(sizeof(OffsetNumber) * n)); newLeafs = (SpGistLeafTuple *) palloc(sizeof(SpGistLeafTuple) * n); ! leafPageSelect = (uint8 *) palloc0(MAXALIGN(sizeof(uint8) * n)); xlrec.node = index->rd_node; STORE_STATE(state, xlrec.stateSrc);
-- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
