]> granicus.if.org Git - postgresql/commitdiff
Install a check for mis-linking of src/port and src/common functions.
authorTom Lane <tgl@sss.pgh.pa.us>
Sun, 9 Sep 2018 16:23:23 +0000 (12:23 -0400)
committerTom Lane <tgl@sss.pgh.pa.us>
Sun, 9 Sep 2018 16:23:23 +0000 (12:23 -0400)
On ELF-based platforms (and maybe others?) it's possible for a shared
library, when dynamically loaded into the backend, to call the backend
versions of src/port and src/common functions rather than the frontend
versions that are actually linked into the shlib.  This is definitely
not what we want, because the frontend versions often behave slightly
differently.  Up to now it's been "slight" enough that nobody noticed;
but with the addition of SCRAM support functions in src/common, we're
observing crashes due to the difference between palloc and malloc
memory allocation rules, as reported in bug #15367 from Jeremy Evans.

The purpose of this patch is to create a direct test for this type of
mis-linking, so that we know whether any given platform requires extra
measures to prevent using the wrong functions.  If the test fails, it
will lead to connection failures in the contrib/postgres_fdw regression
test.  At the moment, *BSD platforms using ELF format are known to have
the problem and can be expected to fail; but we need to know whether
anything else does, and we need a reliable ongoing check for future
platforms.

Actually fixing the problem will be the subject of later commit(s).

Discussion: https://postgr.es/m/153626613985.23143.4743626885618266803@wrigleys.postgresql.org

src/backend/bootstrap/bootstrap.c
src/common/Makefile
src/common/link-canary.c [new file with mode: 0644]
src/include/common/link-canary.h [new file with mode: 0644]
src/interfaces/libpq/.gitignore
src/interfaces/libpq/Makefile
src/interfaces/libpq/fe-connect.c
src/tools/msvc/Mkvcbuild.pm

index cdd71a9bc339bdc5ed4f42de88efab720d8cfaae..578af2e66d8c982ad7bc31b083aaf46a95294096 100644 (file)
@@ -24,6 +24,7 @@
 #include "catalog/index.h"
 #include "catalog/pg_collation.h"
 #include "catalog/pg_type.h"
+#include "common/link-canary.h"
 #include "libpq/pqsignal.h"
 #include "miscadmin.h"
 #include "nodes/makefuncs.h"
@@ -502,6 +503,13 @@ BootstrapModeMain(void)
        Assert(!IsUnderPostmaster);
        Assert(IsBootstrapProcessingMode());
 
+       /*
+        * To ensure that src/common/link-canary.c is linked into the backend, we
+        * must call it from somewhere.  Here is as good as anywhere.
+        */
+       if (pg_link_canary_is_frontend())
+               elog(ERROR, "backend is incorrectly linked to frontend functions");
+
        /*
         * Do backend-like initialization for bootstrap mode
         */
index 1fc2c66225bed8f816c0edff809de68e30f6255c..687174788e50b5397c090e0a3e46dccc07d28fd6 100644 (file)
@@ -41,7 +41,8 @@ override CPPFLAGS := -DFRONTEND $(CPPFLAGS)
 LIBS += $(PTHREAD_LIBS)
 
 OBJS_COMMON = base64.o config_info.o controldata_utils.o exec.o file_perm.o \
-       ip.o keywords.o md5.o pg_lzcompress.o pgfnames.o psprintf.o relpath.o \
+       ip.o keywords.o link-canary.o md5.o pg_lzcompress.o \
+       pgfnames.o psprintf.o relpath.o \
        rmtree.o saslprep.o scram-common.o string.o unicode_norm.o \
        username.o wait_error.o
 
diff --git a/src/common/link-canary.c b/src/common/link-canary.c
new file mode 100644 (file)
index 0000000..5b4e562
--- /dev/null
@@ -0,0 +1,36 @@
+/*-------------------------------------------------------------------------
+ * link-canary.c
+ *       Detect whether src/common functions came from frontend or backend.
+ *
+ * Copyright (c) 2018, PostgreSQL Global Development Group
+ *
+ * IDENTIFICATION
+ *       src/common/link-canary.c
+ *
+ *-------------------------------------------------------------------------
+ */
+#include "c.h"
+
+#include "common/link-canary.h"
+
+/*
+ * This function just reports whether this file was compiled for frontend
+ * or backend environment.  We need this because in some systems, mainly
+ * ELF-based platforms, it is possible for a shlib (such as libpq) loaded
+ * into the backend to call a backend function named XYZ in preference to
+ * the shlib's own function XYZ.  That's bad if the two functions don't
+ * act identically.  This exact situation comes up for many functions in
+ * src/common and src/port, where the same function names exist in both
+ * libpq and the backend but they don't act quite identically.  To verify
+ * that appropriate measures have been taken to prevent incorrect symbol
+ * resolution, libpq should test that this function returns true.
+ */
+bool
+pg_link_canary_is_frontend(void)
+{
+#ifdef FRONTEND
+       return true;
+#else
+       return false;
+#endif
+}
diff --git a/src/include/common/link-canary.h b/src/include/common/link-canary.h
new file mode 100644 (file)
index 0000000..917faae
--- /dev/null
@@ -0,0 +1,17 @@
+/*-------------------------------------------------------------------------
+ *
+ * link-canary.h
+ *       Detect whether src/common functions came from frontend or backend.
+ *
+ * Copyright (c) 2018, PostgreSQL Global Development Group
+ *
+ * src/include/common/link-canary.h
+ *
+ *-------------------------------------------------------------------------
+ */
+#ifndef LINK_CANARY_H
+#define LINK_CANARY_H
+
+extern bool pg_link_canary_is_frontend(void);
+
+#endif                                                 /* LINK_CANARY_H */
index 5c232ae2d1184887fd56d7a41ed817b1eab8f894..ce1576e262d99cc0167229ce3fe44165e49118d7 100644 (file)
@@ -25,6 +25,7 @@
 /ip.c
 /md5.c
 /base64.c
+/link-canary.c
 /scram-common.c
 /sha2.c
 /sha2_openssl.c
index abe0a50e9854b49c5f6344869272d6ae9fdb71a1..ec0122a9012ddf43234281bbb7da8c268b5d8a21 100644 (file)
@@ -49,7 +49,7 @@ endif
 # src/backend/utils/mb
 OBJS += encnames.o wchar.o
 # src/common
-OBJS += base64.o ip.o md5.o scram-common.o saslprep.o unicode_norm.o
+OBJS += base64.o ip.o link-canary.o md5.o scram-common.o saslprep.o unicode_norm.o
 
 ifeq ($(with_openssl),yes)
 OBJS += fe-secure-openssl.o fe-secure-common.o sha2_openssl.o
@@ -106,7 +106,7 @@ backend_src = $(top_srcdir)/src/backend
 chklocale.c crypt.c erand48.c getaddrinfo.c getpeereid.c inet_aton.c inet_net_ntop.c noblock.c open.c system.c pgsleep.c pg_strong_random.c pgstrcasecmp.c pqsignal.c snprintf.c strerror.c strlcpy.c strnlen.c thread.c win32error.c win32setlocale.c: % : $(top_srcdir)/src/port/%
        rm -f $@ && $(LN_S) $< .
 
-ip.c md5.c base64.c scram-common.c sha2.c sha2_openssl.c saslprep.c unicode_norm.c: % : $(top_srcdir)/src/common/%
+ip.c md5.c base64.c link-canary.c scram-common.c sha2.c sha2_openssl.c saslprep.c unicode_norm.c: % : $(top_srcdir)/src/common/%
        rm -f $@ && $(LN_S) $< .
 
 encnames.c wchar.c: % : $(backend_src)/utils/mb/%
@@ -156,7 +156,7 @@ clean distclean: clean-lib
        rm -f pg_config_paths.h
 # Remove files we (may have) symlinked in from src/port and other places
        rm -f chklocale.c crypt.c erand48.c getaddrinfo.c getpeereid.c inet_aton.c inet_net_ntop.c noblock.c open.c system.c pgsleep.c pg_strong_random.c pgstrcasecmp.c pqsignal.c snprintf.c strerror.c strlcpy.c strnlen.c thread.c win32error.c win32setlocale.c
-       rm -f ip.c md5.c base64.c scram-common.c sha2.c sha2_openssl.c saslprep.c unicode_norm.c
+       rm -f ip.c md5.c base64.c link-canary.c scram-common.c sha2.c sha2_openssl.c saslprep.c unicode_norm.c
        rm -f encnames.c wchar.c
 
 maintainer-clean: distclean maintainer-clean-lib
index db5aacd0e9ef0e90e34fc789d88ceba2f0c57540..42cdb971a37e63413ec9898a8990bbd00cb13b1f 100644 (file)
@@ -71,6 +71,7 @@ static int ldapServiceLookup(const char *purl, PQconninfoOption *options,
 #endif
 
 #include "common/ip.h"
+#include "common/link-canary.h"
 #include "common/scram-common.h"
 #include "mb/pg_wchar.h"
 #include "port/pg_bswap.h"
@@ -1748,6 +1749,19 @@ connectDBStart(PGconn *conn)
        if (!conn->options_valid)
                goto connect_errReturn;
 
+       /*
+        * Check for bad linking to backend-internal versions of src/common
+        * functions (see comments in link-canary.c for the reason we need this).
+        * Nobody but developers should see this message, so we don't bother
+        * translating it.
+        */
+       if (!pg_link_canary_is_frontend())
+       {
+               printfPQExpBuffer(&conn->errorMessage,
+                                                 "libpq is incorrectly linked to backend functions\n");
+               goto connect_errReturn;
+       }
+
        /* Ensure our buffers are empty */
        conn->inStart = conn->inCursor = conn->inEnd = 0;
        conn->outCount = 0;
index 14d2a3c4cfde4a315fe1df015ac452800f18fc0c..9ce03ce684e8fbd37c6dd158ccd32af70821035b 100644 (file)
@@ -116,7 +116,8 @@ sub mkvcbuild
 
        our @pgcommonallfiles = qw(
          base64.c config_info.c controldata_utils.c exec.c file_perm.c ip.c
-         keywords.c md5.c pg_lzcompress.c pgfnames.c psprintf.c relpath.c rmtree.c
+         keywords.c link-canary.c md5.c
+         pg_lzcompress.c pgfnames.c psprintf.c relpath.c rmtree.c
          saslprep.c scram-common.c string.c unicode_norm.c username.c
          wait_error.c);