mirror of
https://github.com/postgres/postgres.git
synced 2026-10-23 13:29:09 +03:00
Fix parallel GIN index build with keys larger than 65535 bytes
During a parallel GIN build, each key is serialized into a GinTuple, a transient representation used only while sorting. _gin_build_tuple() lays out the whole tuple (the palloc size, the key memcpy, and the offset of the posting list) from a local variable holding the real key length, but then stored that length in GinTuple.keylen, which was uint16. For a key longer than 65535 bytes, the stored length was thus silently truncated. On read-back, GinTupleGetFirst() and _gin_parse_tuple_items() recompute the posting-list offset from the truncated keylen and land inside the key data, so ginPostingListDecodeAllSegments() decodes garbage: an assertion failure with assertions enabled, and a read past the end of the allocation without, which could in turn lead to a crash or garbage being written into the index. Only parallel builds are affected, because only they materialize a GinTuple; a serial build of the same data succeeds, as index_form_tuple() compresses large keys before the GinMaxItemSize check. To fix, widen GinTuple.keylen to Size, which is what VARSIZE_ANY() returns and what GinBuffer.keylen already uses, and use Size for the local in _gin_build_tuple() too, which was an int that truncated VARSIZE_ANY() the same way. Widening keylen moves GinTuple.data. The key value is accessed in place in data, so data must be MAXALIGN'ed; before, that was only true by accident of the field layout. This branch predates the use of C11 alignas, so force the alignment the way PGAlignedBlock did: put keylen in a union with double and int64 members, which puts data at a MAXALIGN'ed offset on all platforms, including 32-bit ones where Size is 4 bytes and MAXIMUM_ALIGNOF is 8. A static assertion verifies the resulting offset. Bug: #19545 Author: Ewan Young <kdbase.hack@gmail.com> Reported-by: Yuelin Wang <1217816127@qq.com> Reviewed-by: Peter Eisentraut <peter@eisentraut.org> Discussion: https://www.postgresql.org/message-id/flat/19545-0f25b7e47351e8fc%40postgresql.org
This commit is contained in:
@@ -1454,7 +1454,7 @@ GinBufferStoreTuple(GinBuffer *buffer, GinTuple *tup)
|
||||
if (GinBufferIsEmpty(buffer))
|
||||
{
|
||||
buffer->category = tup->category;
|
||||
buffer->keylen = tup->keylen;
|
||||
buffer->keylen = tup->u.keylen;
|
||||
buffer->attnum = tup->attrnum;
|
||||
|
||||
buffer->typlen = tup->typlen;
|
||||
@@ -2238,7 +2238,7 @@ _gin_build_tuple(OffsetNumber attrnum, unsigned char category,
|
||||
char *ptr;
|
||||
|
||||
Size tuplen;
|
||||
int keylen;
|
||||
Size keylen;
|
||||
|
||||
dlist_mutable_iter iter;
|
||||
dlist_head segments;
|
||||
@@ -2314,7 +2314,7 @@ _gin_build_tuple(OffsetNumber attrnum, unsigned char category,
|
||||
tuple->tuplen = tuplen;
|
||||
tuple->attrnum = attrnum;
|
||||
tuple->category = category;
|
||||
tuple->keylen = keylen;
|
||||
tuple->u.keylen = keylen;
|
||||
tuple->nitems = nitems;
|
||||
|
||||
/* key type info */
|
||||
@@ -2387,7 +2387,7 @@ _gin_parse_tuple_key(GinTuple *a)
|
||||
|
||||
if (a->typbyval)
|
||||
{
|
||||
memcpy(&key, a->data, a->keylen);
|
||||
memcpy(&key, a->data, a->u.keylen);
|
||||
return key;
|
||||
}
|
||||
|
||||
@@ -2406,8 +2406,8 @@ _gin_parse_tuple_items(GinTuple *a)
|
||||
int ndecoded;
|
||||
ItemPointer items;
|
||||
|
||||
len = a->tuplen - SHORTALIGN(offsetof(GinTuple, data) + a->keylen);
|
||||
ptr = (char *) a + SHORTALIGN(offsetof(GinTuple, data) + a->keylen);
|
||||
len = a->tuplen - SHORTALIGN(offsetof(GinTuple, data) + a->u.keylen);
|
||||
ptr = (char *) a + SHORTALIGN(offsetof(GinTuple, data) + a->u.keylen);
|
||||
|
||||
items = ginPostingListDecodeAllSegments((GinPostingList *) ptr, len, &ndecoded);
|
||||
|
||||
|
||||
@@ -21,7 +21,25 @@ typedef struct GinTuple
|
||||
{
|
||||
int tuplen; /* length of the whole tuple */
|
||||
OffsetNumber attrnum; /* attnum of index key */
|
||||
uint16 keylen; /* bytes in data for key value */
|
||||
|
||||
/*
|
||||
* The key value is accessed in place, so data (below) must be aligned
|
||||
* well enough for any key type. We include both "double" and "int64" in
|
||||
* the union to ensure that the compiler knows it must be MAXALIGN'ed (cf.
|
||||
* configure's computation of MAXIMUM_ALIGNOF); this puts data at a
|
||||
* MAXALIGN'ed offset, which the static assertion below verifies. On
|
||||
* 64-bit platforms, this already happens anyway; this trick is only
|
||||
* needed on some 32-bit platforms. Note that putting a flexible array
|
||||
* member into a union is not valid C, so we have to pick some other
|
||||
* member to move the alignment around. This one just happens to be a
|
||||
* space-efficient one.
|
||||
*/
|
||||
union
|
||||
{
|
||||
Size keylen; /* bytes in data for key value */
|
||||
double force_align_d;
|
||||
int64 force_align_i64;
|
||||
} u;
|
||||
int16 typlen; /* typlen for key */
|
||||
bool typbyval; /* typbyval for key */
|
||||
signed char category; /* category: normal or NULL? */
|
||||
@@ -29,12 +47,15 @@ typedef struct GinTuple
|
||||
char data[FLEXIBLE_ARRAY_MEMBER];
|
||||
} GinTuple;
|
||||
|
||||
StaticAssertDecl(offsetof(GinTuple, data) % MAXIMUM_ALIGNOF == 0,
|
||||
"GinTuple.data must be MAXALIGN'ed");
|
||||
|
||||
static inline ItemPointer
|
||||
GinTupleGetFirst(GinTuple *tup)
|
||||
{
|
||||
GinPostingList *list;
|
||||
|
||||
list = (GinPostingList *) SHORTALIGN(tup->data + tup->keylen);
|
||||
list = (GinPostingList *) SHORTALIGN(tup->data + tup->u.keylen);
|
||||
|
||||
return &list->first;
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user