Skip to content

Commit 0711524

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 7672bbc commit 0711524

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
@@ -13897,6 +13897,17 @@ fi
1389713897

1389813898
fi
1389913899

13900+
# PGAC_LDAP_SAFE
13901+
# --------------
13902+
# PostgreSQL sometimes loads libldap_r and plain libldap into the same
13903+
# process. Check for OpenLDAP versions known not to tolerate doing so; assume
13904+
# non-OpenLDAP implementations are safe. The dblink test suite exercises the
13905+
# hazardous interaction directly.
13906+
13907+
13908+
13909+
13910+
1390013911
if test "$with_ldap" = yes ; then
1390113912
if test "$PORTNAME" != "win32"; then
1390213913

@@ -14054,6 +14065,72 @@ fi
1405414065

1405514066
done
1405614067

14068+
{ $as_echo "$as_me:$LINENO: checking for compatible LDAP implementation" >&5
14069+
$as_echo_n "checking for compatible LDAP implementation... " >&6; }
14070+
if test "${pgac_cv_ldap_safe+set}" = set; then
14071+
$as_echo_n "(cached) " >&6
14072+
else
14073+
cat >conftest.$ac_ext <<_ACEOF
14074+
/* confdefs.h. */
14075+
_ACEOF
14076+
cat confdefs.h >>conftest.$ac_ext
14077+
cat >>conftest.$ac_ext <<_ACEOF
14078+
/* end confdefs.h. */
14079+
#include <ldap.h>
14080+
#if !defined(LDAP_VENDOR_VERSION) || \
14081+
(defined(LDAP_API_FEATURE_X_OPENLDAP) && \
14082+
LDAP_VENDOR_VERSION >= 20424 && LDAP_VENDOR_VERSION <= 20431)
14083+
choke me
14084+
#endif
14085+
int
14086+
main ()
14087+
{
14088+
14089+
;
14090+
return 0;
14091+
}
14092+
_ACEOF
14093+
rm -f conftest.$ac_objext
14094+
if { (ac_try="$ac_compile"
14095+
case "(($ac_try" in
14096+
*\"* | *\`* | *\\*) ac_try_echo=\$ac_try;;
14097+
*) ac_try_echo=$ac_try;;
14098+
esac
14099+
eval ac_try_echo="\"\$as_me:$LINENO: $ac_try_echo\""
14100+
$as_echo "$ac_try_echo") >&5
14101+
(eval "$ac_compile") 2>conftest.er1
14102+
ac_status=$?
14103+
grep -v '^ *+' conftest.er1 >conftest.err
14104+
rm -f conftest.er1
14105+
cat conftest.err >&5
14106+
$as_echo "$as_me:$LINENO: \$? = $ac_status" >&5
14107+
(exit $ac_status); } && {
14108+
test -z "$ac_c_werror_flag" ||
14109+
test ! -s conftest.err
14110+
} && test -s conftest.$ac_objext; then
14111+
pgac_cv_ldap_safe=yes
14112+
else
14113+
$as_echo "$as_me: failed program was:" >&5
14114+
sed 's/^/| /' conftest.$ac_ext >&5
14115+
14116+
pgac_cv_ldap_safe=no
14117+
fi
14118+
14119+
rm -f core conftest.err conftest.$ac_objext conftest.$ac_ext
14120+
fi
14121+
{ $as_echo "$as_me:$LINENO: result: $pgac_cv_ldap_safe" >&5
14122+
$as_echo "$pgac_cv_ldap_safe" >&6; }
14123+
14124+
if test "$pgac_cv_ldap_safe" != yes; then
14125+
{ $as_echo "$as_me:$LINENO: WARNING:
14126+
*** With OpenLDAP versions 2.4.24 through 2.4.31, inclusive, each backend
14127+
*** process that loads libpq (via WAL receiver, dblink, or postgres_fdw) and
14128+
*** also uses LDAP will crash on exit." >&5
14129+
$as_echo "$as_me: WARNING:
14130+
*** With OpenLDAP versions 2.4.24 through 2.4.31, inclusive, each backend
14131+
*** process that loads libpq (via WAL receiver, dblink, or postgres_fdw) and
14132+
*** also uses LDAP will crash on exit." >&2;}
14133+
fi
1405714134
else
1405814135

1405914136
for ac_header in winldap.h

‎configure.in

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

1083+
# PGAC_LDAP_SAFE
1084+
# --------------
1085+
# PostgreSQL sometimes loads libldap_r and plain libldap into the same
1086+
# process. Check for OpenLDAP versions known not to tolerate doing so; assume
1087+
# non-OpenLDAP implementations are safe. The dblink test suite exercises the
1088+
# hazardous interaction directly.
1089+
1090+
AC_DEFUN([PGAC_LDAP_SAFE],
1091+
[AC_CACHE_CHECK([for compatible LDAP implementation], [pgac_cv_ldap_safe],
1092+
[AC_COMPILE_IFELSE([AC_LANG_PROGRAM(
1093+
[#include <ldap.h>
1094+
#if !defined(LDAP_VENDOR_VERSION) || \
1095+
(defined(LDAP_API_FEATURE_X_OPENLDAP) && \
1096+
LDAP_VENDOR_VERSION >= 20424 && LDAP_VENDOR_VERSION <= 20431)
1097+
choke me
1098+
#endif], [])],
1099+
[pgac_cv_ldap_safe=yes],
1100+
[pgac_cv_ldap_safe=no])])
1101+
1102+
if test "$pgac_cv_ldap_safe" != yes; then
1103+
AC_MSG_WARN([
1104+
*** With OpenLDAP versions 2.4.24 through 2.4.31, inclusive, each backend
1105+
*** process that loads libpq (via WAL receiver, dblink, or postgres_fdw) and
1106+
*** also uses LDAP will crash on exit.])
1107+
fi])
1108+
1109+
1110+
10831111
if test "$with_ldap" = yes ; then
10841112
if test "$PORTNAME" != "win32"; then
10851113
AC_CHECK_HEADERS(ldap.h, [],
10861114
[AC_MSG_ERROR([header file <ldap.h> is required for LDAP])])
1115+
PGAC_LDAP_SAFE
10871116
else
10881117
AC_CHECK_HEADERS(winldap.h, [],
10891118
[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.1.sql dblink--1.0--1.1.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
@@ -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: 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)