Feedback-ID: ie0a040ee:Fastmail
Message-ID: <53f1faeb-72a4-4180-b2cd-88d524003b9d@eisentraut.org>
Date: Wed, 8 Jul 2026 19:14:08 +0200
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: BUG #19545: Integer truncation of `GinTuple.keylen` causes
 out-of-bounds read in parallel GIN index build
To: Ewan Young <kdbase.hack@gmail.com>, Heikki Linnakangas <hlinnaka@iki.fi>
Cc: 1217816127@qq.com, pgsql-bugs@lists.postgresql.org
References: <19545-0f25b7e47351e8fc@postgresql.org>
 <CAON2xHMXYLwkq4UaNbUtmaw1ZKne6OqdyyiACFQUbCj+XRBvQQ@mail.gmail.com>
 <d02fd089-b326-417a-ab2c-aea88e459c63@iki.fi>
 <CAON2xHPV-7O0MsFuizJv8cbUG7ZJfLCAqzkGYgoxF9WBe1v9Jg@mail.gmail.com>
Content-Language: en-US
From: Peter Eisentraut <peter@eisentraut.org>
In-Reply-To: 
 <CAON2xHPV-7O0MsFuizJv8cbUG7ZJfLCAqzkGYgoxF9WBe1v9Jg@mail.gmail.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 8bit
Archived-At: 
 <https://www.postgresql.org/message-id/53f1faeb-72a4-4180-b2cd-88d524003b9d%40eisentraut.org>
Precedence: bulk

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.