From: Tom Lane <tgl@sss.pgh.pa.us>
To: demurgos@demurgos.net
Cc: pgsql-bugs@lists.postgresql.org
Subject: Re: BUG #17053: Memory corruption in parser on prepared query reuse
Date: Wed, 09 Jun 2021 17:41:36 -0400
Message-ID: <1509789.1623274896@sss.pgh.pa.us> (raw)
In-Reply-To: <17053-3ca3f501bbc212b4@postgresql.org>
References: <17053-3ca3f501bbc212b4@postgresql.org>
PG Bug reporting form <noreply@postgresql.org> writes:
> The code is fairly short, the core of the issue is that the prepared query
> `q1` is executed twice and it somehow messes up with the parser because of
> the `CHECK` clause.
Thanks for the report! The core of the problem is that where
transformExpr does this:
case T_NullTest:
{
NullTest *n = (NullTest *) expr;
n->arg = (Expr *) transformExprRecurse(pstate, (Node *) n->arg);
in the case of a prepared statement, it's scribbling on the same NullTest
node that's in the plan cache. So the next time we try to execute that
cache entry, the arg pointer is now pointing at garbage, or at least not
at the untransformed argument it should be pointing at.
Ideally, maybe, the parser wouldn't ever modify its input. However,
this code fragment has got LOTS of company, so making that happen has
never seemed too practical. So what we generally do, before invoking
the parser (or planner), is a quick copyObject() on any node tree that
might be coming from cache.
The proximate cause of this problem is that CREATE/ALTER DOMAIN did
not get that memo. Looking around in typecmds.c, I noted that
CREATE DOMAIN ... DEFAULT ... and ALTER DOMAIN SET DEFAULT have the
same kind of bug.
The attached patch seems to fix it. I wonder though if we ought to
move the copying to someplace more centralized. I cannot escape the
suspicion that there are more of these creepy-crawlies in other
poorly-tested utility statements.
BTW, a note for future testing: you can set up this failure using
pgbench, no custom client code required. I tested stuff like this:
$ cat bug17053b.sql
DROP DOMAIN IF EXISTS schema_meta;
CREATE DOMAIN schema_meta AS bool DEFAULT (42 IS NOT NULL);
$ pgbench -n -M prepared -f bug17053b.sql regression
NOTICE: type "schema_meta" does not exist, skipping
pgbench: error: client 0 script 0 aborted in command 1 query 0: ERROR: unrecognized node type: 2139062143
regards, tom lane
Attachments:copy-raw-expressions-in-domain-DDL.patch (text/x-diff, 1.9K) downloaddiff --git a/src/backend/commands/typecmds.c b/src/backend/commands/typecmds.cindex 58ec65c6af..9a1b261d85 100644--- a/src/backend/commands/typecmds.c+++ b/src/backend/commands/typecmds.c@@ -891,10 +891,12 @@ DefineDomain(CreateDomainStmt *stmt)
pstate = make_parsestate(NULL);
/*
- * Cook the constr->raw_expr into an expression. Note:- * name is strictly for error message+ * Cook the constr->raw_expr into an expression; copy it+ * in case the input is in plan cache. Note: name is used+ * only for error messages.
*/
- defaultExpr = cookDefault(pstate, constr->raw_expr,+ defaultExpr = cookDefault(pstate,+ copyObject(constr->raw_expr),
basetypeoid,
basetypeMod,
domainName,
@@ -2623,10 +2625,10 @@ AlterDomainDefault(List *names, Node *defaultRaw)
pstate = make_parsestate(NULL);
/*
- * Cook the colDef->raw_expr into an expression. Note: Name is- * strictly for error message+ * Cook the raw default into an expression; copy it in case the input+ * is in plan cache. Note: name is used only for error messages.
*/
- defaultExpr = cookDefault(pstate, defaultRaw,+ defaultExpr = cookDefault(pstate, copyObject(defaultRaw),
typTup->typbasetype,
typTup->typtypmod,
NameStr(typTup->typname),
@@ -3508,7 +3510,12 @@ domainAddConstraint(Oid domainOid, Oid domainNamespace, Oid baseTypeOid,
pstate->p_pre_columnref_hook = replace_domain_constraint_value;
pstate->p_ref_hook_state = (void *) domVal;
- expr = transformExpr(pstate, constr->raw_expr, EXPR_KIND_DOMAIN_CHECK);+ /*+ * Transform the expression; first we must copy the input, in case it's in+ * plan cache.+ */+ expr = transformExpr(pstate, copyObject(constr->raw_expr),+ EXPR_KIND_DOMAIN_CHECK);
/*
* Make sure it yields a boolean result.
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-bugs@postgresql.org
Cc: tgl@sss.pgh.pa.us, demurgos@demurgos.net, pgsql-bugs@lists.postgresql.org
Subject: Re: BUG #17053: Memory corruption in parser on prepared query reuse
In-Reply-To: <1509789.1623274896@sss.pgh.pa.us>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox