Обсуждение: 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/
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/
Вложения
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 = on > > and 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)