Replace max_expr_depth parameter with a max_stack_depth parameter that

is measured in kilobytes and checked against actual physical execution
stack depth, as per my proposal of 30-Dec.  This gives us a fairly
bulletproof defense against crashing due to runaway recursive functions.
This commit is contained in:
Tom Lane 2004-03-24 22:40:29 +00:00
parent a09b9a36d3
commit 8899a2aba9
13 changed files with 157 additions and 79 deletions

View File

@ -1,5 +1,5 @@
<!--
$PostgreSQL: pgsql/doc/src/sgml/runtime.sgml,v 1.253 2004/03/24 03:48:41 neilc Exp $
$PostgreSQL: pgsql/doc/src/sgml/runtime.sgml,v 1.254 2004/03/24 22:40:28 tgl Exp $
-->
<Chapter Id="runtime">
@ -889,6 +889,26 @@ SET ENABLE_SEQSCAN TO OFF;
</listitem>
</varlistentry>
<varlistentry id="guc-max-stack-depth" xreflabel="max_stack_depth">
<term><varname>max_stack_depth</varname> (<type>integer</type>)</term>
<listitem>
<para>
Specifies the maximum safe depth of the server's execution stack.
The ideal setting for this parameter is the actual stack size limit
enforced by the kernel (as set by <literal>ulimit -s</> or local
equivalent), less a safety margin of a megabyte or so. The safety
margin is needed because the stack depth is not checked in every
routine in the server, but only in key potentially-recursive routines
such as expression evaluation. Setting the parameter higher than
the actual kernel limit will mean that a runaway recursive function
can crash an individual backend process. The default setting is
2048 KB (two megabytes), which is conservatively small and unlikely
to risk crashes. However, it may be too small to allow execution
of complex functions.
</para>
</listitem>
</varlistentry>
</variablelist>
</sect3>
<sect3 id="runtime-config-resource-fsm">
@ -2573,18 +2593,6 @@ dynamic_library_path = '/usr/local/lib/postgresql:/home/my_project/lib:$libdir'
</listitem>
</varlistentry>
<varlistentry id="guc-max-expr-depth" xreflabel="max_expr_depth">
<term><varname>max_expr_depth</varname> (<type>integer</type>)</term>
<listitem>
<para>
Sets the maximum expression nesting depth of the parser. The
default value of 10000 is high enough for any normal query,
but you can raise it if needed. (But if you raise it too high,
you run the risk of server crashes due to stack overflow.)
</para>
</listitem>
</varlistentry>
</variablelist>
</sect3>
</sect2>

View File

@ -8,7 +8,7 @@
*
*
* IDENTIFICATION
* $PostgreSQL: pgsql/src/backend/executor/execQual.c,v 1.156 2004/03/17 20:48:42 tgl Exp $
* $PostgreSQL: pgsql/src/backend/executor/execQual.c,v 1.157 2004/03/24 22:40:28 tgl Exp $
*
*-------------------------------------------------------------------------
*/
@ -27,6 +27,11 @@
* trying to speed it up, the execution plan should be pre-processed
* to facilitate attribute sharing between nodes wherever possible,
* instead of doing needless copying. -cim 5/31/91
*
* During expression evaluation, we check_stack_depth only in
* ExecMakeFunctionResult rather than at every single node. This
* is a compromise that trades off precision of the stack limit setting
* to gain speed.
*/
#include "postgres.h"
@ -840,6 +845,9 @@ ExecMakeFunctionResult(FuncExprState *fcache,
bool hasSetArg;
int i;
/* Guard against stack overflow due to overly complex expressions */
check_stack_depth();
/*
* arguments is a list of expressions to evaluate before passing to
* the function manager. We skip the evaluation if it was already
@ -1058,6 +1066,9 @@ ExecMakeFunctionResultNoSets(FuncExprState *fcache,
FunctionCallInfoData fcinfo;
int i;
/* Guard against stack overflow due to overly complex expressions */
check_stack_depth();
if (isDone)
*isDone = ExprSingleResult;
@ -2503,6 +2514,10 @@ ExecInitExpr(Expr *node, PlanState *parent)
if (node == NULL)
return NULL;
/* Guard against stack overflow due to overly complex expressions */
check_stack_depth();
switch (nodeTag(node))
{
case T_Var:

View File

@ -8,7 +8,7 @@
*
*
* IDENTIFICATION
* $PostgreSQL: pgsql/src/backend/optimizer/util/clauses.c,v 1.166 2004/03/21 22:29:11 tgl Exp $
* $PostgreSQL: pgsql/src/backend/optimizer/util/clauses.c,v 1.167 2004/03/24 22:40:28 tgl Exp $
*
* HISTORY
* AUTHOR DATE MAJOR EVENT
@ -2347,6 +2347,10 @@ expression_tree_walker(Node *node,
*/
if (node == NULL)
return false;
/* Guard against stack overflow due to overly complex expressions */
check_stack_depth();
switch (nodeTag(node))
{
case T_Var:
@ -2720,6 +2724,10 @@ expression_tree_mutator(Node *node,
if (node == NULL)
return NULL;
/* Guard against stack overflow due to overly complex expressions */
check_stack_depth();
switch (nodeTag(node))
{
case T_Var:

View File

@ -8,7 +8,7 @@
*
*
* IDENTIFICATION
* $PostgreSQL: pgsql/src/backend/parser/parse_expr.c,v 1.166 2004/03/17 20:48:42 tgl Exp $
* $PostgreSQL: pgsql/src/backend/parser/parse_expr.c,v 1.167 2004/03/24 22:40:29 tgl Exp $
*
*-------------------------------------------------------------------------
*/
@ -35,9 +35,6 @@
#include "utils/syscache.h"
int max_expr_depth = DEFAULT_MAX_EXPR_DEPTH;
static int expr_depth_counter = 0;
bool Transform_null_equals = false;
static Node *typecast_expression(ParseState *pstate, Node *expr,
@ -47,19 +44,6 @@ static Node *transformIndirection(ParseState *pstate, Node *basenode,
List *indirection);
/*
* Initialize for parsing a new query.
*
* We reset the expression depth counter here, in case it was left nonzero
* due to ereport()'ing out of the last parsing operation.
*/
void
parse_expr_init(void)
{
expr_depth_counter = 0;
}
/*
* transformExpr -
* Analyze and transform expressions. Type checking and type casting is
@ -92,20 +76,8 @@ transformExpr(ParseState *pstate, Node *expr)
if (expr == NULL)
return NULL;
/*
* Guard against an overly complex expression leading to coredump due
* to stack overflow here, or in later recursive routines that
* traverse expression trees. Note that this is very unlikely to
* happen except with pathological queries; but we don't want someone
* to be able to crash the backend quite that easily...
*/
if (++expr_depth_counter > max_expr_depth)
ereport(ERROR,
(errcode(ERRCODE_STATEMENT_TOO_COMPLEX),
errmsg("expression too complex"),
errdetail("Nesting depth exceeds maximum expression depth %d.",
max_expr_depth),
errhint("Increase the configuration parameter \"max_expr_depth\".")));
/* Guard against stack overflow due to overly complex expressions */
check_stack_depth();
switch (nodeTag(expr))
{
@ -938,8 +910,6 @@ transformExpr(ParseState *pstate, Node *expr)
break;
}
expr_depth_counter--;
return result;
}

View File

@ -14,7 +14,7 @@
* Portions Copyright (c) 1994, Regents of the University of California
*
* IDENTIFICATION
* $PostgreSQL: pgsql/src/backend/parser/parser.c,v 1.60 2003/11/29 19:51:52 pgsql Exp $
* $PostgreSQL: pgsql/src/backend/parser/parser.c,v 1.61 2004/03/24 22:40:29 tgl Exp $
*
*-------------------------------------------------------------------------
*/
@ -50,7 +50,6 @@ raw_parser(const char *str)
scanner_init(str);
parser_init();
parse_expr_init();
yyresult = yyparse();

View File

@ -8,7 +8,7 @@
*
*
* IDENTIFICATION
* $PostgreSQL: pgsql/src/backend/tcop/postgres.c,v 1.396 2004/03/21 22:29:11 tgl Exp $
* $PostgreSQL: pgsql/src/backend/tcop/postgres.c,v 1.397 2004/03/24 22:40:29 tgl Exp $
*
* NOTES
* this is the "main" module of the postgres backend and
@ -92,11 +92,22 @@ bool Log_disconnections = false;
*/
int XfuncMode = 0;
/* GUC variable for maximum stack depth (measured in kilobytes) */
int max_stack_depth = 2048;
/* ----------------
* private variables
* ----------------
*/
/* max_stack_depth converted to bytes for speed of checking */
static int max_stack_depth_bytes = 2048*1024;
/* stack base pointer (initialized by PostgresMain) */
static char *stack_base_ptr = NULL;
/*
* Flag to mark SIGHUP. Whenever the main loop comes around it
* will reread the configuration file. (Better than doing the
@ -1970,6 +1981,64 @@ ProcessInterrupts(void)
}
/*
* check_stack_depth: check for excessively deep recursion
*
* This should be called someplace in any recursive routine that might possibly
* recurse deep enough to overflow the stack. Most Unixen treat stack
* overflow as an unrecoverable SIGSEGV, so we want to error out ourselves
* before hitting the hardware limit. Unfortunately we have no direct way
* to detect the hardware limit, so we have to rely on the admin to set a
* GUC variable for it ...
*/
void
check_stack_depth(void)
{
char stack_top_loc;
int stack_depth;
/*
* Compute distance from PostgresMain's local variables to my own
*
* Note: in theory stack_depth should be ptrdiff_t or some such, but
* since the whole point of this code is to bound the value to something
* much less than integer-sized, int should work fine.
*/
stack_depth = (int) (stack_base_ptr - &stack_top_loc);
/*
* Take abs value, since stacks grow up on some machines, down on others
*/
if (stack_depth < 0)
stack_depth = -stack_depth;
/*
* Trouble?
*
* The test on stack_base_ptr prevents us from erroring out if called
* during process setup or in a non-backend process. Logically it should
* be done first, but putting it here avoids wasting cycles during normal
* cases.
*/
if (stack_depth > max_stack_depth_bytes &&
stack_base_ptr != NULL)
{
ereport(ERROR,
(errcode(ERRCODE_STATEMENT_TOO_COMPLEX),
errmsg("stack depth limit exceeded"),
errhint("Increase the configuration parameter \"max_stack_depth\".")));
}
}
/* GUC assign hook to update max_stack_depth_bytes from max_stack_depth */
bool
assign_max_stack_depth(int newval, bool doit, GucSource source)
{
/* Range check was already handled by guc.c */
if (doit)
max_stack_depth_bytes = newval * 1024;
return true;
}
static void
usage(char *progname)
{
@ -2030,6 +2099,7 @@ PostgresMain(int argc, char *argv[], const char *username)
GucSource gucsource;
char *tmp;
int firstchar;
char stack_base;
StringInfoData input_message;
volatile bool send_rfq = true;
@ -2069,6 +2139,9 @@ PostgresMain(int argc, char *argv[], const char *username)
SetProcessingMode(InitProcessing);
/* Set up reference point for stack depth checking */
stack_base_ptr = &stack_base;
/*
* Set default values for command-line options.
*/

View File

@ -10,7 +10,7 @@
* Written by Peter Eisentraut <peter_e@gmx.net>.
*
* IDENTIFICATION
* $PostgreSQL: pgsql/src/backend/utils/misc/guc.c,v 1.192 2004/03/23 01:23:48 tgl Exp $
* $PostgreSQL: pgsql/src/backend/utils/misc/guc.c,v 1.193 2004/03/24 22:40:29 tgl Exp $
*
*--------------------------------------------------------------------
*/
@ -1023,6 +1023,15 @@ static struct config_int ConfigureNamesInt[] =
16384, 1024, INT_MAX / 1024, NULL, NULL
},
{
{"max_stack_depth", PGC_SUSET, RESOURCES_MEM,
gettext_noop("Sets the maximum stack depth, in kilobytes."),
NULL
},
&max_stack_depth,
2048, 100, INT_MAX / 1024, assign_max_stack_depth, NULL
},
{
{"vacuum_cost_page_hit", PGC_USERSET, RESOURCES,
gettext_noop("Vacuum cost for a page found in the buffer cache."),
@ -1097,14 +1106,6 @@ static struct config_int ConfigureNamesInt[] =
0, 0, INT_MAX, NULL, NULL
},
#endif
{
{"max_expr_depth", PGC_USERSET, CLIENT_CONN_OTHER,
gettext_noop("Sets the maximum expression nesting depth."),
NULL
},
&max_expr_depth,
DEFAULT_MAX_EXPR_DEPTH, 10, INT_MAX, NULL, NULL
},
{
{"statement_timeout", PGC_USERSET, CLIENT_CONN_STATEMENT,
@ -1246,7 +1247,7 @@ static struct config_int ConfigureNamesInt[] =
},
{
{"debug_shared_buffers", PGC_POSTMASTER, RESOURCES_MEM,
{"debug_shared_buffers", PGC_POSTMASTER, STATS_MONITORING,
gettext_noop("Interval to report shared buffer status in seconds"),
NULL
},

View File

@ -58,7 +58,7 @@
#shared_buffers = 1000 # min 16, at least max_connections*2, 8KB each
#work_mem = 1024 # min 64, size in KB
#maintenance_work_mem = 16384 # min 1024, size in KB
#debug_shared_buffers = 0 # 0-600 seconds
#max_stack_depth = 2048 # min 100, size in KB
#vacuum_cost_page_hit = 1 # 0-10000 credits
#vacuum_cost_page_miss = 10 # 0-10000 credits
@ -204,6 +204,8 @@
#log_executor_stats = false
#log_statement_stats = false
#debug_shared_buffers = 0 # 0-600 seconds
# - Query/Index Statistics Collector -
#stats_start_collector = true
@ -243,7 +245,6 @@
#explain_pretty_print = true
#dynamic_library_path = '$libdir'
#max_expr_depth = 10000 # min 10
#---------------------------------------------------------------------------

View File

@ -3,7 +3,7 @@
*
* Copyright (c) 2000-2003, PostgreSQL Global Development Group
*
* $PostgreSQL: pgsql/src/bin/psql/tab-complete.c,v 1.102 2004/03/23 01:23:48 tgl Exp $
* $PostgreSQL: pgsql/src/bin/psql/tab-complete.c,v 1.103 2004/03/24 22:40:29 tgl Exp $
*/
/*----------------------------------------------------------------------
@ -535,11 +535,11 @@ psql_completion(char *text, int start, int end)
"log_statement_stats",
"maintenance_work_mem",
"max_connections",
"max_expr_depth",
"max_files_per_process",
"max_fsm_pages",
"max_fsm_relations",
"max_locks_per_transaction",
"max_stack_depth",
"password_encryption",
"port",
"random_page_cost",

View File

@ -1,18 +1,19 @@
/*-------------------------------------------------------------------------
*
* miscadmin.h
* this file contains general postgres administration and initialization
* This file contains general postgres administration and initialization
* stuff that used to be spread out between the following files:
* globals.h global variables
* pdir.h directory path crud
* pinit.h postgres initialization
* pmod.h processing modes
*
* Over time, this has also become the preferred place for widely known
* resource-limitation stuff, such as work_mem and check_stack_depth().
*
* Portions Copyright (c) 1996-2003, PostgreSQL Global Development Group
* Portions Copyright (c) 1994, Regents of the University of California
*
* $PostgreSQL: pgsql/src/include/miscadmin.h,v 1.154 2004/03/23 01:23:48 tgl Exp $
* $PostgreSQL: pgsql/src/include/miscadmin.h,v 1.155 2004/03/24 22:40:29 tgl Exp $
*
* NOTES
* some of the information in this file should be moved to
@ -70,7 +71,7 @@ extern volatile bool ImmediateInterruptOK;
extern volatile uint32 InterruptHoldoffCount;
extern volatile uint32 CritSectionCount;
/* in postgres.c */
/* in tcop/postgres.c */
extern void ProcessInterrupts(void);
#ifndef WIN32
@ -224,6 +225,10 @@ extern char *UnixSocketDir;
extern char *ListenAddresses;
/* in tcop/postgres.c */
extern void check_stack_depth(void);
/*****************************************************************************
* pdir.h -- *
* POSTGRES directory path definitions. *

View File

@ -7,7 +7,7 @@
* Portions Copyright (c) 1996-2003, PostgreSQL Global Development Group
* Portions Copyright (c) 1994, Regents of the University of California
*
* $PostgreSQL: pgsql/src/include/parser/parse_expr.h,v 1.32 2003/11/29 22:41:09 pgsql Exp $
* $PostgreSQL: pgsql/src/include/parser/parse_expr.h,v 1.33 2004/03/24 22:40:29 tgl Exp $
*
*-------------------------------------------------------------------------
*/
@ -18,7 +18,6 @@
/* GUC parameters */
extern int max_expr_depth;
extern bool Transform_null_equals;
@ -26,6 +25,5 @@ extern Node *transformExpr(ParseState *pstate, Node *expr);
extern Oid exprType(Node *expr);
extern int32 exprTypmod(Node *expr);
extern bool exprIsLengthCoercion(Node *expr, int32 *coercedTypmod);
extern void parse_expr_init(void);
#endif /* PARSE_EXPR_H */

View File

@ -6,7 +6,7 @@
* for developers. If you edit any of these, be sure to do a *full*
* rebuild (and an initdb if noted).
*
* $PostgreSQL: pgsql/src/include/pg_config_manual.h,v 1.11 2004/03/12 00:25:40 neilc Exp $
* $PostgreSQL: pgsql/src/include/pg_config_manual.h,v 1.12 2004/03/24 22:40:29 tgl Exp $
*------------------------------------------------------------------------
*/
@ -112,11 +112,6 @@
*/
#define MAXPGPATH 1024
/*
* DEFAULT_MAX_EXPR_DEPTH: default value of max_expr_depth SET variable.
*/
#define DEFAULT_MAX_EXPR_DEPTH 10000
/*
* PG_SOMAXCONN: maximum accept-queue length limit passed to
* listen(2). You'd think we should use SOMAXCONN from

View File

@ -7,7 +7,7 @@
* Portions Copyright (c) 1996-2003, PostgreSQL Global Development Group
* Portions Copyright (c) 1994, Regents of the University of California
*
* $PostgreSQL: pgsql/src/include/tcop/tcopprot.h,v 1.62 2004/03/15 15:56:27 momjian Exp $
* $PostgreSQL: pgsql/src/include/tcop/tcopprot.h,v 1.63 2004/03/24 22:40:29 tgl Exp $
*
* OLD COMMENTS
* This file was created so that other c files could get the two
@ -23,6 +23,7 @@
#include "executor/execdesc.h"
#include "tcop/dest.h"
#include "utils/guc.h"
extern DLLIMPORT sigjmp_buf Warn_restart;
@ -32,6 +33,7 @@ extern CommandDest whereToSendOutput;
extern bool log_hostname;
extern DLLIMPORT const char *debug_query_string;
extern char *rendezvous_name;
extern int max_stack_depth;
#ifndef BOOTSTRAP_INCLUDE
@ -43,6 +45,9 @@ extern List *pg_analyze_and_rewrite(Node *parsetree,
extern List *pg_rewrite_queries(List *querytree_list);
extern Plan *pg_plan_query(Query *querytree);
extern List *pg_plan_queries(List *querytrees, bool needSnapshot);
extern bool assign_max_stack_depth(int newval, bool doit, GucSource source);
#endif /* BOOTSTRAP_INCLUDE */
extern void die(SIGNAL_ARGS);