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

From: Peter Eisentraut <peter(at)eisentraut(dot)org>
To: Ewan Young <kdbase(dot)hack(at)gmail(dot)com>, Heikki Linnakangas <hlinnaka(at)iki(dot)fi>
Cc: 1217816127(at)qq(dot)com, pgsql-bugs(at)lists(dot)postgresql(dot)org
Subject: Re: BUG #19545: Integer truncation of `GinTuple.keylen` causes out-of-bounds read in parallel GIN index build
Date: 2026-07-08 17:14:08
Message-ID: 53f1faeb-72a4-4180-b2cd-88d524003b9d@eisentraut.org
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-bugs

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(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.

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.

In response to

Browse pgsql-bugs by date

  From Date Subject
Next Message surya poondla 2026-07-08 18:25:54 Re: BUG #19544: PostgreSQL 18.3 backend crashes with SIGSEGV when a PL/pgSQL function
Previous Message Imran Zaheer 2026-07-08 15:00:28 Re: BUG #19519: REPACK can fail due to missing chunk for toast value