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