public inbox for [email protected]
help / color / mirror / Atom feedFrom: Ewan Young <[email protected]>
To: Heikki Linnakangas <[email protected]>
Cc: [email protected]
Cc: [email protected]
Subject: Re: BUG #19545: Integer truncation of `GinTuple.keylen` causes out-of-bounds read in parallel GIN index build
Date: Wed, 8 Jul 2026 19:34:39 +0800
Message-ID: <CAON2xHPV-7O0MsFuizJv8cbUG7ZJfLCAqzkGYgoxF9WBe1v9Jg@mail.gmail.com> (raw)
In-Reply-To: <[email protected]>
References: <[email protected]>
<CAON2xHMXYLwkq4UaNbUtmaw1ZKne6OqdyyiACFQUbCj+XRBvQQ@mail.gmail.com>
<[email protected]>
Hi Heikki,
Thanks a lot for taking a look, and for the good questions!
On Wed, Jul 8, 2026 at 3:52 PM Heikki Linnakangas <[email protected]> wrote:
>
> On 08/07/2026 09:27, Ewan Young wrote:
> > Hi Yuelin,
> >
> > Thanks for the very precise report -- I reproduced it on master and your
> > analysis is exactly right. _gin_build_tuple() builds the whole GinTuple
> > (palloc size, key memcpy, TID-list offset) from the int keylen, but the
> > stored GinTuple.keylen is uint16, so a key wider than 65535 bytes has its
> > stored length truncated. On read-back GinTupleGetFirst() and
> > _gin_parse_tuple_items() recompute the posting-list offset from the
> > truncated value, and ginPostingListDecodeAllSegments() then walks the key
> > bytes, aborting (or reading past the allocation on non-assert builds)
> > exactly as you saw. It's parallel-only because only the parallel path
> > serializes a GinTuple.
> >
> > I went with your fix A -- widening keylen to uint32 (attached). It's the
> > minimal root-cause fix: the stored length now matches the length the rest
> > of the function already uses.
>
> Ugh, the datatypes used for keylen are all over the place. In GinTuple
> struct it was 'uint16', in GinBuffer it's Size, and in the
> _gin_build_tuple() function's local variable it's 'int'. Would be good
> to make them consistent.
Good point, agreed. v2 (attached) uses int for keylen everywhere: in
GinTuple (was uint16) and in GinBuffer (was Size); the local in
_gin_build_tuple() was already int. int matches the tuplen and nitems
fields of GinTuple and is plenty wide (a key can't exceed the 1GB varlena
limit), so it seemed like the natural choice.
>
> > I preferred it over an explicit ereport at
> > UINT16_MAX, since 65535 isn't a meaningful GIN limit -- the entry-tree item
> > limit is much smaller and is applied to the (compressed) tuple by
> > GinFormTuple() -- so rejecting there would be an arbitrary cutoff.
>
> Hmm, we don't compress the key data though, so a tuple with a key larger
> than 65535 will inevitably fail in GinFormTuple(), right? I agree it
That was my first thought too, but it turns out we do compress it, just
not in _gin_build_tuple(). GinFormTuple() builds the on-page tuple via
index_form_tuple(), whose TOAST_INDEX_HACK path compresses a compressible
key over TOAST_INDEX_TARGET (~BLCKSZ/16) inline before the GinMaxItemSize
check runs. So that check sees the *compressed* key, and a large but
compressible key sails through. It's only the parallel path's GinTuple
that keeps the key uncompressed, which is exactly where the uint16
truncation bit us.
Concretely, unpatched master indexes a key far larger than GinMaxItemSize
just fine in a serial build:
CREATE TABLE t (a text[]);
INSERT INTO t SELECT ARRAY[repeat('x',100000)] FROM generate_series(1,50);
SET max_parallel_maintenance_workers = 0; -- force a serial build
CREATE INDEX ON t USING gin (a); -- succeeds; key is
100000 bytes
So a "keylen > GinMaxItemSize" check in _gin_build_tuple() would end up
rejecting, in a parallel build, keys that a serial build happily accepts,
i.e. it would make parallel builds stricter than serial ones. And a genuinely
incompressible oversized key already fails cleanly in GinFormTuple()
("index row size ... exceeds maximum"), with the server staying up, both
serially and (with this patch) in parallel.
So my inclination is to keep GinFormTuple() as the single size gate: it
enforces the real, post-compression limit and keeps the two build paths
behaving identically. I've attached that variant too (it errors when the
uncompressed keylen exceeds GinMaxItemSize). Happy to go whichever
way you think is best.
> would be a little arbitrary to cut off at 65535, but then again, it
> seems a little silly to continue when we know it's just going to fail
> later on. I think it would make sense to check if keylen >
> GinMaxItemSize. It might still fail later in GinFormTuple(), because the
> index tuple headers take some space, but still.
>
> - Heikki
>
--
Regards,
Ewan Young
Attachments:
[application/octet-stream] v2-0001-Fix-parallel-GIN-index-build-keys-larger-than-65535.patch (2.7K, ../CAON2xHPV-7O0MsFuizJv8cbUG7ZJfLCAqzkGYgoxF9WBe1v9Jg@mail.gmail.com/2-v2-0001-Fix-parallel-GIN-index-build-keys-larger-than-65535.patch)
download | inline diff:
From 439897dad9470f1836c18c75b409bc152483870f Mon Sep 17 00:00:00 2001
From: Ewan Young <[email protected]>
Date: Thu, 9 Jul 2026 00:42:19 +0800
Subject: [PATCH v2] 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 to the posting list -- from a local "int keylen". But the
stored GinTuple.keylen field was uint16, so a key wider than 65535
bytes had its stored length 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. ginPostingListDecodeAllSegments() then decodes garbage,
tripping an assertion (or reading past the allocation in a non-assert
build). Only parallel builds are affected, because only the parallel
path materializes a GinTuple.
Widen keylen so the stored length matches the length the rest of
_gin_build_tuple() already computes. While here, make the type used
for the key length consistent -- it was uint16 in GinTuple, Size in
GinBuffer, and int in the _gin_build_tuple() local variable -- by using
int throughout, matching the tuplen and nitems fields of GinTuple.
Bug: #19545
Reported-by: Yuelin Wang <[email protected]>
Discussion: https://postgr.es/m/[email protected]
---
src/backend/access/gin/gininsert.c | 2 +-
src/include/access/gin_tuple.h | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/backend/access/gin/gininsert.c b/src/backend/access/gin/gininsert.c
index cb9ed3b563c..01a7ef23372 100644
--- a/src/backend/access/gin/gininsert.c
+++ b/src/backend/access/gin/gininsert.c
@@ -1190,7 +1190,7 @@ typedef struct GinBuffer
OffsetNumber attnum;
GinNullCategory category;
Datum key; /* 0 if no key (and keylen == 0) */
- Size keylen; /* number of bytes (not typlen) */
+ int keylen; /* number of bytes (not typlen) */
/* type info */
int16 typlen;
diff --git a/src/include/access/gin_tuple.h b/src/include/access/gin_tuple.h
index 7bde05e2de4..9ca578652e7 100644
--- a/src/include/access/gin_tuple.h
+++ b/src/include/access/gin_tuple.h
@@ -23,7 +23,7 @@ typedef struct GinTuple
{
int tuplen; /* length of the whole tuple */
OffsetNumber attrnum; /* attnum of index key */
- uint16 keylen; /* bytes in data for key value */
+ int keylen; /* bytes in data for key value */
int16 typlen; /* typlen for key */
bool typbyval; /* typbyval for key */
signed char category; /* category: normal or NULL? */
--
2.47.3
[application/octet-stream] v2-0001-alt-Fix-parallel-GIN-index-build-fail-fast-variant.patch (3.8K, ../CAON2xHPV-7O0MsFuizJv8cbUG7ZJfLCAqzkGYgoxF9WBe1v9Jg@mail.gmail.com/3-v2-0001-alt-Fix-parallel-GIN-index-build-fail-fast-variant.patch)
download | inline diff:
From 18925af4dd7115a15bdf8b8ea9604b51e45081f6 Mon Sep 17 00:00:00 2001
From: Ewan Young <[email protected]>
Date: Thu, 9 Jul 2026 00:52:01 +0800
Subject: [PATCH v2] Fix parallel GIN index build with keys larger than 65535 (fail-fast variant)
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 to the posting list -- from a local "int keylen". But the
stored GinTuple.keylen field was uint16, so a key wider than 65535
bytes had its stored length 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. ginPostingListDecodeAllSegments() then decodes garbage,
tripping an assertion (or reading past the allocation in a non-assert
build). Only parallel builds are affected, because only the parallel
path materializes a GinTuple.
Widen keylen so the stored length matches the length the rest of
_gin_build_tuple() already computes, and make the type consistent --
it was uint16 in GinTuple, Size in GinBuffer, and int in the
_gin_build_tuple() local variable -- by using int throughout.
In addition, reject in _gin_build_tuple() any key whose (uncompressed)
length exceeds GinMaxItemSize, so a parallel build fails fast rather
than serializing a GinTuple that GinFormTuple() would later be unable
to store.
Bug: #19545
Reported-by: Yuelin Wang <[email protected]>
Discussion: https://postgr.es/m/[email protected]
---
src/backend/access/gin/gininsert.c | 19 ++++++++++++++++++-
src/include/access/gin_tuple.h | 2 +-
2 files changed, 19 insertions(+), 2 deletions(-)
diff --git a/src/backend/access/gin/gininsert.c b/src/backend/access/gin/gininsert.c
index cb9ed3b563c..c2365afd1fd 100644
--- a/src/backend/access/gin/gininsert.c
+++ b/src/backend/access/gin/gininsert.c
@@ -1190,7 +1190,7 @@ typedef struct GinBuffer
OffsetNumber attnum;
GinNullCategory category;
Datum key; /* 0 if no key (and keylen == 0) */
- Size keylen; /* number of bytes (not typlen) */
+ int keylen; /* number of bytes (not typlen) */
/* type info */
int16 typlen;
@@ -2281,6 +2281,23 @@ _gin_build_tuple(OffsetNumber attrnum, unsigned char category,
else
elog(ERROR, "unexpected typlen value (%d)", typlen);
+ /*
+ * Reject a key that cannot possibly be stored in the index, so that a
+ * parallel build fails fast instead of serializing a GinTuple that the
+ * leader will be unable to write out. This mirrors the GinMaxItemSize
+ * limit enforced by GinFormTuple() when the merged tuple is finally
+ * written.
+ *
+ * Note the key here is uncompressed; GinFormTuple() applies inline
+ * compression (via index_form_tuple) before its own check, so this is
+ * more conservative and the GinFormTuple() check is still required.
+ */
+ if (keylen > GinMaxItemSize)
+ ereport(ERROR,
+ (errcode(ERRCODE_PROGRAM_LIMIT_EXCEEDED),
+ errmsg("index row size %d exceeds maximum %zu for a GIN index",
+ keylen, (Size) GinMaxItemSize)));
+
/* compress the item pointers */
ncompressed = 0;
compresslen = 0;
diff --git a/src/include/access/gin_tuple.h b/src/include/access/gin_tuple.h
index 7bde05e2de4..9ca578652e7 100644
--- a/src/include/access/gin_tuple.h
+++ b/src/include/access/gin_tuple.h
@@ -23,7 +23,7 @@ typedef struct GinTuple
{
int tuplen; /* length of the whole tuple */
OffsetNumber attrnum; /* attnum of index key */
- uint16 keylen; /* bytes in data for key value */
+ int keylen; /* bytes in data for key value */
int16 typlen; /* typlen for key */
bool typbyval; /* typbyval for key */
signed char category; /* category: normal or NULL? */
--
2.47.3
view thread (9+ messages) latest in thread
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected], [email protected], [email protected], [email protected]
Subject: Re: BUG #19545: Integer truncation of `GinTuple.keylen` causes out-of-bounds read in parallel GIN index build
In-Reply-To: <CAON2xHPV-7O0MsFuizJv8cbUG7ZJfLCAqzkGYgoxF9WBe1v9Jg@mail.gmail.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox