Currently, only regular backends set the stack base pointer, for the
check_stack_depth() mechanism, in PostgresMain. We don't have stack
overrun protection in auxiliary processes. However, autovacuum workers
at least can run arbitrary user code, and if that overruns the stack,
you get a segfault.
Here's a little script to reproduce that:
begin;
create table atable(id int4);
insert into atable select generate_series(1,10000);
/* not recursive yet */
create or replace function recfunc(i int4) returns bool AS $$begin
return true;
end;
$$ language plpgsql immutable;
/* Create index using the function. */
create index recindex on atable((recfunc(id)));
/* make the function recursive */
create or replace function recfunc(i int4) returns bool AS $$begin
perform recfunc(i);
end;
$$ language plpgsql immutable;
commit;
/* Now wait for autoanalyze to kick in, and crash */
The fix is quite straightforward, we just need to set the stack base
pointer. I think we should set it in all processes, even though most
auxiliary processes like bgwriter can't execute arbitrary code. There's
no harm in doing so, anyway. I'm thinking that we should set the base
pointer in PostmasterMain(), so that it is inherited by all forked
processes, and in SubPostmasterMain() for EXEC_BACKEND.
Proposed patch attached. The comment changes regarding PL/Java are in
anticipation for a fix for the Itanium-issue mentioned here:
http://lists.pgfoundry.org/pipermail/pljava-dev/2012/001906.html.
Nothing has yet been done in PL/Java, but I am assuming it will start
using the set/restore_stack_base() functions introduced in this patch.
However, we might need to do something more complicated to fix the first
PL/Java issue I explain in that email.
I suppose this needs to be backpatched all the way to 8.3.
--
Heikki Linnakangas
EnterpriseDB http://www.enterprisedb.com
commit 9d0b44fd1b8b7e872080c032ddb0cf77c1bd2c40
Author: Heikki Linnakangas <heikki.linnakan...@iki.fi>
Date: Sun Apr 1 19:20:47 2012 +0300
Do stack-depth checking in all postmaster children.
We used to only initialize the stack base pointer when starting up a regular
backend, not in other processes. In particular, autovacuum workers can run
arbitrary user code, and without stack-depth checking, infinite recursion
in e.g an index expression will bring down the whole cluster.
diff --git a/src/backend/postmaster/postmaster.c b/src/backend/postmaster/postmaster.c
index 1dac695..1440f5f 100644
--- a/src/backend/postmaster/postmaster.c
+++ b/src/backend/postmaster/postmaster.c
@@ -971,6 +971,11 @@ PostmasterMain(int argc, char *argv[])
set_max_safe_fds();
/*
+ * Set reference point for stack-depth checking
+ */
+ set_stack_base();
+
+ /*
* Initialize the list of active backends.
*/
BackendList = DLNewList();
@@ -3978,6 +3983,11 @@ SubPostmasterMain(int argc, char *argv[])
read_backend_variables(argv[2], &port);
/*
+ * Set reference point for stack-depth checking
+ */
+ set_stack_base();
+
+ /*
* Set up memory area for GSS information. Mirrors the code in ConnCreate
* for the non-exec case.
*/
diff --git a/src/backend/tcop/postgres.c b/src/backend/tcop/postgres.c
index 02be363..d230118 100644
--- a/src/backend/tcop/postgres.c
+++ b/src/backend/tcop/postgres.c
@@ -115,8 +115,10 @@ int PostAuthDelay = 0;
static long max_stack_depth_bytes = 100 * 1024L;
/*
- * Stack base pointer -- initialized by PostgresMain. This is not static
- * so that PL/Java can modify it.
+ * Stack base pointer -- initialized by PostmasterMain and inherited by
+ * subprocesses. This is not static because old versions of PL/Java modify
+ * it directly. Newer versions use set_stack_base(), but we want to stay
+ * binary-compatible for the time being.
*/
char *stack_base_ptr = NULL;
@@ -2958,6 +2960,53 @@ ia64_get_bsp(void)
/*
+ * set_stack_base: set up reference point for stack depth checking
+ *
+ * Returns the old reference point, if any.
+ */
+pg_stack_base_t
+set_stack_base(void)
+{
+ char stack_base;
+ pg_stack_base_t old;
+
+#if defined(__ia64__) || defined(__ia64)
+ old.stack_base_ptr = stack_base_ptr;
+ old.register_stack_base_ptr = register_stack_base_ptr;
+#else
+ old = stack_base_ptr;
+#endif
+
+ /* Set up reference point for stack depth checking */
+ stack_base_ptr = &stack_base;
+#if defined(__ia64__) || defined(__ia64)
+ register_stack_base_ptr = ia64_get_bsp();
+#endif
+
+ return old;
+}
+
+/*
+ * restore_stack_base: restore reference point for stack depth checking
+ *
+ * This can be used after set_stack_base() to restore the old value. This
+ * is currently only used in PL/Java. When PL/Java calls a backend function
+ * from different thread, the thread's stack is at a different location than
+ * the main thread's stack, so it sets the base pointer before the call, and
+ * restores it afterwards.
+ */
+void
+restore_stack_base(pg_stack_base_t base)
+{
+#if defined(__ia64__) || defined(__ia64)
+ stack_base_ptr = base.stack_base_ptr;
+ register_stack_base_ptr = base.register_stack_base_ptr;
+#else
+ stack_base_ptr = base;
+#endif
+}
+
+/*
* check_stack_depth: check for excessively deep recursion
*
* This should be called someplace in any recursive routine that might possibly
@@ -2972,7 +3021,7 @@ check_stack_depth(void)
long stack_depth;
/*
- * Compute distance from PostgresMain's local variables to my own
+ * Compute distance from reference point to to my local variables.
*/
stack_depth = (long) (stack_base_ptr - &stack_top_loc);
@@ -3434,7 +3483,6 @@ PostgresMain(int argc, char *argv[], const char *username)
{
const char *dbname;
int firstchar;
- char stack_base;
StringInfoData input_message;
sigjmp_buf local_sigjmp_buf;
volatile bool send_ready_for_query = true;
@@ -3461,10 +3509,7 @@ PostgresMain(int argc, char *argv[], const char *username)
SetProcessingMode(InitProcessing);
/* Set up reference point for stack depth checking */
- stack_base_ptr = &stack_base;
-#if defined(__ia64__) || defined(__ia64)
- register_stack_base_ptr = ia64_get_bsp();
-#endif
+ set_stack_base();
/* Compute paths, if we didn't inherit them from postmaster */
if (my_exec_path[0] == '\0')
diff --git a/src/include/miscadmin.h b/src/include/miscadmin.h
index 610cb59..d842117 100644
--- a/src/include/miscadmin.h
+++ b/src/include/miscadmin.h
@@ -246,6 +246,19 @@ extern bool VacuumCostActive;
/* in tcop/postgres.c */
+
+#if defined(__ia64__) || defined(__ia64)
+typedef struct
+{
+ char *stack_base_ptr;
+ char *register_stack_base_ptr;
+} pg_stack_base_t
+#else
+typedef char *pg_stack_base_t;
+#endif
+
+extern pg_stack_base_t set_stack_base(void);
+extern void restore_stack_base(pg_stack_base_t base);
extern void check_stack_depth(void);
/* in tcop/utility.c */
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
