BUG #19545: Integer truncation of `GinTuple.keylen` causes out-of-bounds read in parallel GIN index build
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
Show quoted text
UINT16_MAX`, so the parallel path fails as cleanly as the serial one.
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 `keylenUINT16_MAX`, so the parallel path fails as cleanly as the serial one.
--
Regards,
Ewan Young
Attachments:
v1-0001-Widen-GinTuple.keylen-to-uint32-to-fix-parallel-G.patchapplication/octet-stream; name=v1-0001-Widen-GinTuple.keylen-to-uint32-to-fix-parallel-G.patchDownload+1-2
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
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:
v2-0001-Fix-parallel-GIN-index-build-keys-larger-than-65535.patchapplication/octet-stream; name=v2-0001-Fix-parallel-GIN-index-build-keys-larger-than-65535.patchDownload+2-3
v2-0001-alt-Fix-parallel-GIN-index-build-fail-fast-variant.patchapplication/octet-stream; name=v2-0001-alt-Fix-parallel-GIN-index-build-fail-fast-variant.patchDownload+19-3
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 itThat 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
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.
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:1543That'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 itThat 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 bytesOh, 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:
v3-0001-Fix-parallel-GIN-index-build-with-keys-larger-tha.patchapplication/octet-stream; name=v3-0001-Fix-parallel-GIN-index-build-with-keys-larger-tha.patchDownload+7-6
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
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