Avoid calling strerror[_r] in PQcancel().
authorTom Lane <tgl@sss.pgh.pa.us>
Mon, 17 Jan 2022 17:52:44 +0000 (12:52 -0500)
committerTom Lane <tgl@sss.pgh.pa.us>
Mon, 17 Jan 2022 17:52:44 +0000 (12:52 -0500)
PQcancel() is supposed to be safe to call from a signal handler,
and indeed psql uses it that way.  All of the library functions
it uses are specified to be async-signal-safe by POSIX ...
except for strerror.  Neither plain strerror nor strerror_r
are considered safe.  When this code was written, back in the
dark ages, we probably figured "oh, strerror will just index
into a constant array of strings" ... but in any locale except C,
that's unlikely to be true.  Probably the reason we've not heard
complaints is that (a) this error-handling code is unlikely to be
reached in normal use, and (b) in many scenarios, localized error
strings would already have been loaded, after which maybe it's
safe to call strerror here.  Still, this is clearly unacceptable.

The best we can do without relying on strerror is to print the
decimal value of errno, so make it do that instead.  (This is
probably not much loss of user-friendliness, given that it is
hard to get a failure here.)

Back-patch to all supported branches.

Discussion: https://postgr.es/m/2937814.1641960929@sss.pgh.pa.us

src/interfaces/libpq/fe-connect.c

index 5fc16be849fb272b0574fa36cb2e7c7d2418c1a6..fe3af855d85284c5f3c68ae16ababdfafa277d20 100644 (file)
@@ -4419,7 +4419,6 @@ internal_cancel(SockAddr *raddr, int be_pid, int be_key,
 {
    int         save_errno = SOCK_ERRNO;
    pgsocket    tmpsock = PGINVALID_SOCKET;
-   char        sebuf[PG_STRERROR_R_BUFLEN];
    int         maxlen;
    struct
    {
@@ -4498,8 +4497,25 @@ cancel_errReturn:
    maxlen = errbufsize - strlen(errbuf) - 2;
    if (maxlen >= 0)
    {
-       strncat(errbuf, SOCK_STRERROR(SOCK_ERRNO, sebuf, sizeof(sebuf)),
-               maxlen);
+       /*
+        * We can't invoke strerror here, since it's not signal-safe.  Settle
+        * for printing the decimal value of errno.  Even that has to be done
+        * the hard way.
+        */
+       int         val = SOCK_ERRNO;
+       char        buf[32];
+       char       *bufp;
+
+       bufp = buf + sizeof(buf) - 1;
+       *bufp = '\0';
+       do
+       {
+           *(--bufp) = (val % 10) + '0';
+           val /= 10;
+       } while (val > 0);
+       bufp -= 6;
+       memcpy(bufp, "error ", 6);
+       strncat(errbuf, bufp, maxlen);
        strcat(errbuf, "\n");
    }
    if (tmpsock != PGINVALID_SOCKET)