agora inbox for pgsql-bugs@postgresql.org
help / color / mirror / Atom feedPostmaster crashes on SIGHUP when oauth_validator_libraries holds only whitespace
12+ messages / 4 participants
[nested] [flat]
* Postmaster crashes on SIGHUP when oauth_validator_libraries holds only whitespace
@ 2026-09-14 11:22 Grigorev Jurij <ju.grigorev@ftdata.ru>
0 siblings, 1 reply; 12+ messages in thread
From: Grigorev Jurij @ 2026-09-14 11:22 UTC (permalink / raw)
To: pgsql-bugs@lists.postgresql.org <pgsql-bugs@lists.postgresql.org>; +Cc: jacob.champion@enterprisedb.com <jacob.champion@enterprisedb.com>; daniel@yesql.se <daniel@yesql.se>
Hi,
I ran into a postmaster crash while investigating a static analyzer
report against 18.6. I reproduced it on master (92aaf50e230,
--enable-cassert, Debian 13/aarch64).
To reproduce, set the following in postgresql.conf:
oauth_validator_libraries = ' '
and add an OAuth line without validator= to pg_hba.conf:
host all all 127.0.0.1/32 oauth issuer="https://example.com"; scope="openid"
On reload the server log ends here:
LOG: received SIGHUP, reloading configuration files
LOG: parameter "oauth_validator_libraries" changed to " "
gdb then reports SIGSEGV in check_oauth_validator(), via load_hba().
The running instance goes down with every session on it. The same
configuration also prevents startup. pg_hba_file_rules() parses the
file from a regular backend, so the same NULL dereference there kills
the backend and the postmaster restarts the cluster.
check_oauth_validator() treats an unset list as an error by looking at
the raw GUC string, but SplitDirectoriesString() accepts a string of
spaces and returns NIL. The subsequent elemlist->length dereferences
that. This is a misconfiguration rather than a security issue -- the
GUC has PGC_SIGHUP context and is marked GUC_SUPERUSER_ONLY.
I've attached a patch against master that checks the parsed list
instead. It also adds a TAP test for reload. The patched master built
without warnings, and "make check" in src/test/modules/oauth_validator
passed with PG_TEST_EXTRA=oauth.
This appears to affect v18 onward, where OAuth support was added, so it
may need back-patching. The v18 error message has different wording
and would need a small adjustment.
Could someone take a look at the attached patch and let me know
if this is the right fix?
Thanks,
Yuriy=
Attachments:
[application/octet-stream] v1-0001-Fix-postmaster-crash-on-whitespace-only-oauth_valida.patch (4.1K, ../../04fa84f6ebbe400f940e179ebe1070e9@localhost.localdomain/2-v1-0001-Fix-postmaster-crash-on-whitespace-only-oauth_valida.patch)
download | inline diff:
From cf766ea59614e3c72a698e4f6dc4cd919da1b0d5 Mon Sep 17 00:00:00 2001
From: Yuriy Grigoryev <ju.grigorev@ftdata.ru>
Date: Mon, 14 Sep 2026 15:31:22 +0700
Subject: [PATCH] Fix postmaster crash on whitespace-only
oauth_validator_libraries
check_oauth_validator() rejects an unset validator list by looking at the
raw GUC string, which does not cover a value made up of whitespace only.
SplitDirectoriesString() accepts such a value and hands back an empty
list, so when the HBA line carries no validator= option the code went on
to read elemlist->length and dereferenced NIL.
Every entry point into HBA parsing is affected. load_hba() runs in the
postmaster at startup and again on SIGHUP, so reloading a running server
brings the whole instance down along with every session on it, and
pg_hba_file_rules() parses the file from a regular backend, where the
same crash forces a cluster-wide restart.
Check the parse result instead of the raw string, keeping the message the
empty setting already produced. Add a test that reloads a whitespace-only
setting and verifies that the server reports the error and stays up.
---
src/backend/libpq/auth-oauth.c | 31 +++++++++++--------
.../modules/oauth_validator/t/001_server.pl | 16 ++++++++++
2 files changed, 34 insertions(+), 13 deletions(-)
diff --git a/src/backend/libpq/auth-oauth.c b/src/backend/libpq/auth-oauth.c
index b769931..8dfe5f9 100644
--- a/src/backend/libpq/auth-oauth.c
+++ b/src/backend/libpq/auth-oauth.c
@@ -863,19 +863,6 @@ check_oauth_validator(HbaLine *hbaline, int elevel, char **err_msg)
*err_msg = NULL;
- if (oauth_validator_libraries_string[0] == '\0')
- {
- ereport(elevel,
- errcode(ERRCODE_CONFIG_FILE_ERROR),
- errmsg("parameter \"%s\" must be set for authentication method \"%s\"",
- "oauth_validator_libraries", "oauth"),
- errcontext("line %d of configuration file \"%s\"",
- line_num, file_name));
- *err_msg = psprintf("parameter \"%s\" must be set for authentication method \"%s\"",
- "oauth_validator_libraries", "oauth");
- return false;
- }
-
/* SplitDirectoriesString needs a modifiable copy */
rawstring = pstrdup(oauth_validator_libraries_string);
@@ -891,6 +878,24 @@ check_oauth_validator(HbaLine *hbaline, int elevel, char **err_msg)
goto done;
}
+ /*
+ * An empty or all-whitespace setting is accepted by
+ * SplitDirectoriesString(), which returns an empty list for it, so the
+ * parse result has to be checked rather than the raw string.
+ */
+ if (elemlist == NIL)
+ {
+ ereport(elevel,
+ errcode(ERRCODE_CONFIG_FILE_ERROR),
+ errmsg("parameter \"%s\" must be set for authentication method \"%s\"",
+ "oauth_validator_libraries", "oauth"),
+ errcontext("line %d of configuration file \"%s\"",
+ line_num, file_name));
+ *err_msg = psprintf("parameter \"%s\" must be set for authentication method \"%s\"",
+ "oauth_validator_libraries", "oauth");
+ goto done;
+ }
+
if (!hbaline->oauth_validator)
{
if (elemlist->length == 1)
diff --git a/src/test/modules/oauth_validator/t/001_server.pl b/src/test/modules/oauth_validator/t/001_server.pl
index 8941a35..86307be 100644
--- a/src/test/modules/oauth_validator/t/001_server.pl
+++ b/src/test/modules/oauth_validator/t/001_server.pl
@@ -120,6 +120,22 @@ local all testparam oauth issuer="$issuer/param" scope="openid postgres"
});
$node->reload;
+$log_start =
+ $node->wait_for_log(qr/reloading configuration files/, $log_start);
+
+# An all-whitespace library list parses as an empty list. Reject it without
+# crashing the postmaster during HBA reload.
+$node->append_conf('postgresql.conf',
+ "oauth_validator_libraries = ' '\n");
+$node->reload;
+$log_start = $node->wait_for_log(
+ qr/parameter "oauth_validator_libraries" must be set/, $log_start);
+is($bgconn->query_safe('SELECT 1'), '1',
+ 'postmaster survives an empty OAuth validator list on reload');
+
+$node->append_conf('postgresql.conf',
+ "oauth_validator_libraries = 'validator'\n");
+$node->reload;
$log_start =
$node->wait_for_log(qr/reloading configuration files/, $log_start);
--
2.39.5 (Apple Git-154)
^ permalink raw reply [nested|flat] 12+ messages in thread
* Re: Postmaster crashes on SIGHUP when oauth_validator_libraries holds only whitespace
@ 2026-09-14 11:28 Daniel Gustafsson <daniel@yesql.se>
parent: Grigorev Jurij <ju.grigorev@ftdata.ru>
0 siblings, 1 reply; 12+ messages in thread
From: Daniel Gustafsson @ 2026-09-14 11:28 UTC (permalink / raw)
To: Grigorev Jurij <ju.grigorev@ftdata.ru>; +Cc: pgsql-bugs@lists.postgresql.org <pgsql-bugs@lists.postgresql.org>; jacob.champion@enterprisedb.com <jacob.champion@enterprisedb.com>
> On 14 Sep 2026, at 13:22, Grigorev Jurij <ju.grigorev@ftdata.ru> wrote:
> Could someone take a look at the attached patch and let me know
> if this is the right fix?
Thanks for the report, that does indeed seem like the right fix. Added to my
list of patches to review and apply before the next minors
--
Daniel Gustafsson
^ permalink raw reply [nested|flat] 12+ messages in thread
* Re: Postmaster crashes on SIGHUP when oauth_validator_libraries holds only whitespace
@ 2026-09-14 21:28 Jacob Champion <jacob.champion@enterprisedb.com>
parent: Daniel Gustafsson <daniel@yesql.se>
0 siblings, 1 reply; 12+ messages in thread
From: Jacob Champion @ 2026-09-14 21:28 UTC (permalink / raw)
To: Daniel Gustafsson <daniel@yesql.se>; Grigorev Jurij <ju.grigorev@ftdata.ru>; +Cc: pgsql-bugs@lists.postgresql.org <pgsql-bugs@lists.postgresql.org>
On Mon, Sep 14, 2026 at 4:28 AM Daniel Gustafsson <daniel@yesql.se> wrote:
> Thanks for the report, that does indeed seem like the right fix.
Yes, thanks! Couple thoughts on specific pieces:
> + /*
> + * An empty or all-whitespace setting is accepted by
> + * SplitDirectoriesString(), which returns an empty list for it, so the
> + * parse result has to be checked rather than the raw string.
> + */
Sometimes recording a historical bug in the comments can help prevent
future mistakes... but I don't think this is one of those cases,
especially since the new test prevents accidental regression.
> +is($bgconn->query_safe('SELECT 1'), '1',
> + 'postmaster survives an empty OAuth validator list on reload');
query_safe() doesn't return on failure, so IMO we shouldn't wrap it in is().
--
This seems like a good time to mention that I have a checklist item to
fix the following (shouldn't block this patch):
> if (!SplitDirectoriesString(rawstring, ',', &elemlist))
> ...
> if (strcmp(allowed, hbaline->oauth_validator) == 0)
SplitDirectoriesString() canonicalizes its outputs, which we then
compare against the uncanonicalized hbaline->oauth_validator. That
could lead to annoying false negatives in more complicated setups.
Thanks,
--Jacob
^ permalink raw reply [nested|flat] 12+ messages in thread
* Re: Postmaster crashes on SIGHUP when oauth_validator_libraries holds only whitespace
@ 2026-09-14 21:34 Daniel Gustafsson <daniel@yesql.se>
parent: Jacob Champion <jacob.champion@enterprisedb.com>
0 siblings, 1 reply; 12+ messages in thread
From: Daniel Gustafsson @ 2026-09-14 21:34 UTC (permalink / raw)
To: Jacob Champion <jacob.champion@enterprisedb.com>; +Cc: Grigorev Jurij <ju.grigorev@ftdata.ru>; pgsql-bugs@lists.postgresql.org <pgsql-bugs@lists.postgresql.org>
> On 14 Sep 2026, at 23:28, Jacob Champion <jacob.champion@enterprisedb.com> wrote:
>> + /*
>> + * An empty or all-whitespace setting is accepted by
>> + * SplitDirectoriesString(), which returns an empty list for it, so the
>> + * parse result has to be checked rather than the raw string.
>> + */
>
> Sometimes recording a historical bug in the comments can help prevent
> future mistakes... but I don't think this is one of those cases,
> especially since the new test prevents accidental regression.
+1
> This seems like a good time to mention that I have a checklist item to
> fix the following (shouldn't block this patch):
>
>> if (!SplitDirectoriesString(rawstring, ',', &elemlist))
>> ...
>> if (strcmp(allowed, hbaline->oauth_validator) == 0)
>
> SplitDirectoriesString() canonicalizes its outputs, which we then
> compare against the uncanonicalized hbaline->oauth_validator. That
> could lead to annoying false negatives in more complicated setups.
Right, this patch wont move the needle in the wrong direction for future fixes
AFAICT.
--
Daniel Gustafsson
^ permalink raw reply [nested|flat] 12+ messages in thread
* Re: Postmaster crashes on SIGHUP when oauth_validator_libraries holds only whitespace
@ 2026-09-15 02:55 Grigorev Jurij <ju.grigorev@ftdata.ru>
parent: Daniel Gustafsson <daniel@yesql.se>
0 siblings, 1 reply; 12+ messages in thread
From: Grigorev Jurij @ 2026-09-15 02:55 UTC (permalink / raw)
To: Daniel Gustafsson <daniel@yesql.se>; Jacob Champion <jacob.champion@enterprisedb.com>; +Cc: pgsql-bugs@lists.postgresql.org <pgsql-bugs@lists.postgresql.org>
Hi Jacob, Daniel,
Thanks for the review! In the attached v2 I removed the explanatory
comment and call query_safe() directly. The functional change and the
reload test are otherwise unchanged.
Also left the validator-name canonicalization issue out of this patch.
Checked, the patch builds cleanly against master with --enable-cassert,
and PG_TEST_EXTRA=oauth make check in src/test/modules/oauth_validator
passes all 190 tests.
Regards,
Yuriy
________________________________________
От: Daniel Gustafsson <daniel@yesql.se>
Отправлено: 15 сентября 2026 г. 4:34:08
Кому: Jacob Champion
Копия: Григорьев Юрий; pgsql-bugs@lists.postgresql.org
Тема: Re: Postmaster crashes on SIGHUP when oauth_validator_libraries holds only whitespace
> On 14 Sep 2026, at 23:28, Jacob Champion <jacob.champion@enterprisedb.com> wrote:
>> + /*
>> + * An empty or all-whitespace setting is accepted by
>> + * SplitDirectoriesString(), which returns an empty list for it, so the
>> + * parse result has to be checked rather than the raw string.
>> + */
>
> Sometimes recording a historical bug in the comments can help prevent
> future mistakes... but I don't think this is one of those cases,
> especially since the new test prevents accidental regression.
+1
> This seems like a good time to mention that I have a checklist item to
> fix the following (shouldn't block this patch):
>
>> if (!SplitDirectoriesString(rawstring, ',', &elemlist))
>> ...
>> if (strcmp(allowed, hbaline->oauth_validator) == 0)
>
> SplitDirectoriesString() canonicalizes its outputs, which we then
> compare against the uncanonicalized hbaline->oauth_validator. That
> could lead to annoying false negatives in more complicated setups.
Right, this patch wont move the needle in the wrong direction for future fixes
AFAICT.
--
Daniel Gustafsson
Attachments:
[application/octet-stream] v2-0001-Fix-postmaster-crash-on-whitespace-only-oauth_valida.patch (3.6K, ../../c7e77341b94e406bb77e8cd3367cae59@localhost.localdomain/2-v2-0001-Fix-postmaster-crash-on-whitespace-only-oauth_valida.patch)
download | inline diff:
From c4144dc1a37ea933a2985b9340b79b9232122d8a Mon Sep 17 00:00:00 2001
From: Yuriy Grigoryev <ju.grigorev@ftdata.ru>
Date: Tue, 15 Sep 2026 09:34:36 +0700
Subject: [PATCH v2] Fix postmaster crash on whitespace-only
oauth_validator_libraries
check_oauth_validator() checks the raw GUC string for an empty validator
list. That does not cover a value containing only whitespace.
SplitDirectoriesString() accepts such input and returns an empty list, so
the code dereferences NIL when an OAuth HBA line has no validator option.
This can crash the postmaster while processing SIGHUP.
Check the parsed list instead and add a TAP test that reloads an invalid
whitespace-only setting and verifies that the server remains available.
---
src/backend/libpq/auth-oauth.c | 28 +++++++++----------
.../modules/oauth_validator/t/001_server.pl | 16 +++++++++++
2 files changed, 30 insertions(+), 14 deletions(-)
diff --git a/src/backend/libpq/auth-oauth.c b/src/backend/libpq/auth-oauth.c
index b769931ca4f..90d223eed00 100644
--- a/src/backend/libpq/auth-oauth.c
+++ b/src/backend/libpq/auth-oauth.c
@@ -863,19 +863,6 @@ check_oauth_validator(HbaLine *hbaline, int elevel, char **err_msg)
*err_msg = NULL;
- if (oauth_validator_libraries_string[0] == '\0')
- {
- ereport(elevel,
- errcode(ERRCODE_CONFIG_FILE_ERROR),
- errmsg("parameter \"%s\" must be set for authentication method \"%s\"",
- "oauth_validator_libraries", "oauth"),
- errcontext("line %d of configuration file \"%s\"",
- line_num, file_name));
- *err_msg = psprintf("parameter \"%s\" must be set for authentication method \"%s\"",
- "oauth_validator_libraries", "oauth");
- return false;
- }
-
/* SplitDirectoriesString needs a modifiable copy */
rawstring = pstrdup(oauth_validator_libraries_string);
@@ -891,9 +878,22 @@ check_oauth_validator(HbaLine *hbaline, int elevel, char **err_msg)
goto done;
}
+ if (elemlist == NIL)
+ {
+ ereport(elevel,
+ errcode(ERRCODE_CONFIG_FILE_ERROR),
+ errmsg("parameter \"%s\" must be set for authentication method \"%s\"",
+ "oauth_validator_libraries", "oauth"),
+ errcontext("line %d of configuration file \"%s\"",
+ line_num, file_name));
+ *err_msg = psprintf("parameter \"%s\" must be set for authentication method \"%s\"",
+ "oauth_validator_libraries", "oauth");
+ goto done;
+ }
+
if (!hbaline->oauth_validator)
{
- if (elemlist->length == 1)
+ if (list_length(elemlist) == 1)
{
hbaline->oauth_validator = pstrdup(linitial(elemlist));
goto done;
diff --git a/src/test/modules/oauth_validator/t/001_server.pl b/src/test/modules/oauth_validator/t/001_server.pl
index 8941a355423..65f73cbdbb2 100644
--- a/src/test/modules/oauth_validator/t/001_server.pl
+++ b/src/test/modules/oauth_validator/t/001_server.pl
@@ -123,6 +123,22 @@ $node->reload;
$log_start =
$node->wait_for_log(qr/reloading configuration files/, $log_start);
+# An all-whitespace library list parses as an empty list. Reject it without
+# crashing the postmaster during HBA reload.
+$node->append_conf('postgresql.conf',
+ "oauth_validator_libraries = ' '\n");
+$node->reload;
+$log_start = $node->wait_for_log(
+ qr/parameter "oauth_validator_libraries" must be set for authentication/,
+ $log_start);
+$bgconn->query_safe('SELECT 1');
+
+$node->append_conf('postgresql.conf',
+ "oauth_validator_libraries = 'validator'\n");
+$node->reload;
+$log_start = $node->wait_for_log(qr/reloading configuration files/,
+ $log_start);
+
# Check pg_hba_file_rules() support.
my $contents = $bgconn->query_safe(
qq(SELECT rule_number, auth_method, options
--
2.39.5 (Apple Git-154)
^ permalink raw reply [nested|flat] 12+ messages in thread
* Re: Postmaster crashes on SIGHUP when oauth_validator_libraries holds only whitespace
@ 2026-09-15 21:42 Daniel Gustafsson <daniel@yesql.se>
parent: Grigorev Jurij <ju.grigorev@ftdata.ru>
0 siblings, 2 replies; 12+ messages in thread
From: Daniel Gustafsson @ 2026-09-15 21:42 UTC (permalink / raw)
To: Grigorev Jurij <ju.grigorev@ftdata.ru>; +Cc: Jacob Champion <jacob.champion@enterprisedb.com>; pgsql-bugs@lists.postgresql.org <pgsql-bugs@lists.postgresql.org>
- if (oauth_validator_libraries_string[0] == '\0')
- {
- ereport(elevel,
- errcode(ERRCODE_CONFIG_FILE_ERROR),
- errmsg("parameter \"%s\" must be set for authentication method \"%s\"",
- "oauth_validator_libraries", "oauth"),
- errcontext("line %d of configuration file \"%s\"",
- line_num, file_name));
- *err_msg = psprintf("parameter \"%s\" must be set for authentication method \"%s\"",
- "oauth_validator_libraries", "oauth");
- return false;
- }
-
/* SplitDirectoriesString needs a modifiable copy */
rawstring = pstrdup(oauth_validator_libraries_string);
pstrdup calls strlen which segfault on NULL. oauth_validator_libraries_string
has a default value of "" so it cannot be set to NULL by user action, but the
global variable backing the GUC is initialized as NULL so I wonder if it's
worth adding defensive programming like the below, or perhaps an Assert?
/* SplitDirectoriesString needs a modifiable copy */
- rawstring = pstrdup(oauth_validator_libraries_string);
+ rawstring = pstrdup(oauth_validator_libraries_string ?
+ oauth_validator_libraries_string : "");
--
Daniel Gustafsson
^ permalink raw reply [nested|flat] 12+ messages in thread
* Re: Postmaster crashes on SIGHUP when oauth_validator_libraries holds only whitespace
@ 2026-09-16 08:27 Grigorev Jurij <ju.grigorev@ftdata.ru>
parent: Daniel Gustafsson <daniel@yesql.se>
1 sibling, 0 replies; 12+ messages in thread
From: Grigorev Jurij @ 2026-09-16 08:27 UTC (permalink / raw)
To: Daniel Gustafsson <daniel@yesql.se>; +Cc: Jacob Champion <jacob.champion@enterprisedb.com>; pgsql-bugs@lists.postgresql.org <pgsql-bugs@lists.postgresql.org>
Hi Daniel,
You're right. I would rather not treat NULL as an empty setting, since
that would turn an internal programming error into the user-facing
"must be set" error. Attached v3 adds:
Assert(oauth_validator_libraries_string != NULL);
before the pstrdup().
I also moved the whitespace-only TAP case to after the
pg_hba_file_rules() check. Its previous position made the test racy:
wait_for_log() synchronized with the postmaster reload, but not
necessarily with the process-local GUC state of the existing bgconn.
As a result, pg_hba_file_rules() could run in a backend that still had
the whitespace value loaded and return unexpected empty fields.
Moving the case after that assertion avoids making the
pg_hba_file_rules() result depend on the timing of SIGHUP processing in
bgconn. After restoring the setting, the test runs SHOW
oauth_validator_libraries through bgconn, ensuring that the backend has
processed the second SIGHUP before the later tests continue.
Thanks,
Yuriy=
Attachments:
[application/octet-stream] v3-0001-Fix-postmaster-crash-on-whitespace-only-oauth_valida.patch (4.1K, ../../0bc0874d0b774514a61d8d7b30825e0f@localhost.localdomain/2-v3-0001-Fix-postmaster-crash-on-whitespace-only-oauth_valida.patch)
download | inline diff:
From a52d00740dd22db1203dc7377a75fbead9133981 Mon Sep 17 00:00:00 2001
From: Yuriy Grigoryev <ju.grigorev@ftdata.ru>
Date: Wed, 16 Sep 2026 14:56:16 +0700
Subject: [PATCH v3] Fix postmaster crash on whitespace-only
oauth_validator_libraries
check_oauth_validator() checks the raw GUC string for an empty validator
list. That does not cover a value containing only whitespace.
SplitDirectoriesString() accepts such input and returns an empty list, so
the code dereferences NIL when an OAuth HBA line has no validator option.
This can crash the postmaster while processing SIGHUP.
Check the parsed list instead. Assert that the GUC string is non-NULL
before pstrdup(); users cannot set it to NULL, but the C variable is
initialized that way. Add a TAP test that reloads an invalid
whitespace-only setting after pg_hba_file_rules() and waits until the
existing backend sees the restored GUC.
---
src/backend/libpq/auth-oauth.c | 29 ++++++++++---------
.../modules/oauth_validator/t/001_server.pl | 20 +++++++++++++
2 files changed, 35 insertions(+), 14 deletions(-)
diff --git a/src/backend/libpq/auth-oauth.c b/src/backend/libpq/auth-oauth.c
index b769931ca4f..c01e8ae3524 100644
--- a/src/backend/libpq/auth-oauth.c
+++ b/src/backend/libpq/auth-oauth.c
@@ -863,20 +863,8 @@ check_oauth_validator(HbaLine *hbaline, int elevel, char **err_msg)
*err_msg = NULL;
- if (oauth_validator_libraries_string[0] == '\0')
- {
- ereport(elevel,
- errcode(ERRCODE_CONFIG_FILE_ERROR),
- errmsg("parameter \"%s\" must be set for authentication method \"%s\"",
- "oauth_validator_libraries", "oauth"),
- errcontext("line %d of configuration file \"%s\"",
- line_num, file_name));
- *err_msg = psprintf("parameter \"%s\" must be set for authentication method \"%s\"",
- "oauth_validator_libraries", "oauth");
- return false;
- }
-
/* SplitDirectoriesString needs a modifiable copy */
+ Assert(oauth_validator_libraries_string != NULL);
rawstring = pstrdup(oauth_validator_libraries_string);
if (!SplitDirectoriesString(rawstring, ',', &elemlist))
@@ -891,9 +879,22 @@ check_oauth_validator(HbaLine *hbaline, int elevel, char **err_msg)
goto done;
}
+ if (elemlist == NIL)
+ {
+ ereport(elevel,
+ errcode(ERRCODE_CONFIG_FILE_ERROR),
+ errmsg("parameter \"%s\" must be set for authentication method \"%s\"",
+ "oauth_validator_libraries", "oauth"),
+ errcontext("line %d of configuration file \"%s\"",
+ line_num, file_name));
+ *err_msg = psprintf("parameter \"%s\" must be set for authentication method \"%s\"",
+ "oauth_validator_libraries", "oauth");
+ goto done;
+ }
+
if (!hbaline->oauth_validator)
{
- if (elemlist->length == 1)
+ if (list_length(elemlist) == 1)
{
hbaline->oauth_validator = pstrdup(linitial(elemlist));
goto done;
diff --git a/src/test/modules/oauth_validator/t/001_server.pl b/src/test/modules/oauth_validator/t/001_server.pl
index 8941a355423..4ceb775fe20 100644
--- a/src/test/modules/oauth_validator/t/001_server.pl
+++ b/src/test/modules/oauth_validator/t/001_server.pl
@@ -134,6 +134,26 @@ is( $contents,
3|oauth|\{issuer=$issuer/param,"scope=openid postgres",validator=validator\}},
"pg_hba_file_rules recreates OAuth HBA settings");
+# An all-whitespace library list parses as an empty list. Reject it without
+# crashing the postmaster during HBA reload.
+$node->append_conf('postgresql.conf',
+ "oauth_validator_libraries = ' '\n");
+$node->reload;
+$log_start = $node->wait_for_log(
+ qr/parameter "oauth_validator_libraries" must be set for authentication/,
+ $log_start);
+$bgconn->query_safe('SELECT 1');
+
+$node->append_conf('postgresql.conf',
+ "oauth_validator_libraries = 'validator'\n");
+$node->reload;
+$log_start = $node->wait_for_log(
+ qr/parameter "oauth_validator_libraries" changed to "validator"/,
+ $log_start);
+is( $bgconn->query_safe('SHOW oauth_validator_libraries'),
+ 'validator',
+ 'oauth_validator_libraries restored');
+
{
# Make sure PGOAUTHDEBUG=UNSAFE doesn't disable certificate verification.
local $ENV{PGOAUTHDEBUG} = "UNSAFE";
--
2.54.0 (Apple Git-157)
^ permalink raw reply [nested|flat] 12+ messages in thread
* Re: Postmaster crashes on SIGHUP when oauth_validator_libraries holds only whitespace
@ 2026-09-16 22:16 Jacob Champion <jacob.champion@enterprisedb.com>
parent: Daniel Gustafsson <daniel@yesql.se>
1 sibling, 1 reply; 12+ messages in thread
From: Jacob Champion @ 2026-09-16 22:16 UTC (permalink / raw)
To: Daniel Gustafsson <daniel@yesql.se>; +Cc: Grigorev Jurij <ju.grigorev@ftdata.ru>; pgsql-bugs@lists.postgresql.org <pgsql-bugs@lists.postgresql.org>
On Tue, Sep 15, 2026 at 2:43 PM Daniel Gustafsson <daniel@yesql.se> wrote:
> pstrdup calls strlen which segfault on NULL. oauth_validator_libraries_string
> has a default value of "" so it cannot be set to NULL by user action, but the
> global variable backing the GUC is initialized as NULL so I wonder if it's
> worth adding defensive programming like the below, or perhaps an Assert?
It looks like there are no GUC_LIST_INPUT params with a NULL
.boot_val. I don't know if that's by design, but it's probably for the
best given the decision in ff4597acd4c.
There may be a bit of a tug-of-war going on between committers who
prefer the explicit Assert() crash vs committers who are fine with the
SEGV. (I prefer the assertion when it better documents intent, but at
time of authorship I'm not sure I would have wanted to argue about it.
:D)
Thanks,
--Jacob
^ permalink raw reply [nested|flat] 12+ messages in thread
* Re: Postmaster crashes on SIGHUP when oauth_validator_libraries holds only whitespace
@ 2026-09-16 22:26 Daniel Gustafsson <daniel@yesql.se>
parent: Jacob Champion <jacob.champion@enterprisedb.com>
0 siblings, 1 reply; 12+ messages in thread
From: Daniel Gustafsson @ 2026-09-16 22:26 UTC (permalink / raw)
To: Jacob Champion <jacob.champion@enterprisedb.com>; +Cc: Grigorev Jurij <ju.grigorev@ftdata.ru>; pgsql-bugs@lists.postgresql.org <pgsql-bugs@lists.postgresql.org>
> On 17 Sep 2026, at 00:16, Jacob Champion <jacob.champion@enterprisedb.com> wrote:
> There may be a bit of a tug-of-war going on between committers who
> prefer the explicit Assert() crash vs committers who are fine with the
> SEGV. (I prefer the assertion when it better documents intent, but at
> time of authorship I'm not sure I would have wanted to argue about it.
> :D)
I agree with the documenting aspect of the Assert.
--
Daniel Gustafsson
^ permalink raw reply [nested|flat] 12+ messages in thread
* Re: Postmaster crashes on SIGHUP when oauth_validator_libraries holds only whitespace
@ 2026-09-17 22:24 Daniel Gustafsson <daniel@yesql.se>
parent: Daniel Gustafsson <daniel@yesql.se>
0 siblings, 1 reply; 12+ messages in thread
From: Daniel Gustafsson @ 2026-09-17 22:24 UTC (permalink / raw)
To: Jacob Champion <jacob.champion@enterprisedb.com>; +Cc: Jurij Grigorev <ju.grigorev@ftdata.ru>; pgsql-bugs@lists.postgresql.org
Pushed with tiny bits of tweaking and with a backpatch down to 18. Thanks!
./daniel
^ permalink raw reply [nested|flat] 12+ messages in thread
* Re: Postmaster crashes on SIGHUP when oauth_validator_libraries holds only whitespace
@ 2026-09-30 18:00 Alexander Lakhin <exclusion@gmail.com>
parent: Daniel Gustafsson <daniel@yesql.se>
0 siblings, 1 reply; 12+ messages in thread
From: Alexander Lakhin @ 2026-09-30 18:00 UTC (permalink / raw)
To: Daniel Gustafsson <daniel@yesql.se>; +Cc: Jacob Champion <jacob.champion@enterprisedb.com>; Jurij Grigorev <ju.grigorev@ftdata.ru>; pgsql-bugs@lists.postgresql.org
Hello Daniel,
18.09.2026 01:24, Daniel Gustafsson wrote:
> Pushed with tiny bits of tweaking and with a backpatch down to 18. Thanks!
>
> ./daniel
The test addition coined with bca67e5a3 is not very robust,
unfortunately. Buildfarm animal serinus found a way to break it [1]
as below:
# Failed test 'oauth_validator_libraries restored'
# at /home/bf/bf-build/serinus/HEAD/pgsql/src/test/modules/oauth_validator/t/001_server.pl line 152.
# got: ' '
# expected: 'validator'
# Looks like you failed 1 test of 171.
I've managed to reproduce this locally with a sleep:
--- a/src/backend/utils/misc/guc.c
+++ b/src/backend/utils/misc/guc.c
@@ -569,6 +569,7 @@ ProcessConfigFileInternal(GucContext context, bool applySettings, int elevel)
item->name, item->value)));
}
item->applied = true;
+pg_usleep(100000);
}
else if (scres == 0)
{
It makes oauth_validator/001_server fail deterministically for me.
Could you take a look, please?
[1] https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=serinus&dt=2026-09-29%2013%3A57%3A41
Best regards,
Alexander
^ permalink raw reply [nested|flat] 12+ messages in thread
* Re: Postmaster crashes on SIGHUP when oauth_validator_libraries holds only whitespace
@ 2026-09-30 18:20 Jacob Champion <jacob.champion@enterprisedb.com>
parent: Alexander Lakhin <exclusion@gmail.com>
0 siblings, 0 replies; 12+ messages in thread
From: Jacob Champion @ 2026-09-30 18:20 UTC (permalink / raw)
To: Alexander Lakhin <exclusion@gmail.com>; Daniel Gustafsson <daniel@yesql.se>; +Cc: Jurij Grigorev <ju.grigorev@ftdata.ru>; pgsql-bugs@lists.postgresql.org
On Wed, Sep 30, 2026 at 11:00 AM Alexander Lakhin <exclusion@gmail.com> wrote:
> The test addition coined with bca67e5a3 is not very robust,
> unfortunately. Buildfarm animal serinus found a way to break it [1]
> as below:
Bleh. These SIGHUP/log file races are really annoying, and I'm going
to need to trawl the list to see if anyone's proposed a more general
solution. (I keep thinking about assigning an xid as a generation
number to an in-memory configuration...)
For now, is the following test giving us much, or can we remove it?
We've already done the wait_for_log(). If that "sequence point" turns
out to be inadequate for some other reason, we'd have to fix it
anyway.
> is($bgconn->query_safe('SHOW oauth_validator_libraries'),
> 'validator', 'oauth_validator_libraries restored');
Thanks,
--Jacob
^ permalink raw reply [nested|flat] 12+ messages in thread
end of thread, other threads:[~2026-09-30 18:20 UTC | newest]
Thread overview: 12+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2026-09-14 11:22 Postmaster crashes on SIGHUP when oauth_validator_libraries holds only whitespace Grigorev Jurij <ju.grigorev@ftdata.ru>
2026-09-14 11:28 ` Daniel Gustafsson <daniel@yesql.se>
2026-09-14 21:28 ` Jacob Champion <jacob.champion@enterprisedb.com>
2026-09-14 21:34 ` Daniel Gustafsson <daniel@yesql.se>
2026-09-15 02:55 ` Grigorev Jurij <ju.grigorev@ftdata.ru>
2026-09-15 21:42 ` Daniel Gustafsson <daniel@yesql.se>
2026-09-16 08:27 ` Grigorev Jurij <ju.grigorev@ftdata.ru>
2026-09-16 22:16 ` Jacob Champion <jacob.champion@enterprisedb.com>
2026-09-16 22:26 ` Daniel Gustafsson <daniel@yesql.se>
2026-09-17 22:24 ` Daniel Gustafsson <daniel@yesql.se>
2026-09-30 18:00 ` Alexander Lakhin <exclusion@gmail.com>
2026-09-30 18:20 ` Jacob Champion <jacob.champion@enterprisedb.com>
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox