Re: Use-after-free of a shared Path in add_path()/add_partial_path()

From: Jeevan Chalke <jeevan(dot)chalke(at)enterprisedb(dot)com>
To: 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-09 10:11:03
Message-ID: CAM2+6=VZ+gCtft=yL5NpBJ+MEWYQ0_-x2wPLfNWjkmP96xvKAg@mail.gmail.com
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Here is the Reproducer:

set parallel_setup_cost = 0;
set parallel_tuple_cost = 0;
set min_parallel_table_scan_size = 0;
set max_parallel_workers_per_gather = 2;

create table psrf_upload (
id int, crt_time timestamp, jdata jsonb, tag text
) partition by range (crt_time);
create table psrf_upload_dtl (
id int, upload_id int, crt_time timestamp, jdata jsonb
) partition by range (crt_time);

create table psrf_upload_p0 partition of psrf_upload for values from
('2024-01-01') to ('2024-02-01');
create table psrf_upload_p1 partition of psrf_upload for values from
('2024-02-01') to ('2024-03-01');
create table psrf_upload_p2 partition of psrf_upload for values from
('2024-03-01') to ('2024-04-01');
create table psrf_upload_p3 partition of psrf_upload for values from
('2024-04-01') to ('2024-05-01');
create table psrf_upload_p4 partition of psrf_upload for values from
('2024-05-01') to ('2024-06-01');
create table psrf_upload_p5 partition of psrf_upload for values from
('2024-06-01') to ('2024-07-01');
create table psrf_upload_p6 partition of psrf_upload for values from
('2024-07-01') to ('2024-08-01');

create table psrf_upload_dtl_p0 partition of psrf_upload_dtl for
values from ('2024-01-01') to ('2024-02-01');
create table psrf_upload_dtl_p1 partition of psrf_upload_dtl for
values from ('2024-02-01') to ('2024-03-01');
create table psrf_upload_dtl_p2 partition of psrf_upload_dtl for
values from ('2024-03-01') to ('2024-04-01');
create table psrf_upload_dtl_p3 partition of psrf_upload_dtl for
values from ('2024-04-01') to ('2024-05-01');
create table psrf_upload_dtl_p4 partition of psrf_upload_dtl for
values from ('2024-05-01') to ('2024-06-01');
create table psrf_upload_dtl_p5 partition of psrf_upload_dtl for
values from ('2024-06-01') to ('2024-07-01');
create table psrf_upload_dtl_p6 partition of psrf_upload_dtl for
values from ('2024-07-01') to ('2024-08-01');

insert into psrf_upload values
(1, '2024-05-01 00:00:00', jsonb_build_array(jsonb_build_object('v',
repeat('x', 1000))), 'tag1');
insert into psrf_upload_dtl
select id, id, crt_time, jsonb_build_array(jsonb_build_object('v',
repeat('x', 1000)))
from psrf_upload;

analyze psrf_upload;
analyze psrf_upload_dtl;

select id, tag, length(val) from (
select u.id, u.tag, jsonb_array_elements(u.jdata)->>'v' as val
from psrf_upload u
union
select u.id, u.tag, jsonb_array_elements(d.jdata)->>'v' as val
from psrf_upload u left join psrf_upload_dtl d on d.upload_id = u.id
) s;

On an --enable-cassert build this reliably fails with something like:

ERROR: unrecognized node type: 0

(the exact garbage number varies run to run, since it depends on
whatever reused the freed chunk).

At least 7 partitions per table are needed here to get enough
competing partial-path candidates for add_partial_path()'s own
dominance check to actually discard one that a Gather still depends
on; fewer partitions don't hit the conflict.

Thanks

On Wed, Sep 9, 2026 at 3:39 PM Jeevan Chalke <jeevan(dot)chalke(at)enterprisedb(dot)com>
wrote:

> Hi,
>
> While investigating a customer-reported planner crash ("ERROR: unrecognized
> node type: N"), I found a use-after-free bug in
> add_path()/add_partial_path()
> that turns out to be a generalization of an issue reported here in 2023,
> which
> stalled for lack of a self-contained reproducer:
>
>
> https://www.postgresql.org/message-id/CAM2%2B6%3DUC1mcVtM0Y_LEMBEGHTM58HEkqHPn7vau_V_YfuZjEGg%40mail.gmail.com
>
> This time I have a reproducer (below), and it turns out the same underlying
> defect shows up independently in two different places in the planner, so I
> think it's worth fixing generally rather than patching one call site.
>
> My teammate Shruthi internally reported the crash that got me looking at
> this
> again; once I saw it, the 3-year-old thread above came back to me, and I
> ended
> up fixing both. Thanks to her for the report.
>
> 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.
>
> The problem (see follow-up email for the reproducer)
> -----------
>
> add_path() and add_partial_path() pfree() a dominated old_path, or a
> rejected
> new_path, on the assumption documented in add_path()'s own header comment:
>
> "As noted in optimizer/README, deleting a previously-accepted Path
> is safe because we know that Paths of this rel cannot yet be
> referenced from any other rel, such as a higher-level join."
>
> Two existing callers violate that assumption by handing in a path that is
> still a live member of a *different* rel's pathlist/partial_pathlist,
> without
> copying it:
>
> 1. grouping_planner() exposes a scan/join rel's partial paths to the outer
> query level's final rel:
>
> foreach(lc, current_rel->partial_pathlist)
> {
> Path *partial_path = (Path *) lfirst(lc);
> add_partial_path(final_rel, partial_path);
> }
>
> without removing them from current_rel->partial_pathlist. If
> generate_useful_gather_paths() already built a Gather/Gather Merge path
> over one of those entries (it doesn't remove the entry from
> partial_pathlist either), and that Gather path has since been carried
> into
> final_rel->pathlist too, a later dominance comparison inside
> add_partial_path() can decide the shared partial path is dominated by
> some
> other promoted candidate and pfree() it -- leaving the Gather/Gather
> Merge
> path's subpath pointer dangling.
>
> This crashes planning of a UNION branch that joins parallel-safe
> relations
> and has a set-returning function in its target list.
>
> 2. create_ordered_paths() builds the ORDERED upperrel by, for each path in
> input_rel->pathlist that's already sorted per root->sort_pathkeys,
> passing
> that same Path pointer straight through:
>
> if (is_sorted)
> sorted_path = input_path;
> ...
> add_path(ordered_rel, sorted_path);
>
> input_path remains a live member of input_rel->pathlist throughout, so
> it's
> the same hazard. This is the one from the 2023 thread linked above; it's
> otherwise inert, since nothing else revisits the superseded input_rel's
> pathlist during normal planning, which is why it only ever showed up as
> sporadic "WARNING: could not dump unrecognized node type" while
> debugging
> postgres_fdw with an ad hoc elog(INFO, "foreignrel: %s",
> nodeToString(foreignrel)) dropped into postgresGetForeignPlan(). It's
> still
> reproducible today the same way, via contrib/postgres_fdw's own
> regression
> suite (the existing "subquery+MAX" test, unmodified) with that debug
> line
> temporarily restored.
>
> The fix
> -------
>
> Rather than special-case each known sharing pattern, 0001 adds one general
> check: every Path freshly built for parent_rel already has path->parent
> stamped to parent_rel by its create_*_path() constructor, before
> add_path()/add_partial_path() is ever called with that same rel. So a
> path->parent != parent_rel mismatch can only mean the path is on loan from
> its
> true owning rel, and pfree()'ing it here would leave that rel's list with a
> dangling entry. Skip the pfree() in that case, alongside the existing
> IndexPath exemption (which covers a different, same-rel sharing pattern --
> an
> IndexPath referenced as a child of a BitmapHeapPath -- that this check
> doesn't
> and needn't cover).
>
> No API changes: add_path() and add_partial_path() keep their existing
> signatures, and no caller needs to change.
>
> 0002 adds a regression test for the UNION+parallel+SRF crash (case 1
> above) to
> select_parallel.sql. I did not add one for the
> create_ordered_paths()/postgres_fdw case, since I couldn't find a way to
> make
> that corruption observable without adding debug code.
>
> Testing
> -------
>
> Developed against REL_18_STABLE (--enable-cassert, --enable-debug build):
> full
> regression suite passes, confirmed the fix addresses case 2 as
> well as case 1).
>
> Given this affects both add_path() and add_partial_path() and, per the 2023
> thread, has been present for a while, I'd guess this warrants a back-patch
> once a fix is agreed on, but I'll defer to reviewers on how far back makes
> sense.
>
> Thanks,
>
>

--
*Jeevan Chalke*
*Senior Principal Engineer, Engineering Manager*
*Product Development*

enterprisedb.com <https://www.enterprisedb.com>

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Nazir Bilal Yavuz 2026-09-09 10:12:45 Re: Speed up COPY FROM text/CSV parsing using SIMD
Previous Message Jeevan Chalke 2026-09-09 10:09:04 Use-after-free of a shared Path in add_path()/add_partial_path()