Skip to content

Commit d7cdf6e

Browse files
nmischnmisch
committed
Diagnose incompatible OpenLDAP versions during build and test.
With OpenLDAP versions 2.4.24 through 2.4.31, inclusive, PostgreSQL backends can crash at exit. Raise a warning during "configure" based on the compile-time OpenLDAP version number, and test the crash scenario in the dblink test suite. Back-patch to 9.0 (all supported versions).
1 parent 24e786f commit d7cdf6e

File tree

11 files changed

+216
-1
lines changed

11 files changed

+216
-1
lines changed

‎configure

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9475,6 +9475,17 @@ fi
94759475

94769476
fi
94779477

9478+
# PGAC_LDAP_SAFE
9479+
# --------------
9480+
# PostgreSQL sometimes loads libldap_r and plain libldap into the same
9481+
# process. Check for OpenLDAP versions known not to tolerate doing so; assume
9482+
# non-OpenLDAP implementations are safe. The dblink test suite exercises the
9483+
# hazardous interaction directly.
9484+
9485+
9486+
9487+
9488+
94789489
if test "$with_ldap" = yes ; then
94799490
if test "$PORTNAME" != "win32"; then
94809491
for ac_header in ldap.h
@@ -9491,6 +9502,47 @@ fi
94919502

94929503
done
94939504

9505+
{ $as_echo "$as_me:${as_lineno-$LINENO}: checking for compatible LDAP implementation" >&5
9506+
$as_echo_n "checking for compatible LDAP implementation... " >&6; }
9507+
if ${pgac_cv_ldap_safe+:} false; then :
9508+
$as_echo_n "(cached) " >&6
9509+
else
9510+
cat confdefs.h - <<_ACEOF >conftest.$ac_ext
9511+
/* end confdefs.h. */
9512+
#include <ldap.h>
9513+
#if !defined(LDAP_VENDOR_VERSION) || \
9514+
(defined(LDAP_API_FEATURE_X_OPENLDAP) && \
9515+
LDAP_VENDOR_VERSION >= 20424 && LDAP_VENDOR_VERSION <= 20431)
9516+
choke me
9517+
#endif
9518+
int
9519+
main ()
9520+
{
9521+
9522+
;
9523+
return 0;
9524+
}
9525+
_ACEOF
9526+
if ac_fn_c_try_compile "$LINENO"; then :
9527+
pgac_cv_ldap_safe=yes
9528+
else
9529+
pgac_cv_ldap_safe=no
9530+
fi
9531+
rm -f core conftest.err conftest.$ac_objext conftest.$ac_ext
9532+
fi
9533+
{ $as_echo "$as_me:${as_lineno-$LINENO}: result: $pgac_cv_ldap_safe" >&5
9534+
$as_echo "$pgac_cv_ldap_safe" >&6; }
9535+
9536+
if test "$pgac_cv_ldap_safe" != yes; then
9537+
{ $as_echo "$as_me:${as_lineno-$LINENO}: WARNING:
9538+
*** With OpenLDAP versions 2.4.24 through 2.4.31, inclusive, each backend
9539+
*** process that loads libpq (via WAL receiver, dblink, or postgres_fdw) and
9540+
*** also uses LDAP will crash on exit." >&5
9541+
$as_echo "$as_me: WARNING:
9542+
*** With OpenLDAP versions 2.4.24 through 2.4.31, inclusive, each backend
9543+
*** process that loads libpq (via WAL receiver, dblink, or postgres_fdw) and
9544+
*** also uses LDAP will crash on exit." >&2;}
9545+
fi
94949546
else
94959547
for ac_header in winldap.h
94969548
do :

‎configure.in

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1097,10 +1097,39 @@ if test "$with_libxslt" = yes ; then
10971097
AC_CHECK_HEADER(libxslt/xslt.h, [], [AC_MSG_ERROR([header file <libxslt/xslt.h> is required for XSLT support])])
10981098
fi
10991099

1100+
# PGAC_LDAP_SAFE
1101+
# --------------
1102+
# PostgreSQL sometimes loads libldap_r and plain libldap into the same
1103+
# process. Check for OpenLDAP versions known not to tolerate doing so; assume
1104+
# non-OpenLDAP implementations are safe. The dblink test suite exercises the
1105+
# hazardous interaction directly.
1106+
1107+
AC_DEFUN([PGAC_LDAP_SAFE],
1108+
[AC_CACHE_CHECK([for compatible LDAP implementation], [pgac_cv_ldap_safe],
1109+
[AC_COMPILE_IFELSE([AC_LANG_PROGRAM(
1110+
[#include <ldap.h>
1111+
#if !defined(LDAP_VENDOR_VERSION) || \
1112+
(defined(LDAP_API_FEATURE_X_OPENLDAP) && \
1113+
LDAP_VENDOR_VERSION >= 20424 && LDAP_VENDOR_VERSION <= 20431)
1114+
choke me
1115+
#endif], [])],
1116+
[pgac_cv_ldap_safe=yes],
1117+
[pgac_cv_ldap_safe=no])])
1118+
1119+
if test "$pgac_cv_ldap_safe" != yes; then
1120+
AC_MSG_WARN([
1121+
*** With OpenLDAP versions 2.4.24 through 2.4.31, inclusive, each backend
1122+
*** process that loads libpq (via WAL receiver, dblink, or postgres_fdw) and
1123+
*** also uses LDAP will crash on exit.])
1124+
fi])
1125+
1126+
1127+
11001128
if test "$with_ldap" = yes ; then
11011129
if test "$PORTNAME" != "win32"; then
11021130
AC_CHECK_HEADERS(ldap.h, [],
11031131
[AC_MSG_ERROR([header file <ldap.h> is required for LDAP])])
1132+
PGAC_LDAP_SAFE
11041133
else
11051134
AC_CHECK_HEADERS(winldap.h, [],
11061135
[AC_MSG_ERROR([header file <winldap.h> is required for LDAP])],

‎contrib/dblink/Makefile

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,9 @@ EXTENSION = dblink
1010
DATA = dblink--1.1.sql dblink--1.0--1.1.sql dblink--unpackaged--1.0.sql
1111
PGFILEDESC = "dblink - connect to other PostgreSQL databases"
1212

13-
REGRESS = dblink
13+
REGRESS = paths dblink
14+
REGRESS_OPTS = --dlpath=$(top_builddir)/src/test/regress
15+
EXTRA_CLEAN = sql/paths.sql expected/paths.out
1416

1517
# the db name is hard-coded in the tests
1618
override USE_MODULE_DB =

‎contrib/dblink/expected/.gitignore

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
/paths.out

‎contrib/dblink/expected/dblink.out

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -103,6 +103,33 @@ SELECT *
103103
FROM dblink('SELECT * FROM foo') AS t(a int, b text, c text[])
104104
WHERE t.a > 7;
105105
ERROR: connection not available
106+
-- The first-level connection's backend will crash on exit given OpenLDAP
107+
-- [2.4.24, 2.4.31]. We won't see evidence of any crash until the victim
108+
-- process terminates and the postmaster responds. If process termination
109+
-- entails writing a core dump, that can take awhile. Wait for the process to
110+
-- vanish. At that point, the postmaster has called waitpid() on the crashed
111+
-- process, and it will accept no new connections until it has reinitialized
112+
-- the cluster. (We can't exploit pg_stat_activity, because the crash happens
113+
-- after the backend updates shared memory to reflect its impending exit.)
114+
DO $pl$
115+
DECLARE
116+
detail text;
117+
BEGIN
118+
PERFORM wait_pid(crash_pid)
119+
FROM dblink('dbname=contrib_regression', $$
120+
SELECT pg_backend_pid() FROM dblink(
121+
'service=test_ldap dbname=contrib_regression',
122+
-- This string concatenation is a hack to shoehorn a
123+
-- set_pgservicefile call into the SQL statement.
124+
'SELECT 1' || set_pgservicefile('pg_service.conf')
125+
) t(c int)
126+
$$) AS t(crash_pid int);
127+
EXCEPTION WHEN OTHERS THEN
128+
GET STACKED DIAGNOSTICS detail = PG_EXCEPTION_DETAIL;
129+
-- Expected error in a non-LDAP build.
130+
IF NOT detail LIKE 'syntax error in service file%' THEN RAISE; END IF;
131+
END
132+
$pl$;
106133
-- create a persistent connection
107134
SELECT dblink_connect('dbname=contrib_regression');
108135
dblink_connect

‎contrib/dblink/input/paths.source

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
-- Initialization that requires path substitution.
2+
3+
CREATE FUNCTION putenv(text)
4+
RETURNS void
5+
AS '@libdir@/regress@DLSUFFIX@', 'regress_putenv'
6+
LANGUAGE C STRICT;
7+
8+
CREATE FUNCTION wait_pid(int)
9+
RETURNS void
10+
AS '@libdir@/regress@DLSUFFIX@'
11+
LANGUAGE C STRICT;
12+
13+
CREATE FUNCTION set_pgservicefile(text) RETURNS void LANGUAGE SQL
14+
AS $$SELECT putenv('PGSERVICEFILE=@abs_srcdir@/' || $1)$$;

‎contrib/dblink/output/paths.source

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
-- Initialization that requires path substitution.
2+
CREATE FUNCTION putenv(text)
3+
RETURNS void
4+
AS '@libdir@/regress@DLSUFFIX@', 'regress_putenv'
5+
LANGUAGE C STRICT;
6+
CREATE FUNCTION wait_pid(int)
7+
RETURNS void
8+
AS '@libdir@/regress@DLSUFFIX@'
9+
LANGUAGE C STRICT;
10+
CREATE FUNCTION set_pgservicefile(text) RETURNS void LANGUAGE SQL
11+
AS $$SELECT putenv('PGSERVICEFILE=@abs_srcdir@/' || $1)$$;

‎contrib/dblink/pg_service.conf

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
# pg_service.conf for minimally exercising libpq use of LDAP.
2+
3+
# Having failed to reach an LDAP server, libpq essentially ignores the
4+
# "service=test_ldap" in its connection string. Contact the "discard"
5+
# service; the test works whether or not it answers.
6+
[test_ldap]
7+
ldap://127.0.0.1:9/base?attribute?one?filter

‎contrib/dblink/sql/.gitignore

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
/paths.sql

‎contrib/dblink/sql/dblink.sql

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,34 @@ SELECT *
6565
FROM dblink('SELECT * FROM foo') AS t(a int, b text, c text[])
6666
WHERE t.a > 7;
6767

68+
-- The first-level connection's backend will crash on exit given OpenLDAP
69+
-- [2.4.24, 2.4.31]. We won't see evidence of any crash until the victim
70+
-- process terminates and the postmaster responds. If process termination
71+
-- entails writing a core dump, that can take awhile. Wait for the process to
72+
-- vanish. At that point, the postmaster has called waitpid() on the crashed
73+
-- process, and it will accept no new connections until it has reinitialized
74+
-- the cluster. (We can't exploit pg_stat_activity, because the crash happens
75+
-- after the backend updates shared memory to reflect its impending exit.)
76+
DO $pl$
77+
DECLARE
78+
detail text;
79+
BEGIN
80+
PERFORM wait_pid(crash_pid)
81+
FROM dblink('dbname=contrib_regression', $$
82+
SELECT pg_backend_pid() FROM dblink(
83+
'service=test_ldap dbname=contrib_regression',
84+
-- This string concatenation is a hack to shoehorn a
85+
-- set_pgservicefile call into the SQL statement.
86+
'SELECT 1' || set_pgservicefile('pg_service.conf')
87+
) t(c int)
88+
$$) AS t(crash_pid int);
89+
EXCEPTION WHEN OTHERS THEN
90+
GET STACKED DIAGNOSTICS detail = PG_EXCEPTION_DETAIL;
91+
-- Expected error in a non-LDAP build.
92+
IF NOT detail LIKE 'syntax error in service file%' THEN RAISE; END IF;
93+
END
94+
$pl$;
95+
6896
-- create a persistent connection
6997
SELECT dblink_connect('dbname=contrib_regression');
7098

‎src/test/regress/regress.c

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66

77
#include <float.h>
88
#include <math.h>
9+
#include <signal.h>
910

1011
#include "access/htup_details.h"
1112
#include "access/transam.h"
@@ -16,6 +17,7 @@
1617
#include "commands/trigger.h"
1718
#include "executor/executor.h"
1819
#include "executor/spi.h"
20+
#include "miscadmin.h"
1921
#include "utils/builtins.h"
2022
#include "utils/geo_decls.h"
2123
#include "utils/rel.h"
@@ -822,3 +824,44 @@ make_tuple_indirect(PG_FUNCTION_ARGS)
822824
*/
823825
PG_RETURN_POINTER(newtup->t_data);
824826
}
827+
828+
PG_FUNCTION_INFO_V1(regress_putenv);
829+
830+
Datum
831+
regress_putenv(PG_FUNCTION_ARGS)
832+
{
833+
MemoryContext oldcontext;
834+
char *envbuf;
835+
836+
if (!superuser())
837+
elog(ERROR, "must be superuser to change environment variables");
838+
839+
oldcontext = MemoryContextSwitchTo(TopMemoryContext);
840+
envbuf = text_to_cstring((text *) PG_GETARG_POINTER(0));
841+
MemoryContextSwitchTo(oldcontext);
842+
843+
if (putenv(envbuf) != 0)
844+
elog(ERROR, "could not set environment variable: %m");
845+
846+
PG_RETURN_VOID();
847+
}
848+
849+
/* Sleep until no process has a given PID. */
850+
PG_FUNCTION_INFO_V1(wait_pid);
851+
852+
Datum
853+
wait_pid(PG_FUNCTION_ARGS)
854+
{
855+
int pid = PG_GETARG_INT32(0);
856+
857+
if (!superuser())
858+
elog(ERROR, "must be superuser to check PID liveness");
859+
860+
while (kill(pid, 0) == 0)
861+
pg_usleep(50000);
862+
863+
if (errno != ESRCH)
864+
elog(ERROR, "could not check PID %d liveness: %m", pid);
865+
866+
PG_RETURN_VOID();
867+
}

0 commit comments

Comments
 (0)