Skip to content

Commit 4c6d0ab

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 6e5a39c commit 4c6d0ab

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
@@ -13369,6 +13369,17 @@ fi
1336913369

1337013370
fi
1337113371

13372+
# PGAC_LDAP_SAFE
13373+
# --------------
13374+
# PostgreSQL sometimes loads libldap_r and plain libldap into the same
13375+
# process. Check for OpenLDAP versions known not to tolerate doing so; assume
13376+
# non-OpenLDAP implementations are safe. The dblink test suite exercises the
13377+
# hazardous interaction directly.
13378+
13379+
13380+
13381+
13382+
1337213383
if test "$with_ldap" = yes ; then
1337313384
if test "$PORTNAME" != "win32"; then
1337413385

@@ -13526,6 +13537,72 @@ fi
1352613537

1352713538
done
1352813539

13540+
{ $as_echo "$as_me:$LINENO: checking for compatible LDAP implementation" >&5
13541+
$as_echo_n "checking for compatible LDAP implementation... " >&6; }
13542+
if test "${pgac_cv_ldap_safe+set}" = set; then
13543+
$as_echo_n "(cached) " >&6
13544+
else
13545+
cat >conftest.$ac_ext <<_ACEOF
13546+
/* confdefs.h. */
13547+
_ACEOF
13548+
cat confdefs.h >>conftest.$ac_ext
13549+
cat >>conftest.$ac_ext <<_ACEOF
13550+
/* end confdefs.h. */
13551+
#include <ldap.h>
13552+
#if !defined(LDAP_VENDOR_VERSION) || \
13553+
(defined(LDAP_API_FEATURE_X_OPENLDAP) && \
13554+
LDAP_VENDOR_VERSION >= 20424 && LDAP_VENDOR_VERSION <= 20431)
13555+
choke me
13556+
#endif
13557+
int
13558+
main ()
13559+
{
13560+
13561+
;
13562+
return 0;
13563+
}
13564+
_ACEOF
13565+
rm -f conftest.$ac_objext
13566+
if { (ac_try="$ac_compile"
13567+
case "(($ac_try" in
13568+
*\"* | *\`* | *\\*) ac_try_echo=\$ac_try;;
13569+
*) ac_try_echo=$ac_try;;
13570+
esac
13571+
eval ac_try_echo="\"\$as_me:$LINENO: $ac_try_echo\""
13572+
$as_echo "$ac_try_echo") >&5
13573+
(eval "$ac_compile") 2>conftest.er1
13574+
ac_status=$?
13575+
grep -v '^ *+' conftest.er1 >conftest.err
13576+
rm -f conftest.er1
13577+
cat conftest.err >&5
13578+
$as_echo "$as_me:$LINENO: \$? = $ac_status" >&5
13579+
(exit $ac_status); } && {
13580+
test -z "$ac_c_werror_flag" ||
13581+
test ! -s conftest.err
13582+
} && test -s conftest.$ac_objext; then
13583+
pgac_cv_ldap_safe=yes
13584+
else
13585+
$as_echo "$as_me: failed program was:" >&5
13586+
sed 's/^/| /' conftest.$ac_ext >&5
13587+
13588+
pgac_cv_ldap_safe=no
13589+
fi
13590+
13591+
rm -f core conftest.err conftest.$ac_objext conftest.$ac_ext
13592+
fi
13593+
{ $as_echo "$as_me:$LINENO: result: $pgac_cv_ldap_safe" >&5
13594+
$as_echo "$pgac_cv_ldap_safe" >&6; }
13595+
13596+
if test "$pgac_cv_ldap_safe" != yes; then
13597+
{ $as_echo "$as_me:$LINENO: WARNING:
13598+
*** With OpenLDAP versions 2.4.24 through 2.4.31, inclusive, each backend
13599+
*** process that loads libpq (via WAL receiver, dblink, or postgres_fdw) and
13600+
*** also uses LDAP will crash on exit." >&5
13601+
$as_echo "$as_me: WARNING:
13602+
*** With OpenLDAP versions 2.4.24 through 2.4.31, inclusive, each backend
13603+
*** process that loads libpq (via WAL receiver, dblink, or postgres_fdw) and
13604+
*** also uses LDAP will crash on exit." >&2;}
13605+
fi
1352913606
else
1353013607

1353113608
for ac_header in winldap.h

‎configure.in

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

1064+
# PGAC_LDAP_SAFE
1065+
# --------------
1066+
# PostgreSQL sometimes loads libldap_r and plain libldap into the same
1067+
# process. Check for OpenLDAP versions known not to tolerate doing so; assume
1068+
# non-OpenLDAP implementations are safe. The dblink test suite exercises the
1069+
# hazardous interaction directly.
1070+
1071+
AC_DEFUN([PGAC_LDAP_SAFE],
1072+
[AC_CACHE_CHECK([for compatible LDAP implementation], [pgac_cv_ldap_safe],
1073+
[AC_COMPILE_IFELSE([AC_LANG_PROGRAM(
1074+
[#include <ldap.h>
1075+
#if !defined(LDAP_VENDOR_VERSION) || \
1076+
(defined(LDAP_API_FEATURE_X_OPENLDAP) && \
1077+
LDAP_VENDOR_VERSION >= 20424 && LDAP_VENDOR_VERSION <= 20431)
1078+
choke me
1079+
#endif], [])],
1080+
[pgac_cv_ldap_safe=yes],
1081+
[pgac_cv_ldap_safe=no])])
1082+
1083+
if test "$pgac_cv_ldap_safe" != yes; then
1084+
AC_MSG_WARN([
1085+
*** With OpenLDAP versions 2.4.24 through 2.4.31, inclusive, each backend
1086+
*** process that loads libpq (via WAL receiver, dblink, or postgres_fdw) and
1087+
*** also uses LDAP will crash on exit.])
1088+
fi])
1089+
1090+
1091+
10641092
if test "$with_ldap" = yes ; then
10651093
if test "$PORTNAME" != "win32"; then
10661094
AC_CHECK_HEADERS(ldap.h, [],
10671095
[AC_MSG_ERROR([header file <ldap.h> is required for LDAP])])
1096+
PGAC_LDAP_SAFE
10681097
else
10691098
AC_CHECK_HEADERS(winldap.h, [],
10701099
[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
@@ -7,7 +7,9 @@ SHLIB_LINK = $(libpq)
77

88
DATA_built = dblink.sql
99
DATA = uninstall_dblink.sql
10-
REGRESS = dblink
10+
REGRESS = paths dblink
11+
REGRESS_OPTS = --dbname=$(CONTRIB_TESTDB) --dlpath=$(top_builddir)/src/test/regress
12+
EXTRA_CLEAN = sql/paths.sql expected/paths.out
1113

1214

1315
# the db name is hard-coded in the tests

‎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
@@ -115,6 +115,29 @@ SELECT *
115115
FROM dblink('SELECT * FROM foo') AS t(a int, b text, c text[])
116116
WHERE t.a > 7;
117117
ERROR: connection not available
118+
-- The first-level connection's backend will crash on exit given OpenLDAP
119+
-- [2.4.24, 2.4.31]. We won't see evidence of any crash until the victim
120+
-- process terminates and the postmaster responds. If process termination
121+
-- entails writing a core dump, that can take awhile. Wait for the process to
122+
-- vanish. At that point, the postmaster has called waitpid() on the crashed
123+
-- process, and it will accept no new connections until it has reinitialized
124+
-- the cluster. (We can't exploit pg_stat_activity, because the crash happens
125+
-- after the backend updates shared memory to reflect its impending exit.)
126+
DO $pl$
127+
BEGIN
128+
PERFORM wait_pid(crash_pid)
129+
FROM dblink('dbname=contrib_regression', $$
130+
SELECT pg_backend_pid() FROM dblink(
131+
'service=test_ldap dbname=contrib_regression',
132+
-- This string concatenation is a hack to shoehorn a
133+
-- set_pgservicefile call into the SQL statement.
134+
'SELECT 1' || set_pgservicefile('pg_service.conf')
135+
) t(c int)
136+
$$) AS t(crash_pid int);
137+
EXCEPTION WHEN sqlclient_unable_to_establish_sqlconnection THEN
138+
-- Expect "syntax error in service file" in a non-LDAP build.
139+
END
140+
$pl$;
118141
-- create a persistent connection
119142
SELECT dblink_connect('dbname=contrib_regression');
120143
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
@@ -78,6 +78,30 @@ SELECT *
7878
FROM dblink('SELECT * FROM foo') AS t(a int, b text, c text[])
7979
WHERE t.a > 7;
8080

81+
-- The first-level connection's backend will crash on exit given OpenLDAP
82+
-- [2.4.24, 2.4.31]. We won't see evidence of any crash until the victim
83+
-- process terminates and the postmaster responds. If process termination
84+
-- entails writing a core dump, that can take awhile. Wait for the process to
85+
-- vanish. At that point, the postmaster has called waitpid() on the crashed
86+
-- process, and it will accept no new connections until it has reinitialized
87+
-- the cluster. (We can't exploit pg_stat_activity, because the crash happens
88+
-- after the backend updates shared memory to reflect its impending exit.)
89+
DO $pl$
90+
BEGIN
91+
PERFORM wait_pid(crash_pid)
92+
FROM dblink('dbname=contrib_regression', $$
93+
SELECT pg_backend_pid() FROM dblink(
94+
'service=test_ldap dbname=contrib_regression',
95+
-- This string concatenation is a hack to shoehorn a
96+
-- set_pgservicefile call into the SQL statement.
97+
'SELECT 1' || set_pgservicefile('pg_service.conf')
98+
) t(c int)
99+
$$) AS t(crash_pid int);
100+
EXCEPTION WHEN sqlclient_unable_to_establish_sqlconnection THEN
101+
-- Expect "syntax error in service file" in a non-LDAP build.
102+
END
103+
$pl$;
104+
81105
-- create a persistent connection
82106
SELECT dblink_connect('dbname=contrib_regression');
83107

‎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)