From 9720cda1caaede3e3f8b52c20a0e0406e55381a7 Mon Sep 17 00:00:00 2001 From: Bharath Rupireddy Date: Wed, 30 Sep 2026 02:27:58 +0000 Subject: [PATCH v2] Fix DROP DATABASE FORCE to terminate parallel worker connections. An autovacuum worker, and a background worker that connected with no user name, run as the bootstrap superuser but advertise no role in shared memory, while the parallel workers they launch advertise the bootstrap superuser they were authenticated with. DROP DATABASE FORCE accepts a process advertising no role on purpose, so that it can terminate such a leader, but it refused its parallel workers with "permission denied to terminate process" unless the caller was a superuser. As a result, a non-superuser that owns the database and has privileges of pg_signal_backend could not drop it while a parallel autovacuum was working on it, although it could terminate the autovacuum worker itself. Fix this by checking a parallel worker with the role advertised by its lock group leader. This matches the DROP DATABASE documentation, which says that FORCE terminates background worker connections. Add a test to the test_autovacuum module. It holds the parallel workers of an autovacuum at a new injection point, where a worker is attached to its leader, and runs DROP DATABASE FORCE as the non-superuser owner of the database. Backpatch to all supported versions, since a background worker of an extension can run a parallel query on any of them. Found by Bharath using AI assisted review with Claude. Reported-by: Bharath Rupireddy Author: Bharath Rupireddy Reviewed-by: Masahiko Sawada Discussion: https://postgr.es/m/CALj2ACWK6PkOk5MTfibtJyTYpb%3DAq_8%3DYaexnPKhOj11bOCROQ%40mail.gmail.com Backpatch-through: 14 --- src/backend/commands/vacuumparallel.c | 4 ++ src/backend/storage/ipc/procarray.c | 19 +++++- .../t/001_parallel_autovacuum.pl | 64 +++++++++++++++++++ 3 files changed, 85 insertions(+), 2 deletions(-) diff --git a/src/backend/commands/vacuumparallel.c b/src/backend/commands/vacuumparallel.c index d4572861000..801017ba206 100644 --- a/src/backend/commands/vacuumparallel.c +++ b/src/backend/commands/vacuumparallel.c @@ -46,6 +46,7 @@ #include "storage/bufmgr.h" #include "storage/proc.h" #include "tcop/tcopprot.h" +#include "utils/injection_point.h" #include "utils/lsyscache.h" #include "utils/rel.h" @@ -1320,6 +1321,9 @@ parallel_vacuum_main(dsm_segment *seg, shm_toc *toc) /* Prepare to track buffer usage during parallel execution */ InstrStartParallelQuery(); + /* Used by tests to hold a worker while it is attached to its leader */ + INJECTION_POINT("parallel-vacuum-worker-start", NULL); + /* Process indexes to perform vacuum/cleanup */ parallel_vacuum_process_safe_indexes(&pvs); diff --git a/src/backend/storage/ipc/procarray.c b/src/backend/storage/ipc/procarray.c index b7e03134ed8..aba5e83334a 100644 --- a/src/backend/storage/ipc/procarray.c +++ b/src/backend/storage/ipc/procarray.c @@ -3900,14 +3900,29 @@ TerminateOtherDBBackends(Oid databaseId) if (proc != NULL) { - if (superuser_arg(proc->roleId) && !superuser()) + PGPROC *leader = proc->lockGroupLeader; + Oid roleId = proc->roleId; + + /* + * An autovacuum worker and a background worker with no user + * name advertise no role, while the parallel workers they + * launch advertise the bootstrap superuser they run as. The + * checks below accept such a leader but would refuse its + * workers, so check a parallel worker with the role of its + * leader. + */ + if (leader != NULL && leader != proc && + leader->databaseId == databaseId) + roleId = leader->roleId; + + if (superuser_arg(roleId) && !superuser()) ereport(ERROR, (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE), errmsg("permission denied to terminate process"), errdetail("Only roles with the %s attribute may terminate processes of roles with the %s attribute.", "SUPERUSER", "SUPERUSER"))); - if (!has_privs_of_role(GetUserId(), proc->roleId) && + if (!has_privs_of_role(GetUserId(), roleId) && !has_privs_of_role(GetUserId(), ROLE_PG_SIGNAL_BACKEND)) ereport(ERROR, (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE), diff --git a/src/test/modules/test_autovacuum/t/001_parallel_autovacuum.pl b/src/test/modules/test_autovacuum/t/001_parallel_autovacuum.pl index 33c86bbdc94..09f70ce2dfc 100644 --- a/src/test/modules/test_autovacuum/t/001_parallel_autovacuum.pl +++ b/src/test/modules/test_autovacuum/t/001_parallel_autovacuum.pl @@ -253,5 +253,69 @@ $node->safe_psql('postgres', $node->safe_psql('postgres', "SELECT injection_points_detach('autovacuum-worker-cost-balanced')"); +# Test 4: +# DROP DATABASE FORCE can terminate an autovacuum worker, so it must also be +# able to terminate the parallel workers that worker launched, since both run +# as the bootstrap superuser. + +# Leave the parallel worker slot to the autovacuum below. +$node->safe_psql('postgres', + 'ALTER TABLE test_autovac SET (autovacuum_enabled = false)'); +$node->safe_psql('regress_db2', + 'ALTER TABLE filler SET (autovacuum_enabled = false)'); + +# Hand the second database to a non-superuser that may terminate other +# backends. +$node->safe_psql( + 'postgres', qq{ + CREATE ROLE regress_dbowner LOGIN; + GRANT pg_signal_backend TO regress_dbowner; + ALTER DATABASE regress_db2 OWNER TO regress_dbowner; +}); + +# Hold the parallel worker while it is attached to its leader. +$node->safe_psql('postgres', + "SELECT injection_points_attach('parallel-vacuum-worker-start', 'wait')"); + +# A table with two indexes, so that its autovacuum vacuums one of them with a +# single parallel worker. +$node->safe_psql( + 'regress_db2', qq{ + CREATE TABLE dropdb_force (a int, b int) + WITH (autovacuum_parallel_workers = 1, + autovacuum_vacuum_threshold = 1, + autovacuum_vacuum_scale_factor = 0); + INSERT INTO dropdb_force SELECT g, g FROM generate_series(1, 100) g; + CREATE INDEX ON dropdb_force (a); + CREATE INDEX ON dropdb_force (b); + DELETE FROM dropdb_force; +}); + +# Wait until the parallel worker is held at the injection point. +$node->wait_for_event('parallel worker', 'parallel-vacuum-worker-start'); + +$log_offset = -s $node->logfile; + +my ($psql_out, $psql_err) = ('', ''); +$node->psql( + 'postgres', + 'DROP DATABASE regress_db2 WITH (FORCE)', + connstr => $node->connstr('postgres') . ' user=regress_dbowner', + stdout => \$psql_out, + stderr => \$psql_err); + +is($psql_err, '', 'no error from DROP DATABASE FORCE'); + +# Check that the server logs a FATAL indicating that the parallel worker is +# terminated. +ok( $node->log_contains( + qr/FATAL: .*terminating background worker "parallel worker" due to administrator command/, + $log_offset), + 'DROP DATABASE FORCE terminates parallel autovacuum workers'); + +# The command terminated the held worker, so there is nothing left to wake up. +$node->safe_psql('postgres', + "SELECT injection_points_detach('parallel-vacuum-worker-start')"); + $node->stop; done_testing(); -- 2.47.3