| From: | Ewan Young <kdbase(dot)hack(at)gmail(dot)com> |
|---|---|
| To: | 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 06:27:55 |
| Message-ID: | CAON2xHMXYLwkq4UaNbUtmaw1ZKne6OqdyyiACFQUbCj+XRBvQQ@mail.gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-bugs |
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(at)postgresql(dot)org> wrote:
>
> The following bug has been logged on the website:
>
> Bug reference: 19545
> Logged by: Yuelin Wang
> Email address: 1217816127(at)qq(dot)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
> > UINT16_MAX`, so the parallel path fails as cleanly as the serial one.
>
>
>
>
--
Regards,
Ewan Young
| Attachment | Content-Type | Size |
|---|---|---|
| v1-0001-Widen-GinTuple.keylen-to-uint32-to-fix-parallel-G.patch | application/octet-stream | 2.4 KB |
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Peter Eisentraut | 2026-07-08 07:51:11 | Re: BUG #19542: Boolean syntax in GRAPH_TABLE |
| Previous Message | Michael Paquier | 2026-07-08 01:10:11 | Re: BUG #19519: REPACK can fail due to missing chunk for toast value |