MarkGUCPrefixReserved() doesn't check all options
Hi hackers,
Ekaterina Sokolova and I have found a bug in PG 15. Since 88103567cb
MarkGUCPrefixReserved() is supposed not only reporting GUCs that haven't
been
defined by the extension, but also removing them. However, it removes them
in
a wrong way, so that a GUC that goes after the removed GUC is never checked.
To reproduce the bug add the following to the postgresql.conf
shared_preload_libraries = 'pg_stat_statements'
pg_stat_statements.nonexisting_option_1 = on
pg_stat_statements.nonexisting_option_2 = on
pg_stat_statements.nonexisting_option_3 = on
pg_stat_statements.nonexisting_option_4 = on
and start the server. In the logfile you'll see only first and third options
reported invalid and removed.
In master MarkGUCPrefixReserved() iterates a hash table, not an array as in
PG 15. I'm not sure whether it is safe to remove an entry from this hash
table
while iterating it, but at least I can't reproduce the bug on master.
I attached a bugfix for PG 15.
Best regards,
Karina Litskevich
Postgres Professional: http://postgrespro.com/
Attachments:
v1-0001-Fix-MarkGUCPrefixReserved-to-check-all-options.patchtext/x-patch; charset=US-ASCII; name=v1-0001-Fix-MarkGUCPrefixReserved-to-check-all-options.patchDownload
From 5f0e59b225ebb408a0c971bc78e22050b296ae9a Mon Sep 17 00:00:00 2001
From: Karina Litskevich <litskevichkarina@gmail.com>
Date: Thu, 6 Jul 2023 11:00:36 +0300
Subject: [PATCH v1] Fix MarkGUCPrefixReserved() to check all options
---
src/backend/utils/misc/guc.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c
index 915f557c68..c410ba532d 100644
--- a/src/backend/utils/misc/guc.c
+++ b/src/backend/utils/misc/guc.c
@@ -9723,6 +9723,7 @@ MarkGUCPrefixReserved(const char *className)
num_guc_variables--;
memmove(&guc_variables[i], &guc_variables[i + 1],
(num_guc_variables - i) * sizeof(struct config_generic *));
+ i--;
}
}
--
2.25.1
On 06/07/2023 12:17, Karina Litskevich wrote:
Hi hackers,
Ekaterina Sokolova and I have found a bug in PG 15. Since 88103567cb
MarkGUCPrefixReserved() is supposed not only reporting GUCs that
haven't been defined by the extension, but also removing them.
However, it removes them in a wrong way, so that a GUC that goes
after the removed GUC is never checked.To reproduce the bug add the following to the postgresql.conf
shared_preload_libraries = 'pg_stat_statements'
pg_stat_statements.nonexisting_option_1 = on
pg_stat_statements.nonexisting_option_2 = on
pg_stat_statements.nonexisting_option_3 = on
pg_stat_statements.nonexisting_option_4 = onand start the server. In the logfile you'll see only first and third
options reported invalid and removed.
Good catch!
In master MarkGUCPrefixReserved() iterates a hash table, not an array
as in PG 15. I'm not sure whether it is safe to remove an entry from
this hash table while iterating it, but at least I can't reproduce
the bug on master.
Yes, it's safe to remove the current element, while scanning a hash
table with hash_seq_init/search. See comment on hash_seq_init.
I attached a bugfix for PG 15.
Applied, thanks!
--
Heikki Linnakangas
Neon (https://neon.tech)