Skip to content

Commit 81af418

Browse files
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 124331b commit 81af418

File tree

11 files changed

+236
-1
lines changed

11 files changed

+236
-1
lines changed

configure

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

1379713797
fi
1379813798

13799+
# PGAC_LDAP_SAFE
13800+
# --------------
13801+
# PostgreSQL sometimes loads libldap_r and plain libldap into the same
13802+
# process. Check for OpenLDAP versions known not to tolerate doing so; assume
13803+
# non-OpenLDAP implementations are safe. The dblink test suite exercises the
13804+
# hazardous interaction directly.
13805+
13806+
13807+
13808+
13809+
1379913810
if test "$with_ldap" = yes ; then
1380013811
if test "$PORTNAME" != "win32"; then
1380113812

@@ -13953,6 +13964,72 @@ fi
1395313964

1395413965
done
1395513966

13967+
{ $as_echo "$as_me:$LINENO: checking for compatible LDAP implementation" >&5
13968+
$as_echo_n "checking for compatible LDAP implementation... " >&6; }
13969+
if test "${pgac_cv_ldap_safe+set}" = set; then
13970+
$as_echo_n "(cached) " >&6
13971+
else
13972+
cat >conftest.$ac_ext <<_ACEOF
13973+
/* confdefs.h. */
13974+
_ACEOF
13975+
cat confdefs.h >>conftest.$ac_ext
13976+
cat >>conftest.$ac_ext <<_ACEOF
13977+
/* end confdefs.h. */
13978+
#include <ldap.h>
13979+
#if !defined(LDAP_VENDOR_VERSION) || \
13980+
(defined(LDAP_API_FEATURE_X_OPENLDAP) && \
13981+
LDAP_VENDOR_VERSION >= 20424 && LDAP_VENDOR_VERSION <= 20431)
13982+
choke me
13983+
#endif
13984+
int
13985+
main ()
13986+
{
13987+
13988+
;
13989+
return 0;
13990+
}
13991+
_ACEOF
13992+
rm -f conftest.$ac_objext
13993+
if { (ac_try="$ac_compile"
13994+
case "(($ac_try" in
13995+
*\"* | *\`* | *\\*) ac_try_echo=\$ac_try;;
13996+
*) ac_try_echo=$ac_try;;
13997+
esac
13998+
eval ac_try_echo="\"\$as_me:$LINENO: $ac_try_echo\""
13999+
$as_echo "$ac_try_echo") >&5
14000+
(eval "$ac_compile") 2>conftest.er1
14001+
ac_status=$?
14002+
grep -v '^ *+' conftest.er1 >conftest.err
14003+
rm -f conftest.er1
14004+
cat conftest.err >&5
14005+
$as_echo "$as_me:$LINENO: \$? = $ac_status" >&5
14006+
(exit $ac_status); } && {
14007+
test -z "$ac_c_werror_flag" ||
14008+
test ! -s conftest.err
14009+
} && test -s conftest.$ac_objext; then
14010+
pgac_cv_ldap_safe=yes
14011+
else
14012+
$as_echo "$as_me: failed program was:" >&5
14013+
sed 's/^/| /' conftest.$ac_ext >&5
14014+
14015+
pgac_cv_ldap_safe=no
14016+
fi
14017+
14018+
rm -f core conftest.err conftest.$ac_objext conftest.$ac_ext
14019+
fi
14020+
{ $as_echo "$as_me:$LINENO: result: $pgac_cv_ldap_safe" >&5
14021+
$as_echo "$pgac_cv_ldap_safe" >&6; }
14022+
14023+
if test "$pgac_cv_ldap_safe" != yes; then
14024+
{ $as_echo "$as_me:$LINENO: WARNING:
14025+
*** With OpenLDAP versions 2.4.24 through 2.4.31, inclusive, each backend
14026+
*** process that loads libpq (via WAL receiver, dblink, or postgres_fdw) and
14027+
*** also uses LDAP will crash on exit." >&5
14028+
$as_echo "$as_me: WARNING:
14029+
*** With OpenLDAP versions 2.4.24 through 2.4.31, inclusive, each backend
14030+
*** process that loads libpq (via WAL receiver, dblink, or postgres_fdw) and
14031+
*** also uses LDAP will crash on exit." >&2;}
14032+
fi
1395614033
else
1395714034

1395814035
for ac_header in winldap.h

configure.in

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

1091+
# PGAC_LDAP_SAFE
1092+
# --------------
1093+
# PostgreSQL sometimes loads libldap_r and plain libldap into the same
1094+
# process. Check for OpenLDAP versions known not to tolerate doing so; assume
1095+
# non-OpenLDAP implementations are safe. The dblink test suite exercises the
1096+
# hazardous interaction directly.
1097+
1098+
AC_DEFUN([PGAC_LDAP_SAFE],
1099+
[AC_CACHE_CHECK([for compatible LDAP implementation], [pgac_cv_ldap_safe],
1100+
[AC_COMPILE_IFELSE([AC_LANG_PROGRAM(
1101+
[#include <ldap.h>
1102+
#if !defined(LDAP_VENDOR_VERSION) || \
1103+
(defined(LDAP_API_FEATURE_X_OPENLDAP) && \
1104+
LDAP_VENDOR_VERSION >= 20424 && LDAP_VENDOR_VERSION <= 20431)
1105+
choke me
1106+
#endif], [])],
1107+
[pgac_cv_ldap_safe=yes],
1108+
[pgac_cv_ldap_safe=no])])
1109+
1110+
if test "$pgac_cv_ldap_safe" != yes; then
1111+
AC_MSG_WARN([
1112+
*** With OpenLDAP versions 2.4.24 through 2.4.31, inclusive, each backend
1113+
*** process that loads libpq (via WAL receiver, dblink, or postgres_fdw) and
1114+
*** also uses LDAP will crash on exit.])
1115+
fi])
1116+
1117+
1118+
10911119
if test "$with_ldap" = yes ; then
10921120
if test "$PORTNAME" != "win32"; then
10931121
AC_CHECK_HEADERS(ldap.h, [],
10941122
[AC_MSG_ERROR([header file <ldap.h> is required for LDAP])])
1123+
PGAC_LDAP_SAFE
10951124
else
10961125
AC_CHECK_HEADERS(winldap.h, [],
10971126
[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: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -105,6 +105,29 @@ 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+
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 sqlclient_unable_to_establish_sqlconnection THEN
128+
-- Expect "syntax error in service file" in a non-LDAP build.
129+
END
130+
$pl$;
108131
-- create a persistent connection
109132
SELECT dblink_connect('dbname=contrib_regression');
110133
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: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,30 @@ 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+
BEGIN
78+
PERFORM wait_pid(crash_pid)
79+
FROM dblink('dbname=contrib_regression', $$
80+
SELECT pg_backend_pid() FROM dblink(
81+
'service=test_ldap dbname=contrib_regression',
82+
-- This string concatenation is a hack to shoehorn a
83+
-- set_pgservicefile call into the SQL statement.
84+
'SELECT 1' || set_pgservicefile('pg_service.conf')
85+
) t(c int)
86+
$$) AS t(crash_pid int);
87+
EXCEPTION WHEN sqlclient_unable_to_establish_sqlconnection THEN
88+
-- Expect "syntax error in service file" in a non-LDAP build.
89+
END
90+
$pl$;
91+
6892
-- create a persistent connection
6993
SELECT dblink_connect('dbname=contrib_regression');
7094

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

2023

2124
#define P_MAXDIG 12
@@ -34,6 +37,8 @@ extern char *reverse_name(char *string);
3437
extern int oldstyle_length(int n, text *t);
3538
extern Datum int44in(PG_FUNCTION_ARGS);
3639
extern Datum int44out(PG_FUNCTION_ARGS);
40+
extern Datum regress_putenv(PG_FUNCTION_ARGS);
41+
extern Datum wait_pid(PG_FUNCTION_ARGS);
3742

3843
#ifdef PG_MODULE_MAGIC
3944
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)