]> granicus.if.org Git - postgresql/commitdiff
Push enable/disable of notify and catchup interrupts all the way down
authorTom Lane <tgl@sss.pgh.pa.us>
Thu, 2 Jun 2005 21:03:25 +0000 (21:03 +0000)
committerTom Lane <tgl@sss.pgh.pa.us>
Thu, 2 Jun 2005 21:03:25 +0000 (21:03 +0000)
to just around the bare recv() call that gets a command from the client.
The former placement in PostgresMain was unsafe because the intermediate
processing layers (especially SSL) use facilities such as malloc that are
not necessarily re-entrant.  Per report from counterstorm.com.

src/backend/libpq/be-secure.c
src/backend/tcop/postgres.c
src/include/tcop/tcopprot.h

index f5657efae5155061e179c38740e34bae6c9fc93d..3d5ae65a40e3386d2b2adbe2a2e093bedf5ad212 100644 (file)
@@ -11,7 +11,7 @@
  *
  *
  * IDENTIFICATION
- *       $PostgreSQL: pgsql/src/backend/libpq/be-secure.c,v 1.56 2005/01/08 22:51:12 tgl Exp $
+ *       $PostgreSQL: pgsql/src/backend/libpq/be-secure.c,v 1.57 2005/06/02 21:03:17 tgl Exp $
  *
  *       Since the server static private key ($DataDir/server.key)
  *       will normally be stored unencrypted so that the database
@@ -79,7 +79,6 @@
 #include <sys/stat.h>
 #include <signal.h>
 #include <fcntl.h>
-#include <errno.h>
 #include <ctype.h>
 #include <sys/socket.h>
 #include <unistd.h>
@@ -97,6 +96,8 @@
 
 #include "libpq/libpq.h"
 #include "miscadmin.h"
+#include "tcop/tcopprot.h"
+
 
 #ifdef USE_SSL
 static DH  *load_dh_file(int keylength);
@@ -308,8 +309,14 @@ rloop:
        }
        else
 #endif
+       {
+               prepare_for_client_read();
+
                n = recv(port->sock, ptr, len, 0);
 
+               client_read_ended();
+       }
+
        return n;
 }
 
@@ -410,6 +417,73 @@ wloop:
 /*                                               SSL specific code                                             */
 /* ------------------------------------------------------------ */
 #ifdef USE_SSL
+
+/*
+ * Private substitute BIO: this wraps the SSL library's standard socket BIO
+ * so that we can enable and disable interrupts just while calling recv().
+ * We cannot have interrupts occurring while the bulk of openssl runs,
+ * because it uses malloc() and possibly other non-reentrant libc facilities.
+ *
+ * As of openssl 0.9.7, we can use the reasonably clean method of interposing
+ * a wrapper around the standard socket BIO's sock_read() method.  This relies
+ * on the fact that sock_read() doesn't call anything non-reentrant, in fact
+ * not much of anything at all except recv().  If this ever changes we'd
+ * probably need to duplicate the code of sock_read() in order to push the
+ * interrupt enable/disable down yet another level.
+ */
+
+static bool my_bio_initialized = false;
+static BIO_METHOD my_bio_methods;
+static int (*std_sock_read) (BIO *h, char *buf, int size);
+
+static int
+my_sock_read(BIO *h, char *buf, int size)
+{
+       int             res;
+
+       prepare_for_client_read();
+
+       res = std_sock_read(h, buf, size);
+
+       client_read_ended();
+
+       return res;
+}
+
+static BIO_METHOD *
+my_BIO_s_socket(void)
+{
+       if (!my_bio_initialized)
+       {
+               memcpy(&my_bio_methods, BIO_s_socket(), sizeof(BIO_METHOD));
+               std_sock_read = my_bio_methods.bread;
+               my_bio_methods.bread = my_sock_read;
+               my_bio_initialized = true;
+       }
+       return &my_bio_methods;
+}
+
+/* This should exactly match openssl's SSL_set_fd except for using my BIO */
+static int
+my_SSL_set_fd(SSL *s, int fd)
+{
+       int ret=0;
+       BIO *bio=NULL;
+
+       bio=BIO_new(my_BIO_s_socket());
+
+       if (bio == NULL)
+       {
+               SSLerr(SSL_F_SSL_SET_FD,ERR_R_BUF_LIB);
+               goto err;
+       }
+       BIO_set_fd(bio,fd,BIO_NOCLOSE);
+       SSL_set_bio(s,bio,bio);
+       ret=1;
+err:
+       return(ret);
+}
+
 /*
  *     Load precomputed DH parameters.
  *
@@ -761,7 +835,7 @@ open_server_SSL(Port *port)
                close_SSL(port);
                return -1;
        }
-       if (!SSL_set_fd(port->ssl, port->sock))
+       if (!my_SSL_set_fd(port->ssl, port->sock))
        {
                ereport(COMMERROR,
                                (errcode(ERRCODE_PROTOCOL_VIOLATION),
index fcb93d847d59abcbc7cf020b5997e5555a75bb93..297eb75d8909812306771919b39254ca0d0eecc8 100644 (file)
@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *       $PostgreSQL: pgsql/src/backend/tcop/postgres.c,v 1.445 2005/06/01 23:27:03 momjian Exp $
+ *       $PostgreSQL: pgsql/src/backend/tcop/postgres.c,v 1.446 2005/06/02 21:03:24 tgl Exp $
  *
  * NOTES
  *       this is the "main" module of the postgres backend and
@@ -111,6 +111,13 @@ static volatile sig_atomic_t got_SIGHUP = false;
  */
 static bool xact_started = false;
 
+/*
+ * Flag to indicate that we are doing the outer loop's read-from-client,
+ * as opposed to any random read from client that might happen within
+ * commands like COPY FROM STDIN.
+ */
+static bool DoingCommandRead = false;
+
 /*
  * Flags to implement skip-till-Sync-after-error behavior for messages of
  * the extended query protocol.
@@ -406,6 +413,52 @@ ReadCommand(StringInfo inBuf)
        return result;
 }
 
+/*
+ * prepare_for_client_read -- set up to possibly block on client input
+ *
+ * This must be called immediately before any low-level read from the
+ * client connection.  It is necessary to do it at a sufficiently low level
+ * that there won't be any other operations except the read kernel call
+ * itself between this call and the subsequent client_read_ended() call.
+ * In particular there mustn't be use of malloc() or other potentially
+ * non-reentrant libc functions.  This restriction makes it safe for us
+ * to allow interrupt service routines to execute nontrivial code while
+ * we are waiting for input.
+ */
+void
+prepare_for_client_read(void)
+{
+       if (DoingCommandRead)
+       {
+               /* Enable immediate processing of asynchronous signals */
+               EnableNotifyInterrupt();
+               EnableCatchupInterrupt();
+
+               /* Allow "die" interrupt to be processed while waiting */
+               ImmediateInterruptOK = true;
+
+               /* And don't forget to detect one that already arrived */
+               QueryCancelPending = false;
+               CHECK_FOR_INTERRUPTS();
+       }
+}
+
+/*
+ * client_read_ended -- get out of the client-input state
+ */
+void
+client_read_ended(void)
+{
+       if (DoingCommandRead)
+       {
+               ImmediateInterruptOK = false;
+               QueryCancelPending = false;             /* forget any CANCEL signal */
+
+               DisableNotifyInterrupt();
+               DisableCatchupInterrupt();
+       }
+}
+
 
 /*
  * Parse a query string and pass it through the rewriter.
@@ -2959,6 +3012,7 @@ PostgresMain(int argc, char *argv[], const char *username)
                 * not in other exception-catching places since these interrupts
                 * are only enabled while we wait for client input.
                 */
+               DoingCommandRead = false;
                DisableNotifyInterrupt();
                DisableCatchupInterrupt();
 
@@ -3063,21 +3117,13 @@ PostgresMain(int argc, char *argv[], const char *username)
                }
 
                /*
-                * (2) deal with pending asynchronous NOTIFY from other backends,
-                * and enable async.c's signal handler to execute NOTIFY directly.
-                * Then set up other stuff needed before blocking for input.
+                * (2) Allow asynchronous signals to be executed immediately
+                * if they come in while we are waiting for client input.
+                * (This must be conditional since we don't want, say, reads on
+                * behalf of COPY FROM STDIN doing the same thing.)
                 */
-               QueryCancelPending = false;             /* forget any earlier CANCEL
-                                                                                * signal */
-
-               EnableNotifyInterrupt();
-               EnableCatchupInterrupt();
-
-               /* Allow "die" interrupt to be processed while waiting */
-               ImmediateInterruptOK = true;
-               /* and don't forget to detect one that already arrived */
-               QueryCancelPending = false;
-               CHECK_FOR_INTERRUPTS();
+               QueryCancelPending = false;             /* forget any earlier CANCEL signal */
+               DoingCommandRead = true;
 
                /*
                 * (3) read a command (loop blocks here)
@@ -3087,11 +3133,7 @@ PostgresMain(int argc, char *argv[], const char *username)
                /*
                 * (4) disable async signal conditions again.
                 */
-               ImmediateInterruptOK = false;
-               QueryCancelPending = false;             /* forget any CANCEL signal */
-
-               DisableNotifyInterrupt();
-               DisableCatchupInterrupt();
+               DoingCommandRead = false;
 
                /*
                 * (5) check for any other interesting events that happened while
index 23764dd1f9364f783267e5249afeb19e96bff855..23b338e7ae1ba7469e904387b7752b87e14fbaf2 100644 (file)
@@ -7,7 +7,7 @@
  * Portions Copyright (c) 1996-2005, PostgreSQL Global Development Group
  * Portions Copyright (c) 1994, Regents of the University of California
  *
- * $PostgreSQL: pgsql/src/include/tcop/tcopprot.h,v 1.73 2004/12/31 22:03:44 pgsql Exp $
+ * $PostgreSQL: pgsql/src/include/tcop/tcopprot.h,v 1.74 2005/06/02 21:03:25 tgl Exp $
  *
  * OLD COMMENTS
  *       This file was created so that other c files could get the two
@@ -59,6 +59,8 @@ extern bool assign_max_stack_depth(int newval, bool doit, GucSource source);
 extern void die(SIGNAL_ARGS);
 extern void quickdie(SIGNAL_ARGS);
 extern void authdie(SIGNAL_ARGS);
+extern void prepare_for_client_read(void);
+extern void client_read_ended(void);
 extern int     PostgresMain(int argc, char *argv[], const char *username);
 extern void ResetUsage(void);
 extern void ShowUsage(const char *title);