| From: | Richard Guo <guofenglinux(at)gmail(dot)com> |
|---|---|
| To: | Jeevan Chalke <jeevan(dot)chalke(at)enterprisedb(dot)com> |
| Cc: | PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org> |
| Subject: | Re: Use-after-free of a shared Path in add_path()/add_partial_path() |
| Date: | 2026-09-15 03:05:20 |
| Message-ID: | CAMbWs48fAcrCLqcs8bCbvozcM1-ricLe2bPmqu+o2POWo5WHqw@mail.gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
On Wed, Sep 9, 2026 at 7:09 PM Jeevan Chalke
<jeevan(dot)chalke(at)enterprisedb(dot)com> wrote:
> Note up front: the reproducer below (case 1) only crashes on REL_18_STABLE
> and earlier. On REL_19_STABLE, add_partial_path()'s dominance check was
> rewritten to compare startup cost as well as total cost, and that
> incidentally avoids the specific comparison that discards the shared path for
> this data. The underlying bug is still present on REL_19_STABLE; I just
> haven't found data that gets the new cost comparison to hit it.
I found a query that reproduces it on v19 and master:
set parallel_setup_cost = 0;
set parallel_tuple_cost = 0;
set min_parallel_table_scan_size = 0;
set min_parallel_index_scan_size = 0;
set max_parallel_workers_per_gather = 1;
explain (costs off)
select unique1, generate_series(1, 5000000) from tenk1 where unique2 < 100
union
select 1, 2;
ERROR: unrecognized node type: 3301232
The trick here is the SRF with a large fan-out: it adds the same large
per-row cost to both partial index scans, whose startup costs are
already equal, so their total costs end up within the fuzz factor.
When grouping_planner() re-submits them to final_rel, the sorted one
evicts the other and frees it, while the Gather built over it is still
final_rel's cheapest path.
So this bug is real on all branches, and I think we should fix it.
I think the parent_rel check approach in your 0001 works. The rule it
states is that add_path() and add_partial_path() free only paths built
for the rel they are building, which is what the freeing was always
relying on.
One consequence: postgres_fdw's add_foreign_ordered_paths() and
add_foreign_final_paths() build their upper paths with the input rel
as parent, since postgresGetForeignPlan() deparses from that rel.
Under the check those paths are never freed when they lose a
comparison. That is a node or two per query, so maybe it's ok.
The alternatives are a flat copy of Path, and reference counting (and
maybe more). Copying also fixes the bug, so maybe it would be a
workable solution too. Reference counting is more appealing in
principle, but I think it's far too large a change to back-patch.
Thoughts?
- Richard
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Nisha Moond | 2026-09-15 03:14:49 | Re: Crashes on a partition whose concurrent detach never finished |
| Previous Message | Bruce Momjian | 2026-09-15 03:00:55 | Re: First draft of PG 19 release notes |