From 4fc24f73e95ec44fbc9c0a59ee794a3878902ed6 Mon Sep 17 00:00:00 2001 From: Bharath Rupireddy Date: Wed, 30 Sep 2026 02:27:58 +0000 Subject: [PATCH v5] 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 as having no role when its lock group leader advertises none. 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 | 24 ++++++- .../t/001_parallel_autovacuum.pl | 64 +++++++++++++++++++ 3 files changed, 90 insertions(+), 2 deletions(-) diff --git a/src/backend/commands/vacuumparallel.c b/src/backend/commands/vacuumparallel.c index 4e432c50d37..297aae60df6 100644 --- a/src/backend/commands/vacuumparallel.c +++ b/src/backend/commands/vacuumparallel.c @@ -47,6 +47,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" @@ -1383,6 +1384,9 @@ parallel_vacuum_main(dsm_segment *seg, shm_toc *toc) /* Register this worker for vacuum progress reporting */ pgstat_progress_start_command(PROGRESS_COMMAND_VACUUM, shared->relid); + /* 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..f77e9540ccc 100644 --- a/src/backend/storage/ipc/procarray.c +++ b/src/backend/storage/ipc/procarray.c @@ -3900,14 +3900,34 @@ 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 such a worker with no role as well. + * + * We read the leader without the lock group partition lock. + * The leader's PGPROC is recycled only after the last member + * has exited, so if we read an unrelated backend here, the + * worker we are checking is already gone, which is the same + * as a process exiting before we signal it. + */ + if (leader != NULL && leader != proc && + !OidIsValid(leader->roleId)) + roleId = InvalidOid; + + 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 115f92a49e4..fe3282332a2 100644 --- a/src/test/modules/test_autovacuum/t/001_parallel_autovacuum.pl +++ b/src/test/modules/test_autovacuum/t/001_parallel_autovacuum.pl @@ -254,5 +254,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.37.1 (Apple Git-137.1)