pg.ddx.io pgsql-bugs@postgresql.org mailing list archive
help / color / mirror / Atom feedBUG #19545: Integer truncation of `GinTuple.keylen` causes out-of-bounds read in parallel GIN index build
14+ messages / 5 participants
[nested] [flat]
* BUG #19545: Integer truncation of `GinTuple.keylen` causes out-of-bounds read in parallel GIN index build
@ 2026-07-07 14:13 PG Bug reporting form <noreply@postgresql.org>
0 siblings, 1 reply; 14+ messages in thread
From: PG Bug reporting form @ 2026-07-07 14:13 UTC (permalink / raw)
To: pgsql-bugs@lists.postgresql.org; +Cc: 1217816127@qq.com
The following bug has been logged on the website:
Bug reference: 19545
Logged by: Yuelin Wang
Email address: 1217816127@qq.com
PostgreSQL version: 19beta1
Operating system: Linux (Ubuntu 24.04, x86_64)
Description:
### Summary
`GinTuple.keylen` is declared `uint16` (max 65535), but in
`_gin_build_tuple()` the local `keylen` is an `int` taken from
`VARSIZE_ANY(key)` (up to ~1 GB for a varlena `NORM_KEY`). The store
`tuple->keylen = keylen` **truncates** when the key exceeds 65535 bytes. The
tuple's *memory layout* (palloc size, key memcpy, TID-array pointer) uses
the full `int` keylen and is correct. Only the stored field is truncated. On
read-back the truncated value mis-computes the posting-list region, so the
decoder walks attacker-controlled bytes past the allocation, which is a heap
out-of-bounds read.
### Details
```c
int keylen; /* 2251 */
keylen = VARSIZE_ANY(DatumGetPointer(key)); /* 2278: up to ~1GB */
tuple->keylen = keylen; /* 2327: stored into uint16 ->
truncation */
```
On read-back `_gin_parse_tuple_items()` (2419-2420) uses the truncated
`a->keylen`, so the posting-list pointer lands ~64 KB early inside the
attacker's key content and `ginPostingListDecodeAllSegments()` decodes
attacker-controlled bytes off the end of the allocation. This is
parallel-only because the `GinMaxItemSize` guard lives in the leader's
`GinFormTuple`, whereas a parallel worker reparses the serialized `GinTuple`
*before* any size gate (present in 19devel).
### Proof of Concept
`array_ops`' `extractValue` returns full-size array-element datums, so a
`text[]` element > 65535 bytes flows in as `NORM_KEY`:
```sql
CREATE SCHEMA vuln_001_sch;
CREATE TABLE vuln_001_sch.vuln_001_t (a text[]);
-- one 100000-byte element per row (> 65535 -> keylen truncation), enough
rows for real sort work
INSERT INTO vuln_001_sch.vuln_001_t
SELECT ARRAY[repeat('x', 100000)] FROM generate_series(1, 300);
-- force a parallel maintenance build (all settable by a non-superuser)
ALTER TABLE vuln_001_sch.vuln_001_t SET (parallel_workers = 4);
SET max_parallel_maintenance_workers = 4;
SET min_parallel_table_scan_size = '0';
CREATE INDEX vuln_001_idx ON vuln_001_sch.vuln_001_t USING gin (a);
```
Run:
```
psql -h /tmp -p 36901 -U ylwang -d postgres -f vuln_001.sql
```
### Result
The parallel build crashed 3 workers (PIDs 114475 to 114477) during `CREATE
INDEX ... USING gin`, on exactly the predicted path
(`ginPostingListDecodeAllSegments`):
```
DEBUG: building index "vuln_001_idx" ... with request for 4 parallel
workers
server closed the connection unexpectedly
parallel worker ... ExceptionalCondition ...
parallel worker ... ginPostingListDecodeAllSegments+0x163
TRAP: failed
Assert("OffsetNumberIsValid(ItemPointerGetOffsetNumber(&segment->first))"),
File: "ginpostinglist.c", Line: 324
```
### Fix
Widen the field to hold real key lengths by declaring `GinTuple.keylen` as
`int`/`uint32` in `gin_tuple.h`. Alternatively, reject oversized keys
explicitly in `_gin_build_tuple()` by calling `ereport(ERROR)` when `keylen
> UINT16_MAX`, so the parallel path fails as cleanly as the serial one.
^ permalink raw reply [nested|flat] 14+ messages in thread
* Re: BUG #19545: Integer truncation of `GinTuple.keylen` causes out-of-bounds read in parallel GIN index build
@ 2026-07-08 06:27 Ewan Young <kdbase.hack@gmail.com>
parent: PG Bug reporting form <noreply@postgresql.org>
0 siblings, 1 reply; 14+ messages in thread
From: Ewan Young @ 2026-07-08 06:27 UTC (permalink / raw)
To: 1217816127@qq.com; pgsql-bugs@lists.postgresql.org
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. 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.
Verified on master with your PoC:
- Before: the parallel build aborts in ginPostingListDecodeAllSegments
(Assert at ginpostinglist.c:324), as reported.
- After: the parallel build succrrect
results -- your 100000-byte element compresses well under the
entry-tree item limit, so it irial build.
- A genuinely incompressible key wider than the item limit now fails
cleanly ("index row requires N) in both
the parallel and serial paths, instead of crashing.
I didn't add a regression test -- forcing parallel maintenance workers plus
a >64 kB key deterministically in -- but I'm
happy to add one if preferred.
Attaching v1 against master; this should go back to the branches that have
parallel GIN builds.
On Wed, Jul 8, 2026 at 7:58 AM PG Bug reporting form
<noreply@postgresql.org> wrote:
>
> The following bug has been logged on the website:
>
> Bug reference: 19545
> Logged by: Yuelin Wang
> Email address: 1217816127@qq.com
> PostgreSQL version: 19beta1
> Operating system: Linux (Ubuntu 24.04, x86_64)
> Description:
>
> ### Summary
>
> `GinTuple.keylen` is declared `uint16` (max 65535), but in
> `_gin_build_tuple()` the local `keylen` is an `int` taken from
> `VARSIZE_ANY(key)` (up to ~1 GB for a varlena `NORM_KEY`). The store
> `tuple->keylen = keylen` **truncates** when the key exceeds 65535 bytes. The
> tuple's *memory layout* (palloc size, key memcpy, TID-array pointer) uses
> the full `int` keylen and is correct. Only the stored field is truncated. On
> read-back the truncated value mis-computes the posting-list region, so the
> decoder walks attacker-controlled bytes past the allocation, which is a heap
> out-of-bounds read.
>
> ### Details
>
> ```c
> int keylen; /* 2251 */
> keylen = VARSIZE_ANY(DatumGetPointer(key)); /* 2278: up to ~1GB */
> tuple->keylen = keylen; /* 2327: stored into uint16 ->
> truncation */
> ```
>
> On read-back `_gin_parse_tuple_items()` (2419-2420) uses the truncated
> `a->keylen`, so the posting-list pointer lands ~64 KB early inside the
> attacker's key content and `ginPostingListDecodeAllSegments()` decodes
> attacker-controlled bytes off the end of the allocation. This is
> parallel-only because the `GinMaxItemSize` guard lives in the leader's
> `GinFormTuple`, whereas a parallel worker reparses the serialized `GinTuple`
> *before* any size gate (present in 19devel).
>
> ### Proof of Concept
>
> `array_ops`' `extractValue` returns full-size array-element datums, so a
> `text[]` element > 65535 bytes flows in as `NORM_KEY`:
>
> ```sql
> CREATE SCHEMA vuln_001_sch;
> CREATE TABLE vuln_001_sch.vuln_001_t (a text[]);
>
> -- one 100000-byte element per row (> 65535 -> keylen truncation), enough
> rows for real sort work
> INSERT INTO vuln_001_sch.vuln_001_t
> SELECT ARRAY[repeat('x', 100000)] FROM generate_series(1, 300);
>
> -- force a parallel maintenance build (all settable by a non-superuser)
> ALTER TABLE vuln_001_sch.vuln_001_t SET (parallel_workers = 4);
> SET max_parallel_maintenance_workers = 4;
> SET min_parallel_table_scan_size = '0';
>
> CREATE INDEX vuln_001_idx ON vuln_001_sch.vuln_001_t USING gin (a);
> ```
>
> Run:
>
> ```
> psql -h /tmp -p 36901 -U ylwang -d postgres -f vuln_001.sql
> ```
>
> ### Result
>
> The parallel build crashed 3 workers (PIDs 114475 to 114477) during `CREATE
> INDEX ... USING gin`, on exactly the predicted path
> (`ginPostingListDecodeAllSegments`):
>
> ```
> DEBUG: building index "vuln_001_idx" ... with request for 4 parallel
> workers
> server closed the connection unexpectedly
> parallel worker ... ExceptionalCondition ...
> parallel worker ... ginPostingListDecodeAllSegments+0x163
> TRAP: failed
> Assert("OffsetNumberIsValid(ItemPointerGetOffsetNumber(&segment->first))"),
> File: "ginpostinglist.c", Line: 324
> ```
>
> ### Fix
>
> Widen the field to hold real key lengths by declaring `GinTuple.keylen` as
> `int`/`uint32` in `gin_tuple.h`. Alternatively, reject oversized keys
> explicitly in `_gin_build_tuple()` by calling `ereport(ERROR)` when `keylen
> > UINT16_MAX`, so the parallel path fails as cleanly as the serial one.
>
>
>
>
--
Regards,
Ewan Young
Attachments:
[application/octet-stream] v1-0001-Widen-GinTuple.keylen-to-uint32-to-fix-parallel-G.patch (2.4K, ../../CAON2xHMXYLwkq4UaNbUtmaw1ZKne6OqdyyiACFQUbCj+XRBvQQ@mail.gmail.com/2-v1-0001-Widen-GinTuple.keylen-to-uint32-to-fix-parallel-G.patch)
download | inline diff:
From b78b6b6f0e71403cf93fb15d4ed4470a7dbb9ad9 Mon Sep 17 00:00:00 2001
From: Ewan Young <kdbase.hack@gmail.com>
Date: Wed, 8 Jul 2026 22:18:56 +0800
Subject: [PATCH v1] Widen GinTuple.keylen to uint32 to fix parallel GIN builds
with large keys
During a parallel GIN index build, _gin_build_tuple() computes the key
length as an int from VARSIZE_ANY() (up to ~1GB for a varlena key) and
lays out the serialized GinTuple -- its palloc size, the key memcpy, and
the offset of the compressed TID list -- using that full length. But
GinTuple.keylen was declared uint16, so the stored field wrapped around
for keys larger than 65535 bytes while the rest of the tuple used the
untruncated length.
On read-back GinTupleGetFirst() and _gin_parse_tuple_items() recompute
the TID-list offset from the truncated keylen, so it points into the key
data instead of at the compressed posting list.
ginPostingListDecodeAllSegments() then decodes arbitrary key bytes, which
aborts the build (Assert in ginpostinglist.c) or reads past the
allocation on non-assert builds. Any user able to run CREATE INDEX with
parallel workers can hit this, e.g. a gin index on a text[] column with
an element wider than 65535 bytes.
This is specific to parallel builds: the serial path never materializes a
GinTuple. The general index-tuple size limit is still enforced by
GinFormTuple() in both cases.
Widen keylen to uint32 so the stored length matches the length used to
build the tuple. A parallel build then correctly indexes large keys that
compress below the entry-tree item limit (as a serial build already does),
and still rejects genuinely oversized keys cleanly via GinFormTuple()
instead of crashing.
Reported-by: Yuelin Wang <1217816127@qq.com>
Bug: #19545
---
src/include/access/gin_tuple.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/include/access/gin_tuple.h b/src/include/access/gin_tuple.h
index 7bde05e2de4..a116f3a1a77 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 */
+ uint32 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
^ permalink raw reply [nested|flat] 14+ messages in thread
* Re: BUG #19545: Integer truncation of `GinTuple.keylen` causes out-of-bounds read in parallel GIN index build
@ 2026-07-08 07:52 Heikki Linnakangas <hlinnaka@iki.fi>
parent: Ewan Young <kdbase.hack@gmail.com>
0 siblings, 1 reply; 14+ messages in thread
From: Heikki Linnakangas @ 2026-07-08 07:52 UTC (permalink / raw)
To: Ewan Young <kdbase.hack@gmail.com>; 1217816127@qq.com; pgsql-bugs@lists.postgresql.org
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.
> 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
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
^ permalink raw reply [nested|flat] 14+ messages in thread
* Re: BUG #19545: Integer truncation of `GinTuple.keylen` causes out-of-bounds read in parallel GIN index build
@ 2026-07-08 11:34 Ewan Young <kdbase.hack@gmail.com>
parent: Heikki Linnakangas <hlinnaka@iki.fi>
0 siblings, 2 replies; 14+ messages in thread
From: Ewan Young @ 2026-07-08 11:34 UTC (permalink / raw)
To: Heikki Linnakangas <hlinnaka@iki.fi>; +Cc: 1217816127@qq.com; pgsql-bugs@lists.postgresql.org
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 <hlinnaka@iki.fi> 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 <kdbase.hack@gmail.com>
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 <1217816127@qq.com>
Discussion: https://postgr.es/m/19545-DUMMY-REPLACE-WITH-REAL-MSGID@postgresql.org
---
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 <kdbase.hack@gmail.com>
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 <1217816127@qq.com>
Discussion: https://postgr.es/m/19545-DUMMY-REPLACE-WITH-REAL-MSGID@postgresql.org
---
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
^ permalink raw reply [nested|flat] 14+ messages in thread
* Re: BUG #19545: Integer truncation of `GinTuple.keylen` causes out-of-bounds read in parallel GIN index build
@ 2026-07-08 14:36 Heikki Linnakangas <hlinnaka@iki.fi>
parent: Ewan Young <kdbase.hack@gmail.com>
1 sibling, 1 reply; 14+ messages in thread
From: Heikki Linnakangas @ 2026-07-08 14:36 UTC (permalink / raw)
To: Ewan Young <kdbase.hack@gmail.com>; +Cc: 1217816127@qq.com; pgsql-bugs@lists.postgresql.org
On 08/07/2026 14:34, Ewan Young wrote:
> On Wed, Jul 8, 2026 at 3:52 PM Heikki Linnakangas <hlinnaka@iki.fi> 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.
When I built this with "-fsanitize=alignment,undefined" flag, it
triggers a sanity check in the 'jsonb' test:
(gdb) bt
#0 __pthread_kill_implementation (threadid=281473024218464,
signo=signo@entry=6, no_tid=no_tid@entry=0) at ./nptl/pthread_kill.c:44
#1 0x0000ffff89fa7e24 [PAC] in __pthread_kill_internal
(threadid=<optimized out>, signo=6) at ./nptl/pthread_kill.c:89
#2 0x0000ffff89f56940 in __GI_raise (sig=sig@entry=6) at
../sysdeps/posix/raise.c:26
#3 0x0000ffff89f41a84 [PAC] in __GI_abort () at ./stdlib/abort.c:77
#4 0x0000ffff8a0fc600 [PAC] in __sanitizer::Abort () at
../../../../src/libsanitizer/sanitizer_common/sanitizer_posix_libcdep.cpp:143
#5 0x0000ffff8a10bc94 [PAC] in __sanitizer::Die () at
../../../../src/libsanitizer/sanitizer_common/sanitizer_termination.cpp:58
#6 0x0000ffff8a0e7da0 [PAC] in __ubsan::ScopedReport::~ScopedReport
(this=this@entry=0xffffd8a433d0, __in_chrg=<optimized out>)
at ../../../../src/libsanitizer/ubsan/ubsan_diag.cpp:402
#7 0x0000ffff8a0eb10c [PAC] in handleTypeMismatchImpl (Data=<optimized
out>, Pointer=187650525702668, Opts=...)
at ../../../../src/libsanitizer/ubsan/ubsan_handlers.cpp:137
#8 0x0000ffff8a0ebc0c [PAC] in
__ubsan::__ubsan_handle_type_mismatch_v1_abort (Data=<optimized out>,
Pointer=<optimized out>)
at ../../../../src/libsanitizer/ubsan/ubsan_handlers.cpp:147
#9 0x0000aaaab7d3f2e4 [PAC] in GinBufferKeyEquals
(buffer=0xaaaacaee4330, tup=0xaaaacaed29f8) at
../src/backend/access/gin/gininsert.c:1382
#10 0x0000aaaab7d414c0 in GinBufferCanAddKey (buffer=0xaaaacaee4330,
tup=0xaaaacaed29f8) at ../src/backend/access/gin/gininsert.c:1633
#11 0x0000aaaab7d42500 in _gin_process_worker_data
(state=0xffffd8a43890, worker_sort=0xaaaacae16fb0, progress=true) at
../src/backend/access/gin/gininsert.c:1899
#12 0x0000aaaab7d43618 in _gin_parallel_scan_and_build
(state=0xffffd8a43890, ginshared=0xffff8b9b83a0,
sharedsort=0xffff8b9b8340, heap=0xffff7d7012a8,
index=0xffff7d708768, sortmem=21845, progress=true) at
../src/backend/access/gin/gininsert.c:2085
#13 0x0000aaaab7d42378 in _gin_leader_participate_as_worker
(buildstate=0xffffd8a43890, heap=0xffff7d7012a8, index=0xffff7d708768)
at ../src/backend/access/gin/gininsert.c:1834
#14 0x0000aaaab7d3d7b0 in _gin_begin_parallel
(buildstate=0xffffd8a43890, heap=0xffff7d7012a8, index=0xffff7d708768,
isconcurrent=true, request=2)
at ../src/backend/access/gin/gininsert.c:1103
#15 0x0000aaaab7d3ae3c in ginbuild (heap=0xffff7d7012a8,
index=0xffff7d708768, indexInfo=0xaaaacad8f5b0) at
../src/backend/access/gin/gininsert.c:700
#16 0x0000aaaab807c110 in index_build (heapRelation=0xffff7d7012a8,
indexRelation=0xffff7d708768, indexInfo=0xaaaacad8f5b0, isreindex=false,
parallel=true, progress=true)
at ../src/backend/catalog/index.c:3099
#17 0x0000aaaab8074514 in index_concurrently_build
(heapRelationId=41578, indexRelationId=41993) at
../src/backend/catalog/index.c:1543
That's this line:
#9 0x0000aaaab7d3f2e4 [PAC] in GinBufferKeyEquals
(buffer=0xaaaacaee4330, tup=0xaaaacaed29f8) at
../src/backend/access/gin/gininsert.c:1382
1382 tupkey = (buffer->typbyval) ? *(Datum *) tup->data :
PointerGetDatum(tup->data);
So we have a hidden assumption that 'data' is Datum-aligned.
In _gin_parse_tuple_key() we do this instead:
Datum key;
...
if (a->typbyval)
{
memcpy(&key, a->data, a->keylen);
return key;
}
That one doesn't require the alignment. I would be inclined to always
use memcpy() when 'typbyval==true', as above, to not be sensitive to the
alignment. However, I think we assume that it's aligned for the
'typbyval==false' case anyway, as we just do DatumGetPoint(a->data).
The straightforward fix is to add padding to make 'data' MAXALIGNed. It
makes GinTuples larger, which is bad for performance, but it's probably
fine.
That said, I actually wonder why we need to store 'typbyval' and
'typlen' in GinTuple at all. That information could be looked up using
'attrnum'. Maybe 'typbyval' is good for performance in the comparison
functions, but AFAICS GinTuple->typbyval is only used to copy it into
GinBuffer in GinBufferStoreTuple(), which I think could easily afford to
look it up.
>>> 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
Oh, ok, I stand corrected. Let's keep that working then.
- Heikki
^ permalink raw reply [nested|flat] 14+ messages in thread
* Re: BUG #19545: Integer truncation of `GinTuple.keylen` causes out-of-bounds read in parallel GIN index build
@ 2026-07-08 17:14 Peter Eisentraut <peter@eisentraut.org>
parent: Ewan Young <kdbase.hack@gmail.com>
1 sibling, 0 replies; 14+ messages in thread
From: Peter Eisentraut @ 2026-07-08 17:14 UTC (permalink / raw)
To: Ewan Young <kdbase.hack@gmail.com>; Heikki Linnakangas <hlinnaka@iki.fi>; +Cc: 1217816127@qq.com; pgsql-bugs@lists.postgresql.org
On 08.07.26 13:34, Ewan Young wrote:
> 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 <hlinnaka@iki.fi> 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.
Size (or size_t) is the correct type for sizes of objects in memory.
Note that the return type of VARSIZE_ANY() is already Size, so by using
int you are still doing a type truncation, and by using a signed type
you are introducing unnecessary potential for confusion.
^ permalink raw reply [nested|flat] 14+ messages in thread
* Re: BUG #19545: Integer truncation of `GinTuple.keylen` causes out-of-bounds read in parallel GIN index build
@ 2026-07-09 05:16 Ewan Young <kdbase.hack@gmail.com>
parent: Heikki Linnakangas <hlinnaka@iki.fi>
0 siblings, 2 replies; 14+ messages in thread
From: Ewan Young @ 2026-07-09 05:16 UTC (permalink / raw)
To: Heikki Linnakangas <hlinnaka@iki.fi>; Peter Eisentraut <peter@eisentraut.org>; +Cc: 1217816127@qq.com; pgsql-bugs@lists.postgresql.org
Hi Heikki and Peter,
On Wed, Jul 8, 2026 at 10:36 PM Heikki Linnakangas <hlinnaka@iki.fi> wrote:
>
> On 08/07/2026 14:34, Ewan Young wrote:
> > On Wed, Jul 8, 2026 at 3:52 PM Heikki Linnakangas <hlinnaka@iki.fi> 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.
>
> When I built this with "-fsanitize=alignment,undefined" flag, it
> triggers a sanity check in the 'jsonb' test:
>
> (gdb) bt
> #0 __pthread_kill_implementation (threadid=281473024218464,
> signo=signo@entry=6, no_tid=no_tid@entry=0) at ./nptl/pthread_kill.c:44
> #1 0x0000ffff89fa7e24 [PAC] in __pthread_kill_internal
> (threadid=<optimized out>, signo=6) at ./nptl/pthread_kill.c:89
> #2 0x0000ffff89f56940 in __GI_raise (sig=sig@entry=6) at
> ../sysdeps/posix/raise.c:26
> #3 0x0000ffff89f41a84 [PAC] in __GI_abort () at ./stdlib/abort.c:77
> #4 0x0000ffff8a0fc600 [PAC] in __sanitizer::Abort () at
> ../../../../src/libsanitizer/sanitizer_common/sanitizer_posix_libcdep.cpp:143
> #5 0x0000ffff8a10bc94 [PAC] in __sanitizer::Die () at
> ../../../../src/libsanitizer/sanitizer_common/sanitizer_termination.cpp:58
> #6 0x0000ffff8a0e7da0 [PAC] in __ubsan::ScopedReport::~ScopedReport
> (this=this@entry=0xffffd8a433d0, __in_chrg=<optimized out>)
> at ../../../../src/libsanitizer/ubsan/ubsan_diag.cpp:402
> #7 0x0000ffff8a0eb10c [PAC] in handleTypeMismatchImpl (Data=<optimized
> out>, Pointer=187650525702668, Opts=...)
> at ../../../../src/libsanitizer/ubsan/ubsan_handlers.cpp:137
> #8 0x0000ffff8a0ebc0c [PAC] in
> __ubsan::__ubsan_handle_type_mismatch_v1_abort (Data=<optimized out>,
> Pointer=<optimized out>)
> at ../../../../src/libsanitizer/ubsan/ubsan_handlers.cpp:147
> #9 0x0000aaaab7d3f2e4 [PAC] in GinBufferKeyEquals
> (buffer=0xaaaacaee4330, tup=0xaaaacaed29f8) at
> ../src/backend/access/gin/gininsert.c:1382
> #10 0x0000aaaab7d414c0 in GinBufferCanAddKey (buffer=0xaaaacaee4330,
> tup=0xaaaacaed29f8) at ../src/backend/access/gin/gininsert.c:1633
> #11 0x0000aaaab7d42500 in _gin_process_worker_data
> (state=0xffffd8a43890, worker_sort=0xaaaacae16fb0, progress=true) at
> ../src/backend/access/gin/gininsert.c:1899
> #12 0x0000aaaab7d43618 in _gin_parallel_scan_and_build
> (state=0xffffd8a43890, ginshared=0xffff8b9b83a0,
> sharedsort=0xffff8b9b8340, heap=0xffff7d7012a8,
> index=0xffff7d708768, sortmem=21845, progress=true) at
> ../src/backend/access/gin/gininsert.c:2085
> #13 0x0000aaaab7d42378 in _gin_leader_participate_as_worker
> (buildstate=0xffffd8a43890, heap=0xffff7d7012a8, index=0xffff7d708768)
> at ../src/backend/access/gin/gininsert.c:1834
> #14 0x0000aaaab7d3d7b0 in _gin_begin_parallel
> (buildstate=0xffffd8a43890, heap=0xffff7d7012a8, index=0xffff7d708768,
> isconcurrent=true, request=2)
> at ../src/backend/access/gin/gininsert.c:1103
> #15 0x0000aaaab7d3ae3c in ginbuild (heap=0xffff7d7012a8,
> index=0xffff7d708768, indexInfo=0xaaaacad8f5b0) at
> ../src/backend/access/gin/gininsert.c:700
> #16 0x0000aaaab807c110 in index_build (heapRelation=0xffff7d7012a8,
> indexRelation=0xffff7d708768, indexInfo=0xaaaacad8f5b0, isreindex=false,
> parallel=true, progress=true)
> at ../src/backend/catalog/index.c:3099
> #17 0x0000aaaab8074514 in index_concurrently_build
> (heapRelationId=41578, indexRelationId=41993) at
> ../src/backend/catalog/index.c:1543
>
> That's this line:
>
> #9 0x0000aaaab7d3f2e4 [PAC] in GinBufferKeyEquals
> (buffer=0xaaaacaee4330, tup=0xaaaacaed29f8) at
> ../src/backend/access/gin/gininsert.c:1382
> 1382 tupkey = (buffer->typbyval) ? *(Datum *) tup->data :
> PointerGetDatum(tup->data);
>
> So we have a hidden assumption that 'data' is Datum-aligned.
>
>
> In _gin_parse_tuple_key() we do this instead:
>
> Datum key;
> ...
> if (a->typbyval)
> {
> memcpy(&key, a->data, a->keylen);
> return key;
> }
>
> That one doesn't require the alignment. I would be inclined to always
> use memcpy() when 'typbyval==true', as above, to not be sensitive to the
> alignment. However, I think we assume that it's aligned for the
> 'typbyval==false' case anyway, as we just do DatumGetPoint(a->data).
Good catch, and this is really surfaced by widening keylen: on master
GinTuple.data lands at offset 16, which is MAXALIGN'd, so that read is
(accidentally) fine; growing the header pushed data off an 8-byte
boundary and exposed the unaligned Datum load.
Rather than pad data back to MAXALIGN (which grows every GinTuple), I did
what you suggest here -- read the key via the existing
_gin_parse_tuple_key() helper, which already copies byval keys out with
memcpy() and so makes no alignment assumption. That also removes the
duplicated "byval ? deref : pointer" logic, so the key is now read the
same way everywhere; the byref branch is unchanged.
With the _gin_parse_tuple_key() change the sanitizer is clean -- both at that
4-aligned offset and with Size (where data happens to be back to
MAXALIGN'd), so the fix doesn't depend on the realignment.
>
> The straightforward fix is to add padding to make 'data' MAXALIGNed. It
> makes GinTuples larger, which is bad for performance, but it's probably
> fine.
>
> That said, I actually wonder why we need to store 'typbyval' and
> 'typlen' in GinTuple at all. That information could be looked up using
> 'attrnum'. Maybe 'typbyval' is good for performance in the comparison
> functions, but AFAICS GinTuple->typbyval is only used to copy it into
> GinBuffer in GinBufferStoreTuple(), which I think could easily afford to
> look it up.
I like the idea, but they're not only used to seed GinBuffer -- typbyval
is also read on the sort's hot path, in _gin_parse_tuple_key() (and thus
_gin_compare_tuples()), which only receives the GinTuple, not the index
tupdesc. So dropping them means threading the attr metadata into the
tuplesort comparator, which felt like a larger, separable cleanup than
this fix. Happy to look at it as a follow-up if you think it's
worthwhile, but I'd lean toward not blocking the bug fix on it.
>
> >>> 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
>
> Oh, ok, I stand corrected. Let's keep that working then.
Thanks -- leaving GinFormTuple() as the single size gate, then.
> Size (or size_t) is the correct type for sizes of objects in memory.
>
> Note that the return type of VARSIZE_ANY() is already Size, so by using
> int you are still doing a type truncation, and by using a signed type
> you are introducing unnecessary potential for confusion.
Agreed, that's clearly better. v3 (attached) uses Size for
GinTuple.keylen (GinBuffer.keylen already was Size), and also for the
local in _gin_build_tuple(), which was the int that truncated
VARSIZE_ANY() in the first place.
Thanks again for the review!
>
> - Heikki
>
--
Regards,
Ewan Young
Attachments:
[application/octet-stream] v3-0001-Fix-parallel-GIN-index-build-with-keys-larger-tha.patch (3.7K, ../../CAON2xHN5qGXcn1Z00pQAogf4y5rCQ1K04yvrYAORBa5i+ucLBQ@mail.gmail.com/2-v3-0001-Fix-parallel-GIN-index-build-with-keys-larger-tha.patch)
download | inline diff:
From 2e2d5c5cec2cefd04ae64486a8333e4ed4191112 Mon Sep 17 00:00:00 2001
From: Ewan Young <kdbase.hack@gmail.com>
Date: Thu, 9 Jul 2026 18:19:51 +0800
Subject: [PATCH v3] 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 keylen holding VARSIZE_ANY()
of the key. 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 GinTuple.keylen to Size so the stored length matches what
_gin_build_tuple() computes. Size is the natural type here: it's what
VARSIZE_ANY() returns and what GinBuffer.keylen already uses. The
_gin_build_tuple() local was int, which truncated VARSIZE_ANY() the
same way; make it Size too.
Widening keylen also moves GinTuple.data, which happened to land at an
8-byte boundary before. While the Size layout keeps data MAXALIGNed,
GinBufferKeyEquals() should not depend on that: it read an unaligned
Datum via "*(Datum *) tup->data" for byval keys. Use
_gin_parse_tuple_key() there instead, which copies the key out with
memcpy() and so makes no assumption about the alignment of the data
array, matching how the key is read everywhere else.
Bug: #19545
Reported-by: Yuelin Wang <1217816127@qq.com>
Discussion: https://postgr.es/m/19545-0f25b7e47351e8fc@postgresql.org
---
src/backend/access/gin/gininsert.c | 10 ++++++----
src/include/access/gin_tuple.h | 2 +-
2 files changed, 7 insertions(+), 5 deletions(-)
diff --git a/src/backend/access/gin/gininsert.c b/src/backend/access/gin/gininsert.c
index cb9ed3b563c..53d2a3f9f70 100644
--- a/src/backend/access/gin/gininsert.c
+++ b/src/backend/access/gin/gininsert.c
@@ -1376,10 +1376,12 @@ GinBufferKeyEquals(GinBuffer *buffer, GinTuple *tup)
return true;
/*
- * For the tuple, get either the first sizeof(Datum) bytes for byval
- * types, or a pointer to the beginning of the data array.
+ * Get the key from the tuple. We use _gin_parse_tuple_key() rather than
+ * reading tup->data directly, because for byval types the data is only
+ * stored/read with memcpy() (the data array is not guaranteed to be
+ * aligned enough to dereference as a Datum).
*/
- tupkey = (buffer->typbyval) ? *(Datum *) tup->data : PointerGetDatum(tup->data);
+ tupkey = _gin_parse_tuple_key(tup);
r = ApplySortComparator(buffer->key, false,
tupkey, false,
@@ -2248,7 +2250,7 @@ _gin_build_tuple(OffsetNumber attrnum, unsigned char category,
char *ptr;
Size tuplen;
- int keylen;
+ Size keylen;
dlist_mutable_iter iter;
dlist_head segments;
diff --git a/src/include/access/gin_tuple.h b/src/include/access/gin_tuple.h
index 7bde05e2de4..99a31578ac1 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 */
+ Size 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
^ permalink raw reply [nested|flat] 14+ messages in thread
* Re: BUG #19545: Integer truncation of `GinTuple.keylen` causes out-of-bounds read in parallel GIN index build
@ 2026-07-09 05:28 Tom Lane <tgl@sss.pgh.pa.us>
parent: Ewan Young <kdbase.hack@gmail.com>
1 sibling, 1 reply; 14+ messages in thread
From: Tom Lane @ 2026-07-09 05:28 UTC (permalink / raw)
To: Ewan Young <kdbase.hack@gmail.com>; +Cc: Heikki Linnakangas <hlinnaka@iki.fi>; Peter Eisentraut <peter@eisentraut.org>; 1217816127@qq.com; pgsql-bugs@lists.postgresql.org
Ewan Young <kdbase.hack@gmail.com> writes:
> Agreed, that's clearly better. v3 (attached) uses Size for
> GinTuple.keylen (GinBuffer.keylen already was Size), and also for the
> local in _gin_build_tuple(), which was the int that truncated
> VARSIZE_ANY() in the first place.
Am I reading this correctly that you propose using Size for the
length of the key value (keylen) along with int for the length of the
whole tuple (tuplen)?
{
int tuplen; /* length of the whole tuple */
OffsetNumber attrnum; /* attnum of index key */
- uint16 keylen; /* bytes in data for key value */
+ Size keylen; /* bytes in data for key value */
int16 typlen; /* typlen for key */
bool typbyval; /* typbyval for key */
signed char category; /* category: normal or NULL? */
Please explain how that's sane.
I kind of agree with the upthread comment that we should just reject
key lengths exceeding BLCKSZ or so up-front, rather than fooling
around with these field widths. This patch widens GinTuple
noticeably, and will do so more if we also widen tuplen. Is that
free?
regards, tom lane
^ permalink raw reply [nested|flat] 14+ messages in thread
* Re: BUG #19545: Integer truncation of `GinTuple.keylen` causes out-of-bounds read in parallel GIN index build
@ 2026-07-09 08:32 Ewan Young <kdbase.hack@gmail.com>
parent: Tom Lane <tgl@sss.pgh.pa.us>
0 siblings, 0 replies; 14+ messages in thread
From: Ewan Young @ 2026-07-09 08:32 UTC (permalink / raw)
To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: Heikki Linnakangas <hlinnaka@iki.fi>; Peter Eisentraut <peter@eisentraut.org>; 1217816127@qq.com; pgsql-bugs@lists.postgresql.org
Hi Tom,
Thanks for taking a look at this, and for pushing back on the field widths.
It made me reconsider the whole approach.
On Thu, Jul 9, 2026 at 1:28 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
>
> Ewan Young <kdbase.hack@gmail.com> writes:
> > Agreed, that's clearly better. v3 (attached) uses Size for
> > GinTuple.keylen (GinBuffer.keylen already was Size), and also for the
> > local in _gin_build_tuple(), which was the int that truncated
> > VARSIZE_ANY() in the first place.
>
> Am I reading this correctly that you propose using Size for the
> length of the key value (keylen) along with int for the length of the
> whole tuple (tuplen)?
>
> {
> int tuplen; /* length of the whole tuple */
> OffsetNumber attrnum; /* attnum of index key */
> - uint16 keylen; /* bytes in data for key value */
> + Size keylen; /* bytes in data for key value */
> int16 typlen; /* typlen for key */
> bool typbyval; /* typbyval for key */
> signed char category; /* category: normal or NULL? */
>
> Please explain how that's sane.
It isn't -- you're right, and let me back up and lay out the root cause,
because I think it also settles which way to fix it.
In a parallel build each key is serialized into a transient GinTuple for
the tuplesort. _gin_build_tuple() computes the key length into a
full-width local, and lays out the whole tuple from it -- the allocation,
the key copy, and the offset to the posting list that follows the key.
But it then stores that length in GinTuple.keylen, which is uint16, so
for a key longer than 65535 bytes the stored length is truncated.
On read-back (during the merge in _gin_process_worker_data() /
_gin_parallel_merge()), GinTupleGetFirst() and _gin_parse_tuple_items()
recompute the posting-list offset as SHORTALIGN(offsetof(data) + keylen)
using the *truncated* keylen. That lands ~64kB too early, inside the key
bytes, and ginPostingListDecodeAllSegments() then decodes those bytes as
a posting list -- tripping the OffsetNumberIsValid() assert with
assertions on, or reading past the allocation without.
The key point is that this only happens in a parallel build, because only
the parallel path materializes a GinTuple; a serial build inserts the key
straight into the index via GinFormTuple(). So for a >65535-byte key the
two builds already diverge today:
serial: indexes it if it compresses, else clean "index row size
... exceeds maximum"
parallel: crashes
A parallel build is supposed to be equivalent to a serial one -- it's an
optimization that should be transparent to the user, not something that
changes whether CREATE INDEX succeeds.
>
> I kind of agree with the upthread comment that we should just reject
> key lengths exceeding BLCKSZ or so up-front, rather than fooling
I looked into that, and it runs into the equivalence point above, plus
two others:
* _gin_build_tuple() only runs in the parallel path, so a reject there
makes a parallel build refuse keys a serial build indexes fine -- i.e.
it replaces the crash with a *different* serial/parallel divergence,
where the same CREATE INDEX on the same data succeeds or fails
depending on max_parallel_maintenance_workers.
* It would reject keys that serial builds accept today. The real limit is
enforced by GinFormTuple() on the *compressed* tuple, so a
large-but-compressible key is valid and indexes fine:
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, 100000-byte key
Heikki reached the same conclusion earlier in the thread.
* The threshold would be arbitrary: at _gin_build_tuple() time the key is
necessarily uncompressed (it has to stay uncompressed so the tuplesort
comparator can compare key values), so an uncompressed-length cap
doesn't correspond to the GinMaxItemSize limit, which only applies
after compression.
To avoid the divergence we'd have to apply the cap in the serial path
too, which changes long-standing behavior (rejecting keys that index fine
today).
> around with these field widths. This patch widens GinTuple
> noticeably, and will do so more if we also widen tuplen. Is that
> free?
Not free, but small: with keylen as int it's +4 bytes per GinTuple
(offsetof(data) 16 -> 20), tuplen stays int so nothing widens beyond
that, and there's no per-tuple runtime cost.
>
> regards, tom lane
--
Regards,
Ewan Young
^ permalink raw reply [nested|flat] 14+ messages in thread
* Re: BUG #19545: Integer truncation of `GinTuple.keylen` causes out-of-bounds read in parallel GIN index build
@ 2026-09-04 10:25 Peter Eisentraut <peter@eisentraut.org>
parent: Ewan Young <kdbase.hack@gmail.com>
1 sibling, 1 reply; 14+ messages in thread
From: Peter Eisentraut @ 2026-09-04 10:25 UTC (permalink / raw)
To: Ewan Young <kdbase.hack@gmail.com>; Heikki Linnakangas <hlinnaka@iki.fi>; +Cc: 1217816127@qq.com; pgsql-bugs@lists.postgresql.org
On 09.07.26 07:16, Ewan Young wrote:
>> So we have a hidden assumption that 'data' is Datum-aligned.
>>
>>
>> In _gin_parse_tuple_key() we do this instead:
>>
>> Datum key;
>> ...
>> if (a->typbyval)
>> {
>> memcpy(&key, a->data, a->keylen);
>> return key;
>> }
>>
>> That one doesn't require the alignment. I would be inclined to always
>> use memcpy() when 'typbyval==true', as above, to not be sensitive to the
>> alignment. However, I think we assume that it's aligned for the
>> 'typbyval==false' case anyway, as we just do DatumGetPoint(a->data).
> Good catch, and this is really surfaced by widening keylen: on master
> GinTuple.data lands at offset 16, which is MAXALIGN'd, so that read is
> (accidentally) fine; growing the header pushed data off an 8-byte
> boundary and exposed the unaligned Datum load.
>
> Rather than pad data back to MAXALIGN (which grows every GinTuple), I did
> what you suggest here -- read the key via the existing
> _gin_parse_tuple_key() helper, which already copies byval keys out with
> memcpy() and so makes no alignment assumption. That also removes the
> duplicated "byval ? deref : pointer" logic, so the key is now read the
> same way everywhere; the byref branch is unchanged.
>
> With the _gin_parse_tuple_key() change the sanitizer is clean -- both at that
> 4-aligned offset and with Size (where data happens to be back to
> MAXALIGN'd), so the fix doesn't depend on the realignment.
In don't think the use of _gin_parse_tuple_key() is sufficient because
it only handles the byval side, but the byref side can still fail
because of misalignment. Fixing that by copying out byref values as
well seems like a more extensive change.
The easiest fix (considering backpatching) is that we ensure that
GinTuple.data is maxaligned. On 64-bit platforms, we get that by
changing keylen to type Size, and that's the correct type anyway
relative to the surrounding code, so that seems sound. There is no
point in trying for a smaller type (like uint32); that wouldn't buy
anything unless you want to consider more extensive surgery in that struct.
(I suggest leaving the type of the .typlen field out of this discussion.
It would be more correct to use Size for that as well, considering the
surrounding code. But then we might realize that .nitems is also of the
wrong type, and the types of GinTuple and GinBuffer don't match
completely, and so on. Let's leave that for another day.)
On 32-bit platforms, it's more complicated because some of them have
MAXIMUM_ALIGNOF 4 and some 8. So using a 4-byte Size would make the
offset of .data 20 but that wouldn't work on platforms with
MAXIMUM_ALIGNOF == 8.
In PG19 and later we can force the alignment directly using
alignas(MAXIMUM_ALIGNOF) char data[FLEXIBLE_ARRAY_MEMBER];
In PG18, we could either force the alignment using some union trick, or
we could brute-force the issue by making keylen of type uint64, possibly
combined with a static assertion about the alignment of the .data field
somewhere.
I would prefer the union trick. That seems consistent with how
alignment is forced elsewhere (before the introduction of alignas).
(The code is new in PG18, commit 8492feb98f6.)
^ permalink raw reply [nested|flat] 14+ messages in thread
* Re: BUG #19545: Integer truncation of `GinTuple.keylen` causes out-of-bounds read in parallel GIN index build
@ 2026-09-07 08:36 Ewan Young <kdbase.hack@gmail.com>
parent: Peter Eisentraut <peter@eisentraut.org>
0 siblings, 1 reply; 14+ messages in thread
From: Ewan Young @ 2026-09-07 08:36 UTC (permalink / raw)
To: Peter Eisentraut <peter@eisentraut.org>; +Cc: Heikki Linnakangas <hlinnaka@iki.fi>; 1217816127@qq.com; pgsql-bugs@lists.postgresql.org
Hi Peter,
Thanks for picking this up again, and for the detailed guidance.
On Fri, Sep 4, 2026 at 6:25 PM Peter Eisentraut <peter@eisentraut.org> wrote:
>
> On 09.07.26 07:16, Ewan Young wrote:
> >> So we have a hidden assumption that 'data' is Datum-aligned.
> >>
> >>
> >> In _gin_parse_tuple_key() we do this instead:
> >>
> >> Datum key;
> >> ...
> >> if (a->typbyval)
> >> {
> >> memcpy(&key, a->data, a->keylen);
> >> return key;
> >> }
> >>
> >> That one doesn't require the alignment. I would be inclined to always
> >> use memcpy() when 'typbyval==true', as above, to not be sensitive to the
> >> alignment. However, I think we assume that it's aligned for the
> >> 'typbyval==false' case anyway, as we just do DatumGetPoint(a->data).
> > Good catch, and this is really surfaced by widening keylen: on master
> > GinTuple.data lands at offset 16, which is MAXALIGN'd, so that read is
> > (accidentally) fine; growing the header pushed data off an 8-byte
> > boundary and exposed the unaligned Datum load.
> >
> > Rather than pad data back to MAXALIGN (which grows every GinTuple), I did
> > what you suggest here -- read the key via the existing
> > _gin_parse_tuple_key() helper, which already copies byval keys out with
> > memcpy() and so makes no alignment assumption. That also removes the
> > duplicated "byval ? deref : pointer" logic, so the key is now read the
> > same way everywhere; the byref branch is unchanged.
> >
> > With the _gin_parse_tuple_key() change the sanitizer is clean -- both at that
> > 4-aligned offset and with Size (where data happens to be back to
> > MAXALIGN'd), so the fix doesn't depend on the realignment.
>
> In don't think the use of _gin_parse_tuple_key() is sufficient because
> it only handles the byval side, but the byref side can still fail
> because of misalignment. Fixing that by copying out byref values as
> well seems like a more extensive change.
Good point, I had only looked at the byval side. Dropped that hunk.
>
> The easiest fix (considering backpatching) is that we ensure that
> GinTuple.data is maxaligned. On 64-bit platforms, we get that by
Done in the attached v5, as two patches:
master: keylen (and the local in _gin_build_tuple()) becomes Size, and
data gets alignas(MAXIMUM_ALIGNOF). I put alignas between the type and
the name, because pgindent mangles a leading alignas on a struct member
that isn't the first one (the existing alignas members in the tree all
are); pg_crc32c_armv8.c has precedent for that ordering. Easy to flip
if you'd rather.
REL_18_STABLE: the union variant. A flexible array member isn't
allowed in a union, so the union goes on keylen instead, with the
PGAlignedBlock members:
union
{
Size keylen;
double force_align_d;
int64 force_align_i64;
} k;
That makes it 8 bytes and MAXALIGN'ed even where Size is 4, and the
fields after it add up to 8 bytes, so data lands at offset 24 on every
platform. A StaticAssertDecl on offsetof(GinTuple, data) guards that.
If you'd rather have the uint64 variant, I'm happy to switch, it's a
quick change.
typlen etc. left alone, as suggested.
One correction to my earlier reply to Tom: with Size the header grows
from 16 to 24 bytes on 64-bit, i.e. 8 bytes per GinTuple, not 4.
Tested both branches with -fsanitize=alignment,undefined: the reported
case now builds and passes gin_index_check(); an incompressible 128kB
key fails with the same error in serial and parallel builds; parallel
builds over int4/int8/float8/text/numeric/timestamp arrays and jsonb
are amcheck-clean and match seqscan results; make check passes with no
sanitizer reports.
Thanks again for the review.
> changing keylen to type Size, and that's the correct type anyway
> relative to the surrounding code, so that seems sound. There is no
> point in trying for a smaller type (like uint32); that wouldn't buy
> anything unless you want to consider more extensive surgery in that struct.
>
> (I suggest leaving the type of the .typlen field out of this discussion.
> It would be more correct to use Size for that as well, considering the
> surrounding code. But then we might realize that .nitems is also of the
> wrong type, and the types of GinTuple and GinBuffer don't match
> completely, and so on. Let's leave that for another day.)
>
> On 32-bit platforms, it's more complicated because some of them have
> MAXIMUM_ALIGNOF 4 and some 8. So using a 4-byte Size would make the
> offset of .data 20 but that wouldn't work on platforms with
> MAXIMUM_ALIGNOF == 8.
>
> In PG19 and later we can force the alignment directly using
>
> alignas(MAXIMUM_ALIGNOF) char data[FLEXIBLE_ARRAY_MEMBER];
>
> In PG18, we could either force the alignment using some union trick, or
> we could brute-force the issue by making keylen of type uint64, possibly
> combined with a static assertion about the alignment of the .data field
> somewhere.
>
> I would prefer the union trick. That seems consistent with how
> alignment is forced elsewhere (before the introduction of alignas).
>
> (The code is new in PG18, commit 8492feb98f6.)
>
--
Regards,
Ewan Young
Attachments:
[application/octet-stream] v5-0001-Fix-parallel-GIN-index-build-with-keys-larger-than-65535-bytes-master.patch (3.3K, ../../CAON2xHOv4M3rcLzhRcduSMqqmBgzWpZOR9TsK71_agAPNgi=Tg@mail.gmail.com/2-v5-0001-Fix-parallel-GIN-index-build-with-keys-larger-than-65535-bytes-master.patch)
download | inline diff:
From 22e3bd831552aa648f85176918d3a0e1de802d20 Mon Sep 17 00:00:00 2001
From: Ewan Young <kdbase.hack@gmail.com>
Date: Mon, 7 Sep 2026 23:22:24 +0800
Subject: [PATCH v5] 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. 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.
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. Make that explicit with
alignas(MAXIMUM_ALIGNOF) on data, so that the layout stays correct on
32-bit platforms, where Size is 4 bytes and would otherwise leave data
under-aligned when MAXIMUM_ALIGNOF is 8.
Bug: #19545
Reported-by: Yuelin Wang <1217816127@qq.com>
Discussion: https://postgr.es/m/19545-0f25b7e47351e8fc@postgresql.org
---
src/backend/access/gin/gininsert.c | 2 +-
src/include/access/gin_tuple.h | 9 +++++++--
2 files changed, 8 insertions(+), 3 deletions(-)
diff --git a/src/backend/access/gin/gininsert.c b/src/backend/access/gin/gininsert.c
index 37f689a2cac..aaef7020981 100644
--- a/src/backend/access/gin/gininsert.c
+++ b/src/backend/access/gin/gininsert.c
@@ -2250,7 +2250,7 @@ _gin_build_tuple(OffsetNumber attrnum, unsigned char category,
char *ptr;
Size tuplen;
- int keylen;
+ Size keylen;
dlist_mutable_iter iter;
dlist_head segments;
diff --git a/src/include/access/gin_tuple.h b/src/include/access/gin_tuple.h
index 7bde05e2de4..d2c7b50d1de 100644
--- a/src/include/access/gin_tuple.h
+++ b/src/include/access/gin_tuple.h
@@ -23,12 +23,17 @@ typedef struct GinTuple
{
int tuplen; /* length of the whole tuple */
OffsetNumber attrnum; /* attnum of index key */
- uint16 keylen; /* bytes in data for key value */
+ Size keylen; /* bytes in data for key value */
int16 typlen; /* typlen for key */
bool typbyval; /* typbyval for key */
signed char category; /* category: normal or NULL? */
int nitems; /* number of TIDs in the data */
- char data[FLEXIBLE_ARRAY_MEMBER];
+
+ /*
+ * The key value is accessed in place, so it must be aligned well enough
+ * for any key type.
+ */
+ char alignas(MAXIMUM_ALIGNOF) data[FLEXIBLE_ARRAY_MEMBER];
} GinTuple;
static inline ItemPointer
--
2.47.3
[application/octet-stream] v5-0001-Fix-parallel-GIN-index-build-with-keys-larger-than-65535-bytes-REL_18_STABLE.patch (5.2K, ../../CAON2xHOv4M3rcLzhRcduSMqqmBgzWpZOR9TsK71_agAPNgi=Tg@mail.gmail.com/3-v5-0001-Fix-parallel-GIN-index-build-with-keys-larger-than-65535-bytes-REL_18_STABLE.patch)
download | inline diff:
From 23c04b263ef298a56dd8dc974c77fa2a86132b2f Mon Sep 17 00:00:00 2001
From: Ewan Young <kdbase.hack@gmail.com>
Date: Mon, 7 Sep 2026 23:22:25 +0800
Subject: [PATCH v5] 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. 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.
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
Reported-by: Yuelin Wang <1217816127@qq.com>
Discussion: https://postgr.es/m/19545-0f25b7e47351e8fc@postgresql.org
---
src/backend/access/gin/gininsert.c | 12 ++++++------
src/include/access/gin_tuple.h | 21 +++++++++++++++++++--
2 files changed, 25 insertions(+), 8 deletions(-)
diff --git a/src/backend/access/gin/gininsert.c b/src/backend/access/gin/gininsert.c
index a575afacab3..fb660e4f0c4 100644
--- a/src/backend/access/gin/gininsert.c
+++ b/src/backend/access/gin/gininsert.c
@@ -1454,7 +1454,7 @@ GinBufferStoreTuple(GinBuffer *buffer, GinTuple *tup)
if (GinBufferIsEmpty(buffer))
{
buffer->category = tup->category;
- buffer->keylen = tup->keylen;
+ buffer->keylen = tup->k.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->k.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->k.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->k.keylen);
+ ptr = (char *) a + SHORTALIGN(offsetof(GinTuple, data) + a->k.keylen);
items = ginPostingListDecodeAllSegments((GinPostingList *) ptr, len, &ndecoded);
diff --git a/src/include/access/gin_tuple.h b/src/include/access/gin_tuple.h
index 702f7d12889..27da458650e 100644
--- a/src/include/access/gin_tuple.h
+++ b/src/include/access/gin_tuple.h
@@ -21,7 +21,21 @@ 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); together with the fields
+ * that follow it, this puts data at a MAXALIGN'ed offset, which the
+ * static assertion below verifies.
+ */
+ union
+ {
+ Size keylen; /* bytes in data for key value */
+ double force_align_d;
+ int64 force_align_i64;
+ } k;
int16 typlen; /* typlen for key */
bool typbyval; /* typbyval for key */
signed char category; /* category: normal or NULL? */
@@ -29,12 +43,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->k.keylen);
return &list->first;
}
--
2.47.3
^ permalink raw reply [nested|flat] 14+ messages in thread
* Re: BUG #19545: Integer truncation of `GinTuple.keylen` causes out-of-bounds read in parallel GIN index build
@ 2026-09-14 16:03 Peter Eisentraut <peter@eisentraut.org>
parent: Ewan Young <kdbase.hack@gmail.com>
0 siblings, 1 reply; 14+ messages in thread
From: Peter Eisentraut @ 2026-09-14 16:03 UTC (permalink / raw)
To: Ewan Young <kdbase.hack@gmail.com>; +Cc: Heikki Linnakangas <hlinnaka@iki.fi>; 1217816127@qq.com; pgsql-bugs@lists.postgresql.org
I have committed these, thanks.
On 07.09.26 10:36, Ewan Young wrote:
> Hi Peter,
>
> Thanks for picking this up again, and for the detailed guidance.
>
> On Fri, Sep 4, 2026 at 6:25 PM Peter Eisentraut <peter@eisentraut.org> wrote:
>>
>> On 09.07.26 07:16, Ewan Young wrote:
>>>> So we have a hidden assumption that 'data' is Datum-aligned.
>>>>
>>>>
>>>> In _gin_parse_tuple_key() we do this instead:
>>>>
>>>> Datum key;
>>>> ...
>>>> if (a->typbyval)
>>>> {
>>>> memcpy(&key, a->data, a->keylen);
>>>> return key;
>>>> }
>>>>
>>>> That one doesn't require the alignment. I would be inclined to always
>>>> use memcpy() when 'typbyval==true', as above, to not be sensitive to the
>>>> alignment. However, I think we assume that it's aligned for the
>>>> 'typbyval==false' case anyway, as we just do DatumGetPoint(a->data).
>>> Good catch, and this is really surfaced by widening keylen: on master
>>> GinTuple.data lands at offset 16, which is MAXALIGN'd, so that read is
>>> (accidentally) fine; growing the header pushed data off an 8-byte
>>> boundary and exposed the unaligned Datum load.
>>>
>>> Rather than pad data back to MAXALIGN (which grows every GinTuple), I did
>>> what you suggest here -- read the key via the existing
>>> _gin_parse_tuple_key() helper, which already copies byval keys out with
>>> memcpy() and so makes no alignment assumption. That also removes the
>>> duplicated "byval ? deref : pointer" logic, so the key is now read the
>>> same way everywhere; the byref branch is unchanged.
>>>
>>> With the _gin_parse_tuple_key() change the sanitizer is clean -- both at that
>>> 4-aligned offset and with Size (where data happens to be back to
>>> MAXALIGN'd), so the fix doesn't depend on the realignment.
>>
>> In don't think the use of _gin_parse_tuple_key() is sufficient because
>> it only handles the byval side, but the byref side can still fail
>> because of misalignment. Fixing that by copying out byref values as
>> well seems like a more extensive change.
>
> Good point, I had only looked at the byval side. Dropped that hunk.
>
>>
>> The easiest fix (considering backpatching) is that we ensure that
>> GinTuple.data is maxaligned. On 64-bit platforms, we get that by
>
> Done in the attached v5, as two patches:
>
> master: keylen (and the local in _gin_build_tuple()) becomes Size, and
> data gets alignas(MAXIMUM_ALIGNOF). I put alignas between the type and
> the name, because pgindent mangles a leading alignas on a struct member
> that isn't the first one (the existing alignas members in the tree all
> are); pg_crc32c_armv8.c has precedent for that ordering. Easy to flip
> if you'd rather.
>
> REL_18_STABLE: the union variant. A flexible array member isn't
> allowed in a union, so the union goes on keylen instead, with the
> PGAlignedBlock members:
>
> union
> {
> Size keylen;
> double force_align_d;
> int64 force_align_i64;
> } k;
>
> That makes it 8 bytes and MAXALIGN'ed even where Size is 4, and the
> fields after it add up to 8 bytes, so data lands at offset 24 on every
> platform. A StaticAssertDecl on offsetof(GinTuple, data) guards that.
> If you'd rather have the uint64 variant, I'm happy to switch, it's a
> quick change.
>
> typlen etc. left alone, as suggested.
>
> One correction to my earlier reply to Tom: with Size the header grows
> from 16 to 24 bytes on 64-bit, i.e. 8 bytes per GinTuple, not 4.
>
> Tested both branches with -fsanitize=alignment,undefined: the reported
> case now builds and passes gin_index_check(); an incompressible 128kB
> key fails with the same error in serial and parallel builds; parallel
> builds over int4/int8/float8/text/numeric/timestamp arrays and jsonb
> are amcheck-clean and match seqscan results; make check passes with no
> sanitizer reports.
>
> Thanks again for the review.
>
>> changing keylen to type Size, and that's the correct type anyway
>> relative to the surrounding code, so that seems sound. There is no
>> point in trying for a smaller type (like uint32); that wouldn't buy
>> anything unless you want to consider more extensive surgery in that struct.
>>
>> (I suggest leaving the type of the .typlen field out of this discussion.
>> It would be more correct to use Size for that as well, considering the
>> surrounding code. But then we might realize that .nitems is also of the
>> wrong type, and the types of GinTuple and GinBuffer don't match
>> completely, and so on. Let's leave that for another day.)
>>
>> On 32-bit platforms, it's more complicated because some of them have
>> MAXIMUM_ALIGNOF 4 and some 8. So using a 4-byte Size would make the
>> offset of .data 20 but that wouldn't work on platforms with
>> MAXIMUM_ALIGNOF == 8.
>>
>> In PG19 and later we can force the alignment directly using
>>
>> alignas(MAXIMUM_ALIGNOF) char data[FLEXIBLE_ARRAY_MEMBER];
>>
>> In PG18, we could either force the alignment using some union trick, or
>> we could brute-force the issue by making keylen of type uint64, possibly
>> combined with a static assertion about the alignment of the .data field
>> somewhere.
>>
>> I would prefer the union trick. That seems consistent with how
>> alignment is forced elsewhere (before the introduction of alignas).
>>
>> (The code is new in PG18, commit 8492feb98f6.)
>>
>
>
^ permalink raw reply [nested|flat] 14+ messages in thread
* Re: BUG #19545: Integer truncation of `GinTuple.keylen` causes out-of-bounds read in parallel GIN index build
@ 2026-09-18 02:07 Tom Lane <tgl@sss.pgh.pa.us>
parent: Peter Eisentraut <peter@eisentraut.org>
0 siblings, 1 reply; 14+ messages in thread
From: Tom Lane @ 2026-09-18 02:07 UTC (permalink / raw)
To: Peter Eisentraut <peter@eisentraut.org>; +Cc: Ewan Young <kdbase.hack@gmail.com>; Heikki Linnakangas <hlinnaka@iki.fi>; 1217816127@qq.com; pgsql-bugs@lists.postgresql.org
Peter Eisentraut <peter@eisentraut.org> writes:
> I have committed these, thanks.
The ABI-compliance-checking buildfarm animals have all been unhappy
since this went in:
'struct GinTuple' changed:
type size changed from 16 to 24 (in bytes)
1 data member deletion:
'uint16 keylen', at offset 6 (in bytes)
1 data member insertion:
'union {Size keylen; double force_align_d; int64 force_align_i64;} u', at offset 8 (in bytes)
there are data member changes:
'int16 typlen' offset changed from 8 to 16 (in bytes) (by +8 bytes)
'bool typbyval' offset changed from 10 to 18 (in bytes) (by +8 bytes)
'signed char category' offset changed from 11 to 19 (in bytes) (by +8 bytes)
'int nitems' offset changed from 12 to 20 (in bytes) (by +8 bytes)
'char data[]' offset changed from 16 to 24 (in bytes) (by +8 bytes)
Is it really okay to change this struct in v18?
If so, the .abi-compliance-history files need to be updated.
regards, tom lane
^ permalink raw reply [nested|flat] 14+ messages in thread
* Re: BUG #19545: Integer truncation of `GinTuple.keylen` causes out-of-bounds read in parallel GIN index build
@ 2026-09-18 14:37 Peter Eisentraut <peter@eisentraut.org>
parent: Tom Lane <tgl@sss.pgh.pa.us>
0 siblings, 0 replies; 14+ messages in thread
From: Peter Eisentraut @ 2026-09-18 14:37 UTC (permalink / raw)
To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: Ewan Young <kdbase.hack@gmail.com>; Heikki Linnakangas <hlinnaka@iki.fi>; 1217816127@qq.com; pgsql-bugs@lists.postgresql.org
On 18.09.26 04:07, Tom Lane wrote:
> Peter Eisentraut <peter@eisentraut.org> writes:
>> I have committed these, thanks.
>
> The ABI-compliance-checking buildfarm animals have all been unhappy
> since this went in:
>
> 'struct GinTuple' changed:
> type size changed from 16 to 24 (in bytes)
> 1 data member deletion:
> 'uint16 keylen', at offset 6 (in bytes)
> 1 data member insertion:
> 'union {Size keylen; double force_align_d; int64 force_align_i64;} u', at offset 8 (in bytes)
> there are data member changes:
> 'int16 typlen' offset changed from 8 to 16 (in bytes) (by +8 bytes)
> 'bool typbyval' offset changed from 10 to 18 (in bytes) (by +8 bytes)
> 'signed char category' offset changed from 11 to 19 (in bytes) (by +8 bytes)
> 'int nitems' offset changed from 12 to 20 (in bytes) (by +8 bytes)
> 'char data[]' offset changed from 16 to 24 (in bytes) (by +8 bytes)
>
> Is it really okay to change this struct in v18?
> If so, the .abi-compliance-history files need to be updated.
Yes, I think this is okay. The type is effectively for internal use
only and is only used transiently. I will add the
.abi-compliance-history entry.
^ permalink raw reply [nested|flat] 14+ messages in thread
end of thread, other threads:[~2026-09-18 14:37 UTC | newest]
Thread overview: 14+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2026-07-07 14:13 BUG #19545: Integer truncation of `GinTuple.keylen` causes out-of-bounds read in parallel GIN index build PG Bug reporting form <noreply@postgresql.org>
2026-07-08 06:27 ` Ewan Young <kdbase.hack@gmail.com>
2026-07-08 07:52 ` Heikki Linnakangas <hlinnaka@iki.fi>
2026-07-08 11:34 ` Ewan Young <kdbase.hack@gmail.com>
2026-07-08 14:36 ` Heikki Linnakangas <hlinnaka@iki.fi>
2026-07-09 05:16 ` Ewan Young <kdbase.hack@gmail.com>
2026-07-09 05:28 ` Tom Lane <tgl@sss.pgh.pa.us>
2026-07-09 08:32 ` Ewan Young <kdbase.hack@gmail.com>
2026-09-04 10:25 ` Peter Eisentraut <peter@eisentraut.org>
2026-09-07 08:36 ` Ewan Young <kdbase.hack@gmail.com>
2026-09-14 16:03 ` Peter Eisentraut <peter@eisentraut.org>
2026-09-18 02:07 ` Tom Lane <tgl@sss.pgh.pa.us>
2026-09-18 14:37 ` Peter Eisentraut <peter@eisentraut.org>
2026-07-08 17:14 ` Peter Eisentraut <peter@eisentraut.org>
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox