Re: BUG #18497: Heap-use-after-free in plpgsql

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: n(dot)kalinin(at)postgrespro(dot)ru
Cc: pgsql-bugs(at)lists(dot)postgresql(dot)org
Subject: Re: BUG #18497: Heap-use-after-free in plpgsql
Date: 2024-06-12 23:24:46
Message-ID: 1694260.1718234686@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-bugs

PG Bug reporting form <noreply(at)postgresql(dot)org> writes:
> When building postgresql on REL_16_STABLE tag with ASAN assertion error:

Thanks for the report!

> Two sql files are required:

This can actually be reproduced more simply: you don't need two
sessions, because it's purely straight-line behavior. You don't
need ASAN either, as a plain old --enable-cassert (or more
specifically -DCLOBBER_FREED_MEMORY) build will show it too --
or at least it does on HEAD:

regression=# select f1();
ERROR: unexpected plan node type: 328
CONTEXT: PL/pgSQL function f1() line 4 at assignment
regression=# select f1();
ERROR: unrecognized node type: 2139062143
CONTEXT: PL/pgSQL function f1() line 4 at assignment

It appears to me that there are two independent bugs here.

One is that exec_eval_simple_expr() assumes, when revalidating
a simple-expression plan, that it doesn't have to recheck the
"purely syntactic" tests made in exec_simple_check_plan().
I think that's mostly correct, but it's wrong at least with
respect to checking query->hasTargetSRFs: this test case shows
how to break that. You could probably cause query->hasAggs to
change in the same way. On the whole it seems safest to
refactor so that we recheck all of those things.

Right now, since we aren't rechecking that, we end up trying to
apply exec_save_simple_expr to a plan tree containing a ProjectSet
node, which it isn't expecting, leading to the "unexpected plan
node type" error. And then we have the second bug: longjmp'ing
out of this leaves behind some dangling pointers, which is what
causes the "unrecognized node type: 2139062143" error (or the ASAN
complaint) during the second call. So we need to be more careful
about leaving a clean state in case we fail out of here.

The attached seems to be enough to fix this. I verified the
second aspect by leaving the new exec_is_simple_query(expr) test
out of exec_eval_simple_expr, in which case you still get the
"unexpected plan node type" error but things are OK on retries.

regards, tom lane

Attachment Content-Type Size
be-more-careful-in-simple-expression-revalidation.patch text/x-diff 10.1 KB

In response to

Browse pgsql-bugs by date

  From Date Subject
Next Message Tender Wang 2024-06-13 03:31:22 Re: BUG #18377: Assert false in "partdesc->nparts >= pinfo->nparts", fileName="execPartition.c", lineNumber=1943
Previous Message Thomas Munro 2024-06-12 21:54:13 Re: BUG #18503: Reproducible 'Segmentation fault' in 16.3 on ARM64