Re: BUG #19545: Integer truncation of `GinTuple.keylen` causes out-of-bounds read in parallel GIN index build

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(at)iki(dot)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