]> granicus.if.org Git - postgresql/commitdiff
Be more careful to not lose sync in the FE/BE protocol.
authorHeikki Linnakangas <heikki.linnakangas@iki.fi>
Mon, 2 Feb 2015 15:09:12 +0000 (17:09 +0200)
committerHeikki Linnakangas <heikki.linnakangas@iki.fi>
Mon, 2 Feb 2015 15:09:35 +0000 (17:09 +0200)
If any error occurred while we were in the middle of reading a protocol
message from the client, we could lose sync, and incorrectly try to
interpret a part of another message as a new protocol message. That will
usually lead to an "invalid frontend message" error that terminates the
connection. However, this is a security issue because an attacker might
be able to deliberately cause an error, inject a Query message in what's
supposed to be just user data, and have the server execute it.

We were quite careful to not have CHECK_FOR_INTERRUPTS() calls or other
operations that could ereport(ERROR) in the middle of processing a message,
but a query cancel interrupt or statement timeout could nevertheless cause
it to happen. Also, the V2 fastpath and COPY handling were not so careful.
It's very difficult to recover in the V2 COPY protocol, so we will just
terminate the connection on error. In practice, that's what happened
previously anyway, as we lost protocol sync.

To fix, add a new variable in pqcomm.c, PqCommReadingMsg, that is set
whenever we're in the middle of reading a message. When it's set, we cannot
safely ERROR out and continue running, because we might've read only part
of a message. PqCommReadingMsg acts somewhat similarly to critical sections
in that if an error occurs while it's set, the error handler will force the
connection to be terminated, as if the error was FATAL. It's not
implemented by promoting ERROR to FATAL in elog.c, like ERROR is promoted
to PANIC in critical sections, because we want to be able to use
PG_TRY/CATCH to recover and regain protocol sync. pq_getmessage() takes
advantage of that to prevent an OOM error from terminating the connection.

To prevent unnecessary connection terminations, add a holdoff mechanism
similar to HOLD/RESUME_INTERRUPTS() that can be used hold off query cancel
interrupts, but still allow die interrupts. The rules on which interrupts
are processed when are now a bit more complicated, so refactor
ProcessInterrupts() and the calls to it in signal handlers so that the
signal handlers always call it if ImmediateInterruptOK is set, and
ProcessInterrupts() can decide to not do anything if the other conditions
are not met.

Reported by Emil Lenngren. Patch reviewed by Noah Misch and Andres Freund.
Backpatch to all supported versions.

Security: CVE-2015-0244

13 files changed:
src/backend/commands/copy.c
src/backend/libpq/auth.c
src/backend/libpq/pqcomm.c
src/backend/postmaster/postmaster.c
src/backend/replication/walsender.c
src/backend/storage/lmgr/proc.c
src/backend/tcop/fastpath.c
src/backend/tcop/postgres.c
src/backend/utils/error/elog.c
src/backend/utils/init/globals.c
src/include/libpq/libpq.h
src/include/miscadmin.h
src/include/tcop/fastpath.h

index f19382f5920f8cfe5b366872c77130af08841cdc..55192741aa6e8ad77c0858b303771c69cc723065 100644 (file)
@@ -395,6 +395,8 @@ ReceiveCopyBegin(CopyState cstate)
                                        (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
                        errmsg("COPY BINARY is not supported to stdout or from stdin")));
                pq_putemptymessage('G');
+               /* any error in old protocol will make us lose sync */
+               pq_startmsgread();
                cstate->copy_dest = COPY_OLD_FE;
        }
        else
@@ -405,6 +407,8 @@ ReceiveCopyBegin(CopyState cstate)
                                        (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
                        errmsg("COPY BINARY is not supported to stdout or from stdin")));
                pq_putemptymessage('D');
+               /* any error in old protocol will make us lose sync */
+               pq_startmsgread();
                cstate->copy_dest = COPY_OLD_FE;
        }
        /* We *must* flush here to ensure FE knows it can send. */
@@ -565,6 +569,8 @@ CopyGetData(CopyState cstate, void *databuf, int minread, int maxread)
                                        int                     mtype;
 
                        readmessage:
+                                       HOLD_CANCEL_INTERRUPTS();
+                                       pq_startmsgread();
                                        mtype = pq_getbyte();
                                        if (mtype == EOF)
                                                ereport(ERROR,
@@ -574,6 +580,7 @@ CopyGetData(CopyState cstate, void *databuf, int minread, int maxread)
                                                ereport(ERROR,
                                                                (errcode(ERRCODE_CONNECTION_FAILURE),
                                                                 errmsg("unexpected EOF on client connection with an open transaction")));
+                                       RESUME_CANCEL_INTERRUPTS();
                                        switch (mtype)
                                        {
                                                case 'd':               /* CopyData */
@@ -2137,6 +2144,13 @@ CopyFrom(CopyState cstate)
 
        MemoryContextSwitchTo(oldcontext);
 
+       /*
+        * In the old protocol, tell pqcomm that we can process normal protocol
+        * messages again.
+        */
+       if (cstate->copy_dest == COPY_OLD_FE)
+               pq_endmsgread();
+
        /* Execute AFTER STATEMENT insertion triggers */
        ExecASInsertTriggers(estate, resultRelInfo);
 
index 3973a295b1c5e2b698dce32510d33b53c1b9ee64..ef1d6f16f48291515bd6dc8c4c0d0545cccb9ce9 100644 (file)
@@ -649,6 +649,7 @@ recv_password_packet(Port *port)
 {
        StringInfoData buf;
 
+       pq_startmsgread();
        if (PG_PROTOCOL_MAJOR(port->proto) >= 3)
        {
                /* Expect 'p' message type */
@@ -1054,6 +1055,7 @@ pg_GSS_recvauth(Port *port)
         */
        do
        {
+               pq_startmsgread();
                mtype = pq_getbyte();
                if (mtype != 'p')
                {
@@ -1292,6 +1294,7 @@ pg_SSPI_recvauth(Port *port)
         */
        do
        {
+               pq_startmsgread();
                mtype = pq_getbyte();
                if (mtype != 'p')
                {
index c2472e7cd98bb3508ef283c65f9a7f1e799ed955..aeeffd72609d80b64d36ad78b990eb9f63ec3011 100644 (file)
@@ -129,8 +129,9 @@ static int  PqRecvLength;           /* End of data available in PqRecvBuffer */
 /*
  * Message status
  */
-static bool PqCommBusy;
-static bool DoingCopyOut;
+static bool PqCommBusy;                        /* busy sending data to the client */
+static bool PqCommReadingMsg;  /* in the middle of reading a message */
+static bool DoingCopyOut;              /* in old-protocol COPY OUT processing */
 
 
 /* Internal functions */
@@ -156,6 +157,7 @@ pq_init(void)
        PqSendBuffer = MemoryContextAlloc(TopMemoryContext, PqSendBufferSize);
        PqSendPointer = PqSendStart = PqRecvPointer = PqRecvLength = 0;
        PqCommBusy = false;
+       PqCommReadingMsg = false;
        DoingCopyOut = false;
        on_proc_exit(pq_close, 0);
 }
@@ -860,6 +862,8 @@ pq_recvbuf(void)
 int
 pq_getbyte(void)
 {
+       Assert(PqCommReadingMsg);
+
        while (PqRecvPointer >= PqRecvLength)
        {
                if (pq_recvbuf())               /* If nothing in buffer, then recv some */
@@ -898,6 +902,8 @@ pq_getbyte_if_available(unsigned char *c)
 {
        int                     r;
 
+       Assert(PqCommReadingMsg);
+
        if (PqRecvPointer < PqRecvLength)
        {
                *c = PqRecvBuffer[PqRecvPointer++];
@@ -950,6 +956,8 @@ pq_getbytes(char *s, size_t len)
 {
        size_t          amount;
 
+       Assert(PqCommReadingMsg);
+
        while (len > 0)
        {
                while (PqRecvPointer >= PqRecvLength)
@@ -982,6 +990,8 @@ pq_discardbytes(size_t len)
 {
        size_t          amount;
 
+       Assert(PqCommReadingMsg);
+
        while (len > 0)
        {
                while (PqRecvPointer >= PqRecvLength)
@@ -1018,6 +1028,8 @@ pq_getstring(StringInfo s)
 {
        int                     i;
 
+       Assert(PqCommReadingMsg);
+
        resetStringInfo(s);
 
        /* Read until we get the terminating '\0' */
@@ -1049,6 +1061,58 @@ pq_getstring(StringInfo s)
 }
 
 
+/* --------------------------------
+ *             pq_startmsgread - begin reading a message from the client.
+ *
+ *             This must be called before any of the pq_get* functions.
+ * --------------------------------
+ */
+void
+pq_startmsgread(void)
+{
+       /*
+        * There shouldn't be a read active already, but let's check just to be
+        * sure.
+        */
+       if (PqCommReadingMsg)
+               ereport(FATAL,
+                               (errcode(ERRCODE_PROTOCOL_VIOLATION),
+                                errmsg("terminating connection because protocol sync was lost")));
+
+       PqCommReadingMsg = true;
+}
+
+
+/* --------------------------------
+ *             pq_endmsgread   - finish reading message.
+ *
+ *             This must be called after reading a V2 protocol message with
+ *             pq_getstring() and friends, to indicate that we have read the whole
+ *             message. In V3 protocol, pq_getmessage() does this implicitly.
+ * --------------------------------
+ */
+void
+pq_endmsgread(void)
+{
+       Assert(PqCommReadingMsg);
+
+       PqCommReadingMsg = false;
+}
+
+/* --------------------------------
+ *             pq_is_reading_msg - are we currently reading a message?
+ *
+ * This is used in error recovery at the outer idle loop to detect if we have
+ * lost protocol sync, and need to terminate the connection. pq_startmsgread()
+ * will check for that too, but it's nicer to detect it earlier.
+ * --------------------------------
+ */
+bool
+pq_is_reading_msg(void)
+{
+       return PqCommReadingMsg;
+}
+
 /* --------------------------------
  *             pq_getmessage   - get a message with length word from connection
  *
@@ -1070,6 +1134,8 @@ pq_getmessage(StringInfo s, int maxlen)
 {
        int32           len;
 
+       Assert(PqCommReadingMsg);
+
        resetStringInfo(s);
 
        /* Read message length word */
@@ -1111,6 +1177,9 @@ pq_getmessage(StringInfo s, int maxlen)
                                ereport(COMMERROR,
                                                (errcode(ERRCODE_PROTOCOL_VIOLATION),
                                                 errmsg("incomplete message from client")));
+
+                       /* we discarded the rest of the message so we're back in sync. */
+                       PqCommReadingMsg = false;
                        PG_RE_THROW();
                }
                PG_END_TRY();
@@ -1128,6 +1197,9 @@ pq_getmessage(StringInfo s, int maxlen)
                s->data[len] = '\0';
        }
 
+       /* finished reading the message. */
+       PqCommReadingMsg = false;
+
        return 0;
 }
 
index d3f089e65bb8c6d23a573379e59bfa2bd260835e..07aca31a93c205ba7ab0f36c0217f8132dd3881a 100644 (file)
@@ -1523,6 +1523,7 @@ ProcessStartupPacket(Port *port, bool SSLdone)
        ProtocolVersion proto;
        MemoryContext oldcontext;
 
+       pq_startmsgread();
        if (pq_getbytes((char *) &len, 4) == EOF)
        {
                /*
@@ -1567,6 +1568,7 @@ ProcessStartupPacket(Port *port, bool SSLdone)
                                 errmsg("incomplete startup packet")));
                return STATUS_ERROR;
        }
+       pq_endmsgread();
 
        /*
         * The first field is either a protocol version number or a special
index e73982a6fdcaa3a63d06de988dd67db87a77c6bb..7deaa82fd64ea96e551b0ad587b4d56e6c93b3d0 100644 (file)
@@ -216,8 +216,16 @@ WalSndHandshake(void)
                set_ps_display("idle", false);
 
                /* Wait for a command to arrive */
+               pq_startmsgread();
                firstchar = pq_getbyte();
 
+               /* Read the message contents */
+               if (firstchar != EOF)
+               {
+                       if (pq_getmessage(&input_message, 0))
+                               firstchar = EOF;        /* suitable message already logged */
+               }
+
                /*
                 * Emergency bailout if postmaster has died.  This is to avoid the
                 * necessity for manual cleanup of all postmaster children.
@@ -235,16 +243,6 @@ WalSndHandshake(void)
                        ProcessConfigFile(PGC_SIGHUP);
                }
 
-               if (firstchar != EOF)
-               {
-                       /*
-                        * Read the message contents. This is expected to be done without
-                        * blocking because we've been able to get message type code.
-                        */
-                       if (pq_getmessage(&input_message, 0))
-                               firstchar = EOF;        /* suitable message already logged */
-               }
-
                /* Handle the very limited subset of commands expected in this phase */
                switch (firstchar)
                {
@@ -513,6 +511,7 @@ ProcessRepliesIfAny(void)
 
        for (;;)
        {
+               pq_startmsgread();
                r = pq_getbyte_if_available(&firstchar);
                if (r < 0)
                {
@@ -525,9 +524,20 @@ ProcessRepliesIfAny(void)
                if (r == 0)
                {
                        /* no data available without blocking */
+                       pq_endmsgread();
                        break;
                }
 
+               /* Read the message contents */
+               resetStringInfo(&reply_message);
+               if (pq_getmessage(&reply_message, 0))
+               {
+                       ereport(COMMERROR,
+                                       (errcode(ERRCODE_PROTOCOL_VIOLATION),
+                                        errmsg("unexpected EOF on standby connection")));
+                       proc_exit(0);
+               }
+
                /* Handle the very limited subset of commands expected in this phase */
                switch (firstchar)
                {
@@ -568,19 +578,6 @@ ProcessStandbyMessage(void)
 {
        char            msgtype;
 
-       resetStringInfo(&reply_message);
-
-       /*
-        * Read the message contents.
-        */
-       if (pq_getmessage(&reply_message, 0))
-       {
-               ereport(COMMERROR,
-                               (errcode(ERRCODE_PROTOCOL_VIOLATION),
-                                errmsg("unexpected EOF on standby connection")));
-               proc_exit(0);
-       }
-
        /*
         * Check message type from the first byte.
         */
index 6045bf2b79ede90dc70dcc6531832a88dc4c012d..cac061f6661aaed1ddd787f4b01f08fed87e3905 100644 (file)
@@ -665,11 +665,16 @@ LockErrorCleanup(void)
 {
        LWLockId        partitionLock;
 
+       HOLD_INTERRUPTS();
+
        AbortStrongLockAcquire();
 
        /* Nothing to do if we weren't waiting for a lock */
        if (lockAwaited == NULL)
+       {
+               RESUME_INTERRUPTS();
                return;
+       }
 
        /* Turn off the deadlock timer, if it's still running (see ProcSleep) */
        disable_sig_alarm(false);
@@ -708,6 +713,8 @@ LockErrorCleanup(void)
         * wakeup signal isn't harmful, and it seems not worth expending cycles to
         * get rid of a signal that most likely isn't there.
         */
+
+       RESUME_INTERRUPTS();
 }
 
 
index 9bf983ed33d92d55f0978ff22978e01088b1e38f..e73979951522e1255fd4096bbbc392e7c2badccb 100644 (file)
@@ -73,7 +73,7 @@ static int16 parse_fcall_arguments_20(StringInfo msgBuf, struct fp_info * fip,
  * The caller should already have initialized buf to empty.
  * ----------------
  */
-static int
+int
 GetOldFunctionMessage(StringInfo buf)
 {
        int32           ibuf;
@@ -278,33 +278,6 @@ HandleFunctionRequest(StringInfo msgBuf)
        bool            was_logged = false;
        char            msec_str[32];
 
-       /*
-        * Read message contents if not already done.
-        */
-       if (PG_PROTOCOL_MAJOR(FrontendProtocol) < 3)
-       {
-               if (GetOldFunctionMessage(msgBuf))
-               {
-                       if (IsTransactionState())
-                               ereport(COMMERROR,
-                                               (errcode(ERRCODE_CONNECTION_FAILURE),
-                                                errmsg("unexpected EOF on client connection with an open transaction")));
-                       else
-                       {
-                               /*
-                                * Can't send DEBUG log messages to client at this point.
-                                * Since we're disconnecting right away, we don't need to
-                                * restore whereToSendOutput.
-                                */
-                               whereToSendOutput = DestNone;
-                               ereport(DEBUG1,
-                                               (errcode(ERRCODE_CONNECTION_DOES_NOT_EXIST),
-                                                errmsg("unexpected EOF on client connection")));
-                       }
-                       return EOF;
-               }
-       }
-
        /*
         * Now that we've eaten the input message, check to see if we actually
         * want to do the function call or not.  It's now safe to ereport(); we
index 6a0d7996e903003efa99d9a82d249684be9ea20a..39017ff940c63dce76018d4aa3138bcb38d35bee 100644 (file)
@@ -339,6 +339,8 @@ SocketBackend(StringInfo inBuf)
        /*
         * Get message type code from the frontend.
         */
+       HOLD_CANCEL_INTERRUPTS();
+       pq_startmsgread();
        qtype = pq_getbyte();
 
        if (qtype == EOF)                       /* frontend disconnected */
@@ -387,7 +389,7 @@ SocketBackend(StringInfo inBuf)
                                        {
                                                /*
                                                 * Can't send DEBUG log messages to client at this
-                                                * point.Since we're disconnecting right away, we
+                                                * point. Since we're disconnecting right away, we
                                                 * don't need to restore whereToSendOutput.
                                                 */
                                                whereToSendOutput = DestNone;
@@ -401,8 +403,30 @@ SocketBackend(StringInfo inBuf)
                        break;
 
                case 'F':                               /* fastpath function call */
-                       /* we let fastpath.c cope with old-style input of this */
                        doing_extended_query_message = false;
+                       if (PG_PROTOCOL_MAJOR(FrontendProtocol) < 3)
+                       {
+                               if (GetOldFunctionMessage(inBuf))
+                               {
+                                       if (IsTransactionState())
+                                               ereport(COMMERROR,
+                                                               (errcode(ERRCODE_CONNECTION_FAILURE),
+                                                                errmsg("unexpected EOF on client connection with an open transaction")));
+                                       else
+                                       {
+                                               /*
+                                                * Can't send DEBUG log messages to client at this
+                                                * point. Since we're disconnecting right away, we
+                                                * don't need to restore whereToSendOutput.
+                                                */
+                                               whereToSendOutput = DestNone;
+                                               ereport(DEBUG1,
+                                                               (errcode(ERRCODE_CONNECTION_DOES_NOT_EXIST),
+                                                        errmsg("unexpected EOF on client connection")));
+                                       }
+                                       return EOF;
+                               }
+                       }
                        break;
 
                case 'X':                               /* terminate */
@@ -470,6 +494,9 @@ SocketBackend(StringInfo inBuf)
                if (pq_getmessage(inBuf, 0))
                        return EOF;                     /* suitable message already logged */
        }
+       else
+               pq_endmsgread();
+       RESUME_CANCEL_INTERRUPTS();
 
        return qtype;
 }
@@ -514,7 +541,7 @@ prepare_for_client_read(void)
                EnableNotifyInterrupt();
                EnableCatchupInterrupt();
 
-               /* Allow cancel/die interrupts to be processed while waiting */
+               /* Allow die interrupts to be processed while waiting */
                ImmediateInterruptOK = true;
 
                /* And don't forget to detect one that already arrived */
@@ -2591,21 +2618,11 @@ die(SIGNAL_ARGS)
                ProcDiePending = true;
 
                /*
-                * If it's safe to interrupt, and we're waiting for input or a lock,
-                * service the interrupt immediately
+                * If we're waiting for input or a lock so that it's safe to
+                * interrupt, service the interrupt immediately
                 */
-               if (ImmediateInterruptOK && InterruptHoldoffCount == 0 &&
-                       CritSectionCount == 0)
-               {
-                       /* bump holdoff count to make ProcessInterrupts() a no-op */
-                       /* until we are done getting ready for it */
-                       InterruptHoldoffCount++;
-                       LockErrorCleanup(); /* prevent CheckDeadLock from running */
-                       DisableNotifyInterrupt();
-                       DisableCatchupInterrupt();
-                       InterruptHoldoffCount--;
+               if (ImmediateInterruptOK)
                        ProcessInterrupts();
-               }
        }
 
        /* If we're still here, waken anything waiting on the process latch */
@@ -2633,21 +2650,11 @@ StatementCancelHandler(SIGNAL_ARGS)
                QueryCancelPending = true;
 
                /*
-                * If it's safe to interrupt, and we're waiting for input or a lock,
-                * service the interrupt immediately
+                * If we're waiting for input or a lock so that it's safe to
+                * interrupt, service the interrupt immediately
                 */
-               if (ImmediateInterruptOK && InterruptHoldoffCount == 0 &&
-                       CritSectionCount == 0)
-               {
-                       /* bump holdoff count to make ProcessInterrupts() a no-op */
-                       /* until we are done getting ready for it */
-                       InterruptHoldoffCount++;
-                       LockErrorCleanup(); /* prevent CheckDeadLock from running */
-                       DisableNotifyInterrupt();
-                       DisableCatchupInterrupt();
-                       InterruptHoldoffCount--;
+               if (ImmediateInterruptOK)
                        ProcessInterrupts();
-               }
        }
 
        /* If we're still here, waken anything waiting on the process latch */
@@ -2792,21 +2799,11 @@ RecoveryConflictInterrupt(ProcSignalReason reason)
                        RecoveryConflictRetryable = false;
 
                /*
-                * If it's safe to interrupt, and we're waiting for input or a lock,
-                * service the interrupt immediately
+                * If we're waiting for input or a lock so that it's safe to
+                * interrupt, service the interrupt immediately.
                 */
-               if (ImmediateInterruptOK && InterruptHoldoffCount == 0 &&
-                       CritSectionCount == 0)
-               {
-                       /* bump holdoff count to make ProcessInterrupts() a no-op */
-                       /* until we are done getting ready for it */
-                       InterruptHoldoffCount++;
-                       LockErrorCleanup(); /* prevent CheckDeadLock from running */
-                       DisableNotifyInterrupt();
-                       DisableCatchupInterrupt();
-                       InterruptHoldoffCount--;
+               if (ImmediateInterruptOK)
                        ProcessInterrupts();
-               }
        }
 
        /*
@@ -2832,15 +2829,17 @@ RecoveryConflictInterrupt(ProcSignalReason reason)
 void
 ProcessInterrupts(void)
 {
-       /* OK to accept interrupt now? */
+       /* OK to accept any interrupts now? */
        if (InterruptHoldoffCount != 0 || CritSectionCount != 0)
                return;
        InterruptPending = false;
+
        if (ProcDiePending)
        {
                ProcDiePending = false;
                QueryCancelPending = false;             /* ProcDie trumps QueryCancel */
                ImmediateInterruptOK = false;   /* not idle anymore */
+               LockErrorCleanup();
                DisableNotifyInterrupt();
                DisableCatchupInterrupt();
                /* As in quickdie, don't risk sending to client during auth */
@@ -2877,6 +2876,7 @@ ProcessInterrupts(void)
        {
                QueryCancelPending = false;             /* lost connection trumps QueryCancel */
                ImmediateInterruptOK = false;   /* not idle anymore */
+               LockErrorCleanup();
                DisableNotifyInterrupt();
                DisableCatchupInterrupt();
                /* don't send to client, we already know the connection to be dead. */
@@ -2885,12 +2885,53 @@ ProcessInterrupts(void)
                                (errcode(ERRCODE_CONNECTION_FAILURE),
                                 errmsg("connection to client lost")));
        }
+
+       /*
+        * If a recovery conflict happens while we are waiting for input from the
+        * client, the client is presumably just sitting idle in a transaction,
+        * preventing recovery from making progress.  Terminate the connection to
+        * dislodge it.
+        */
+       if (RecoveryConflictPending && DoingCommandRead)
+       {
+               QueryCancelPending = false;                     /* this trumps QueryCancel */
+               ImmediateInterruptOK = false;           /* not idle anymore */
+               RecoveryConflictPending = false;
+               LockErrorCleanup();
+               DisableNotifyInterrupt();
+               DisableCatchupInterrupt();
+               pgstat_report_recovery_conflict(RecoveryConflictReason);
+               ereport(FATAL,
+                               (errcode(ERRCODE_T_R_SERIALIZATION_FAILURE),
+                                errmsg("terminating connection due to conflict with recovery"),
+                                errdetail_recovery_conflict(),
+                                errhint("In a moment you should be able to reconnect to the"
+                                                " database and repeat your command.")));
+       }
+
        if (QueryCancelPending)
        {
+               /*
+                * Don't allow query cancel interrupts while reading input from the
+                * client, because we might lose sync in the FE/BE protocol.  (Die
+                * interrupts are OK, because we won't read any further messages from
+                * the client in that case.)
+                */
+               if (QueryCancelHoldoffCount != 0)
+               {
+                       /*
+                        * Re-arm InterruptPending so that we process the cancel request
+                        * as soon as we're done reading the message.
+                        */
+                       InterruptPending = true;
+                       return;
+               }
+
                QueryCancelPending = false;
                if (ClientAuthInProgress)
                {
                        ImmediateInterruptOK = false;           /* not idle anymore */
+                       LockErrorCleanup();
                        DisableNotifyInterrupt();
                        DisableCatchupInterrupt();
                        /* As in quickdie, don't risk sending to client during auth */
@@ -2903,6 +2944,7 @@ ProcessInterrupts(void)
                if (cancel_from_timeout)
                {
                        ImmediateInterruptOK = false;           /* not idle anymore */
+                       LockErrorCleanup();
                        DisableNotifyInterrupt();
                        DisableCatchupInterrupt();
                        ereport(ERROR,
@@ -2912,6 +2954,7 @@ ProcessInterrupts(void)
                if (IsAutoVacuumWorkerProcess())
                {
                        ImmediateInterruptOK = false;           /* not idle anymore */
+                       LockErrorCleanup();
                        DisableNotifyInterrupt();
                        DisableCatchupInterrupt();
                        ereport(ERROR,
@@ -2922,21 +2965,14 @@ ProcessInterrupts(void)
                {
                        ImmediateInterruptOK = false;           /* not idle anymore */
                        RecoveryConflictPending = false;
+                       LockErrorCleanup();
                        DisableNotifyInterrupt();
                        DisableCatchupInterrupt();
                        pgstat_report_recovery_conflict(RecoveryConflictReason);
-                       if (DoingCommandRead)
-                               ereport(FATAL,
-                                               (errcode(ERRCODE_T_R_SERIALIZATION_FAILURE),
-                                                errmsg("terminating connection due to conflict with recovery"),
-                                                errdetail_recovery_conflict(),
-                                errhint("In a moment you should be able to reconnect to the"
-                                                " database and repeat your command.")));
-                       else
-                               ereport(ERROR,
-                                               (errcode(ERRCODE_T_R_SERIALIZATION_FAILURE),
+                       ereport(ERROR,
+                                       (errcode(ERRCODE_T_R_SERIALIZATION_FAILURE),
                                 errmsg("canceling statement due to conflict with recovery"),
-                                                errdetail_recovery_conflict()));
+                                        errdetail_recovery_conflict()));
                }
 
                /*
@@ -2947,6 +2983,7 @@ ProcessInterrupts(void)
                if (!DoingCommandRead)
                {
                        ImmediateInterruptOK = false;           /* not idle anymore */
+                       LockErrorCleanup();
                        DisableNotifyInterrupt();
                        DisableCatchupInterrupt();
                        ereport(ERROR,
@@ -3851,6 +3888,19 @@ PostgresMain(int argc, char *argv[],
                /* We don't have a transaction command open anymore */
                xact_started = false;
 
+               /*
+                * If an error occurred while we were reading a message from the
+                * client, we have potentially lost track of where the previous
+                * message ends and the next one begins.  Even though we have
+                * otherwise recovered from the error, we cannot safely read any more
+                * messages from the client, so there isn't much we can do with the
+                * connection anymore.
+                */
+               if (pq_is_reading_msg())
+                       ereport(FATAL,
+                                       (errcode(ERRCODE_PROTOCOL_VIOLATION),
+                                        errmsg("terminating connection because protocol sync was lost")));
+
                /* Now we can allow interrupts again */
                RESUME_INTERRUPTS();
        }
@@ -3935,7 +3985,14 @@ PostgresMain(int argc, char *argv[],
 
                /*
                 * (4) disable async signal conditions again.
+                *
+                * Query cancel is supposed to be a no-op when there is no query in
+                * progress, so if a query cancel arrived while we were idle, just
+                * reset QueryCancelPending. ProcessInterrupts() has that effect when
+                * it's called when DoingCommandRead is set, so check for interrupts
+                * before resetting DoingCommandRead.
                 */
+               CHECK_FOR_INTERRUPTS();
                DoingCommandRead = false;
 
                /*
index 91c6238063f232c88c186c7a013c5f5fc8d15dc2..3dd0bd623c49125607b954593c2069f36b56a05d 100644 (file)
@@ -462,6 +462,7 @@ errfinish(int dummy,...)
                 * while doing error cleanup.
                 */
                InterruptHoldoffCount = 0;
+               QueryCancelHoldoffCount = 0;
 
                CritSectionCount = 0;   /* should be unnecessary, but... */
 
index 4b66bd3e358bb8aea34b165f6ab47477e022305d..e4928a2815accc3919f3215005c70e9cc799ed10 100644 (file)
@@ -32,6 +32,7 @@ volatile bool ProcDiePending = false;
 volatile bool ClientConnectionLost = false;
 volatile bool ImmediateInterruptOK = false;
 volatile uint32 InterruptHoldoffCount = 0;
+volatile uint32 QueryCancelHoldoffCount = 0;
 volatile uint32 CritSectionCount = 0;
 
 int                    MyProcPid;
index 7083cd866b68ea906ab66b4f59a9e3b6dfd47885..6dd91bae22faedb6025ffacdf9a2bce287fc46bb 100644 (file)
@@ -54,6 +54,9 @@ extern void pq_init(void);
 extern void pq_comm_reset(void);
 extern int     pq_getbytes(char *s, size_t len);
 extern int     pq_getstring(StringInfo s);
+extern void pq_startmsgread(void);
+extern void pq_endmsgread(void);
+extern bool pq_is_reading_msg(void);
 extern int     pq_getmessage(StringInfo s, int maxlen);
 extern int     pq_getbyte(void);
 extern int     pq_peekbyte(void);
index cd470649d18c2e7ad4ad1e7d3a2f89846aead078..1051ca488434c47aaea06132116a1c7e256b4b80 100644 (file)
  * will be held off until CHECK_FOR_INTERRUPTS() is done outside any
  * HOLD_INTERRUPTS() ... RESUME_INTERRUPTS() section.
  *
+ * There is also a mechanism to prevent query cancel interrupts, while still
+ * allowing die interrupts: HOLD_CANCEL_INTERRUPTS() and
+ * RESUME_CANCEL_INTERRUPTS().
+ *
  * Special mechanisms are used to let an interrupt be accepted when we are
  * waiting for a lock or when we are waiting for command input (but, of
  * course, only if the interrupt holdoff counter is zero).  See the
@@ -80,6 +84,7 @@ extern volatile bool ClientConnectionLost;
 /* these are marked volatile because they are examined by signal handlers: */
 extern volatile bool ImmediateInterruptOK;
 extern PGDLLIMPORT volatile uint32 InterruptHoldoffCount;
+extern PGDLLIMPORT volatile uint32 QueryCancelHoldoffCount;
 extern PGDLLIMPORT volatile uint32 CritSectionCount;
 
 /* in tcop/postgres.c */
@@ -112,6 +117,14 @@ do { \
        InterruptHoldoffCount--; \
 } while(0)
 
+#define HOLD_CANCEL_INTERRUPTS()  (QueryCancelHoldoffCount++)
+
+#define RESUME_CANCEL_INTERRUPTS() \
+do { \
+       Assert(QueryCancelHoldoffCount > 0); \
+       QueryCancelHoldoffCount--; \
+} while(0)
+
 #define START_CRIT_SECTION()  (CritSectionCount++)
 
 #define END_CRIT_SECTION() \
index 1b94cb5d46badbfa5d7783faaad0b131fe27b31b..09f63c9732f80c6b7af8bb777fbd25d84d988c6c 100644 (file)
@@ -15,6 +15,7 @@
 
 #include "lib/stringinfo.h"
 
+extern int GetOldFunctionMessage(StringInfo buf);
 extern int     HandleFunctionRequest(StringInfo msgBuf);
 
 #endif   /* FASTPATH_H */