OSDN Git Service

Replace max_expr_depth parameter with a max_stack_depth parameter that
authorTom Lane <tgl@sss.pgh.pa.us>
Wed, 24 Mar 2004 22:40:29 +0000 (22:40 +0000)
committerTom Lane <tgl@sss.pgh.pa.us>
Wed, 24 Mar 2004 22:40:29 +0000 (22:40 +0000)
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.

13 files changed:
doc/src/sgml/runtime.sgml
src/backend/executor/execQual.c
src/backend/optimizer/util/clauses.c
src/backend/parser/parse_expr.c
src/backend/parser/parser.c
src/backend/tcop/postgres.c
src/backend/utils/misc/guc.c
src/backend/utils/misc/postgresql.conf.sample
src/bin/psql/tab-complete.c
src/include/miscadmin.h
src/include/parser/parse_expr.h
src/include/pg_config_manual.h
src/include/tcop/tcopprot.h

index da6d6a5..9390204 100644 (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>
index fc16ccc..4c6f95a 100644 (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 $
  *
  *-------------------------------------------------------------------------
  */
  *             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:
index c006cd4..b05f760 100644 (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:
index 3881cc3..83daae9 100644 (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,
@@ -48,19 +45,6 @@ static Node *transformIndirection(ParseState *pstate, Node *basenode,
 
 
 /*
- * 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
  *       done here. The optimizer and the executor cannot handle the original
@@ -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;
 }
 
index 4e31e79..54cc8a1 100644 (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();
 
index 91442d4..ff0ac6a 100644 (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.
         */
index 280977d..c14b428 100644 (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 $
  *
  *--------------------------------------------------------------------
  */
@@ -1024,6 +1024,15 @@ static struct config_int ConfigureNamesInt[] =
        },
 
        {
+               {"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."),
                        NULL
@@ -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
                },
index ff12fbd..0001a9f 100644 (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
 #log_executor_stats = false
 #log_statement_stats = false
 
+#debug_shared_buffers = 0      # 0-600 seconds
+
 # - Query/Index Statistics Collector -
 
 #stats_start_collector = true
 
 #explain_pretty_print = true
 #dynamic_library_path = '$libdir'
-#max_expr_depth = 10000                # min 10
 
 
 #---------------------------------------------------------------------------
index 8b60715..ff9e8c4 100644 (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",
index f34ebb0..4aba00b 100644 (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.                                                     *
index d5b87da..a8be03a 100644 (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 */
index 6fcf38e..87ddd1f 100644 (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 $
  *------------------------------------------------------------------------
  */
 
 #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
  * <sys/socket.h>, but on many systems that symbol is much smaller
index 20c84dd..b2520b2 100644 (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);