Skip to content

Commit cec0c21

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 f54d97c commit cec0c21

File tree

11 files changed

+244
-1
lines changed

11 files changed

+244
-1
lines changed

‎configure

Lines changed: 77 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13663,6 +13663,17 @@ fi
1366313663

1366413664
fi
1366513665

13666+
# PGAC_LDAP_SAFE
13667+
# --------------
13668+
# PostgreSQL sometimes loads libldap_r and plain libldap into the same
13669+
# process. Check for OpenLDAP versions known not to tolerate doing so; assume
13670+
# non-OpenLDAP implementations are safe. The dblink test suite exercises the
13671+
# hazardous interaction directly.
13672+
13673+
13674+
13675+
13676+
1366613677
if test "$with_ldap" = yes ; then
1366713678
if test "$PORTNAME" != "win32"; then
1366813679

@@ -13820,6 +13831,72 @@ fi
1382013831

1382113832
done
1382213833

13834+
{ $as_echo "$as_me:$LINENO: checking for compatible LDAP implementation" >&5
13835+
$as_echo_n "checking for compatible LDAP implementation... " >&6; }
13836+
if test "${pgac_cv_ldap_safe+set}" = set; then
13837+
$as_echo_n "(cached) " >&6
13838+
else
13839+
cat >conftest.$ac_ext <<_ACEOF
13840+
/* confdefs.h. */
13841+
_ACEOF
13842+
cat confdefs.h >>conftest.$ac_ext
13843+
cat >>conftest.$ac_ext <<_ACEOF
13844+
/* end confdefs.h. */
13845+
#include <ldap.h>
13846+
#if !defined(LDAP_VENDOR_VERSION) || \
13847+
(defined(LDAP_API_FEATURE_X_OPENLDAP) && \
13848+
LDAP_VENDOR_VERSION >= 20424 && LDAP_VENDOR_VERSION <= 20431)
13849+
choke me
13850+
#endif
13851+
int
13852+
main ()
13853+
{
13854+
13855+
;
13856+
return 0;
13857+
}
13858+
_ACEOF
13859+
rm -f conftest.$ac_objext
13860+
if { (ac_try="$ac_compile"
13861+
case "(($ac_try" in
13862+
*\"* | *\`* | *\\*) ac_try_echo=\$ac_try;;
13863+
*) ac_try_echo=$ac_try;;
13864+
esac
13865+
eval ac_try_echo="\"\$as_me:$LINENO: $ac_try_echo\""
13866+
$as_echo "$ac_try_echo") >&5
13867+
(eval "$ac_compile") 2>conftest.er1
13868+
ac_status=$?
13869+
grep -v '^ *+' conftest.er1 >conftest.err
13870+
rm -f conftest.er1
13871+
cat conftest.err >&5
13872+
$as_echo "$as_me:$LINENO: \$? = $ac_status" >&5
13873+
(exit $ac_status); } && {
13874+
test -z "$ac_c_werror_flag" ||
13875+
test ! -s conftest.err
13876+
} && test -s conftest.$ac_objext; then
13877+
pgac_cv_ldap_safe=yes
13878+
else
13879+
$as_echo "$as_me: failed program was:" >&5
13880+
sed 's/^/| /' conftest.$ac_ext >&5
13881+
13882+
pgac_cv_ldap_safe=no
13883+
fi
13884+
13885+
rm -f core conftest.err conftest.$ac_objext conftest.$ac_ext
13886+
fi
13887+
{ $as_echo "$as_me:$LINENO: result: $pgac_cv_ldap_safe" >&5
13888+
$as_echo "$pgac_cv_ldap_safe" >&6; }
13889+
13890+
if test "$pgac_cv_ldap_safe" != yes; then
13891+
{ $as_echo "$as_me:$LINENO: WARNING:
13892+
*** With OpenLDAP versions 2.4.24 through 2.4.31, inclusive, each backend
13893+
*** process that loads libpq (via WAL receiver, dblink, or postgres_fdw) and
13894+
*** also uses LDAP will crash on exit." >&5
13895+
$as_echo "$as_me: WARNING:
13896+
*** With OpenLDAP versions 2.4.24 through 2.4.31, inclusive, each backend
13897+
*** process that loads libpq (via WAL receiver, dblink, or postgres_fdw) and
13898+
*** also uses LDAP will crash on exit." >&2;}
13899+
fi
1382313900
else
1382413901

1382513902
for ac_header in winldap.h

‎configure.in

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

1070+
# PGAC_LDAP_SAFE
1071+
# --------------
1072+
# PostgreSQL sometimes loads libldap_r and plain libldap into the same
1073+
# process. Check for OpenLDAP versions known not to tolerate doing so; assume
1074+
# non-OpenLDAP implementations are safe. The dblink test suite exercises the
1075+
# hazardous interaction directly.
1076+
1077+
AC_DEFUN([PGAC_LDAP_SAFE],
1078+
[AC_CACHE_CHECK([for compatible LDAP implementation], [pgac_cv_ldap_safe],
1079+
[AC_COMPILE_IFELSE([AC_LANG_PROGRAM(
1080+
[#include <ldap.h>
1081+
#if !defined(LDAP_VENDOR_VERSION) || \
1082+
(defined(LDAP_API_FEATURE_X_OPENLDAP) && \
1083+
LDAP_VENDOR_VERSION >= 20424 && LDAP_VENDOR_VERSION <= 20431)
1084+
choke me
1085+
#endif], [])],
1086+
[pgac_cv_ldap_safe=yes],
1087+
[pgac_cv_ldap_safe=no])])
1088+
1089+
if test "$pgac_cv_ldap_safe" != yes; then
1090+
AC_MSG_WARN([
1091+
*** With OpenLDAP versions 2.4.24 through 2.4.31, inclusive, each backend
1092+
*** process that loads libpq (via WAL receiver, dblink, or postgres_fdw) and
1093+
*** also uses LDAP will crash on exit.])
1094+
fi])
1095+
1096+
1097+
10701098
if test "$with_ldap" = yes ; then
10711099
if test "$PORTNAME" != "win32"; then
10721100
AC_CHECK_HEADERS(ldap.h, [],
10731101
[AC_MSG_ERROR([header file <ldap.h> is required for LDAP])])
1102+
PGAC_LDAP_SAFE
10741103
else
10751104
AC_CHECK_HEADERS(winldap.h, [],
10761105
[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
@@ -9,7 +9,9 @@ SHLIB_PREREQS = submake-libpq
99
EXTENSION = dblink
1010
DATA = dblink--1.0.sql dblink--unpackaged--1.0.sql
1111

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

1416
# the db name is hard-coded in the tests
1517
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
@@ -105,6 +105,33 @@ SELECT *
105105
FROM dblink('SELECT * FROM foo') AS t(a int, b text, c text[])
106106
WHERE t.a > 7;
107107
ERROR: connection not available
108+
-- The first-level connection's backend will crash on exit given OpenLDAP
109+
-- [2.4.24, 2.4.31]. We won't see evidence of any crash until the victim
110+
-- process terminates and the postmaster responds. If process termination
111+
-- entails writing a core dump, that can take awhile. Wait for the process to
112+
-- vanish. At that point, the postmaster has called waitpid() on the crashed
113+
-- process, and it will accept no new connections until it has reinitialized
114+
-- the cluster. (We can't exploit pg_stat_activity, because the crash happens
115+
-- after the backend updates shared memory to reflect its impending exit.)
116+
DO $pl$
117+
DECLARE
118+
detail text;
119+
BEGIN
120+
PERFORM wait_pid(crash_pid)
121+
FROM dblink('dbname=contrib_regression', $$
122+
SELECT pg_backend_pid() FROM dblink(
123+
'service=test_ldap dbname=contrib_regression',
124+
-- This string concatenation is a hack to shoehorn a
125+
-- set_pgservicefile call into the SQL statement.
126+
'SELECT 1' || set_pgservicefile('pg_service.conf')
127+
) t(c int)
128+
$$) AS t(crash_pid int);
129+
EXCEPTION WHEN OTHERS THEN
130+
GET STACKED DIAGNOSTICS detail = PG_EXCEPTION_DETAIL;
131+
-- Expected error in a non-LDAP build.
132+
IF NOT detail LIKE 'syntax error in service file%' THEN RAISE; END IF;
133+
END
134+
$pl$;
108135
-- create a persistent connection
109136
SELECT dblink_connect('dbname=contrib_regression');
110137
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: 46 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/transam.h"
1112
#include "access/xact.h"
@@ -14,8 +15,10 @@
1415
#include "commands/trigger.h"
1516
#include "executor/executor.h"
1617
#include "executor/spi.h"
18+
#include "miscadmin.h"
1719
#include "utils/builtins.h"
1820
#include "utils/geo_decls.h"
21+
#include "utils/memutils.h"
1922
#include "utils/rel.h"
2023

2124

@@ -35,6 +38,8 @@ extern char *reverse_name(char *string);
3538
extern int oldstyle_length(int n, text *t);
3639
extern Datum int44in(PG_FUNCTION_ARGS);
3740
extern Datum int44out(PG_FUNCTION_ARGS);
41+
extern Datum regress_putenv(PG_FUNCTION_ARGS);
42+
extern Datum wait_pid(PG_FUNCTION_ARGS);
3843

3944
#ifdef PG_MODULE_MAGIC
4045
PG_MODULE_MAGIC;
@@ -737,3 +742,44 @@ int44out(PG_FUNCTION_ARGS)
737742
*--walk = '\0';
738743
PG_RETURN_CSTRING(result);
739744
}
745+
746+
PG_FUNCTION_INFO_V1(regress_putenv);
747+
748+
Datum
749+
regress_putenv(PG_FUNCTION_ARGS)
750+
{
751+
MemoryContext oldcontext;
752+
char *envbuf;
753+
754+
if (!superuser())
755+
elog(ERROR, "must be superuser to change environment variables");
756+
757+
oldcontext = MemoryContextSwitchTo(TopMemoryContext);
758+
envbuf = text_to_cstring((text *) PG_GETARG_POINTER(0));
759+
MemoryContextSwitchTo(oldcontext);
760+
761+
if (putenv(envbuf) != 0)
762+
elog(ERROR, "could not set environment variable: %m");
763+
764+
PG_RETURN_VOID();
765+
}
766+
767+
/* Sleep until no process has a given PID. */
768+
PG_FUNCTION_INFO_V1(wait_pid);
769+
770+
Datum
771+
wait_pid(PG_FUNCTION_ARGS)
772+
{
773+
int pid = PG_GETARG_INT32(0);
774+
775+
if (!superuser())
776+
elog(ERROR, "must be superuser to check PID liveness");
777+
778+
while (kill(pid, 0) == 0)
779+
pg_usleep(50000);
780+
781+
if (errno != ESRCH)
782+
elog(ERROR, "could not check PID %d liveness: %m", pid);
783+
784+
PG_RETURN_VOID();
785+
}

0 commit comments

Comments
 (0)