database/postgres
1
0
Fork 0
mirror of https://github.com/postgres/postgres.git synced 2026-10-23 13:29:09 +03:00
Code Activity

Fix DROP DATABASE FORCE failing on parallel autovacuum workers.

Browse Source 
An autovacuum worker advertises no role in its PGPROC, but its
parallel workers advertise the bootstrap superuser. So DROP DATABASE
... FORCE run by a non-superuser with pg_signal_backend failed with
"permission denied to terminate process" while a parallel autovacuum
was running in the database, although the same user can terminate the
autovacuum worker itself, and DROP DATABASE without FORCE succeeds.

Fix by checking a parallel worker against its leader's role when the
leader advertises no role. The leader is in the same database and is
checked too, so this doesn't let anyone terminate a process they
couldn't already terminate through its leader.

Parallel workers launched by a background worker connected without a
user name have had the same problem in older branches, but we haven't
received any report of it, and the next minor release is the final one
for v14, leaving no chance to fix a mistake there. So backpatch only
to v19, where parallel autovacuum made the problem easy to hit. If we
get a report, we can backpatch it further.

Found by Bharath using AI assisted review.

Reported-by: Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
Author: Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
Reviewed-by: Masahiko Sawada <sawada.mshk@gmail.com>
Reviewed-by: shihao zhong <zhong950419@gmail.com>
Discussion: https://postgr.es/m/CALj2ACWK6PkOk5MTfibtJyTYpb=Aq_8=YaexnPKhOj11bOCROQ@mail.gmail.com
Backpatch-through: 19
This commit is contained in:
Masahiko Sawada
2026-10-15 10:33:18 -07:00
parent 8f2b598b4b
commit a39e53a700
3 changed files with 67 additions and 4 deletions
Download Patch File Download Diff File Expand all files Collapse all files

4
src/backend/commands/vacuumparallel.c
View File

@@ -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"
@@ -1289,6 +1290,9 @@ parallel_vacuum_main(dsm_segment *seg, shm_toc *toc)
elog(DEBUG1, "starting parallel vacuum worker");
/* Used by tests to hold a worker while it is attached to its leader */
INJECTION_POINT("parallel-vacuum-worker-start", NULL);
shared = (PVShared *) shm_toc_lookup(toc, PARALLEL_VACUUM_KEY_SHARED, false);
/* Set debug_query_string for individual workers */

25
src/backend/storage/ipc/procarray.c
View File

@@ -3882,8 +3882,9 @@ TerminateOtherDBBackends(Oid databaseId)
* Permissions checks relax the pg_terminate_backend checks in two
* ways, both by omitting the !OidIsValid(proc->roleId) check:
*
* - Accept terminating autovacuum workers, since DROP DATABASE
* without FORCE terminates them.
* - Accept terminating autovacuum workers and the parallel workers
* launched by them, since DROP DATABASE without FORCE terminates
* them.
*
* - Accept terminating bgworkers. For bgworker authors, it's
* convenient to be able to recommend FORCE if a worker is blocking
@@ -3900,14 +3901,30 @@ 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 bgworker with no user name
* advertise no role in PGPROC and run as the bootstrap
* superuser (see InitializeSessionUserIdStandalone()
* callsites in postinit.c). The parallel workers they launch
* inherit the same superuser and advertise it in their
* PGPROC. Accept such parallel workers for terminating as
* well.
*/
if (leader != NULL && leader != proc &&
!OidIsValid(leader->roleId))
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),

42
src/test/modules/test_autovacuum/t/001_parallel_autovacuum.pl
View File

@@ -254,5 +254,47 @@ $node->safe_psql('postgres',
$node->safe_psql('postgres',
"SELECT injection_points_detach('autovacuum-worker-cost-balanced')");
# Test 4:
# Check that DROP DATABASE FORCE run by a non-superuser terminates both an
# autovacuum worker and its parallel workers.
$node->safe_psql(
'postgres', qq{
DROP TABLE test_autovac;
CREATE ROLE regress_dbowner LOGIN IN ROLE pg_signal_backend;
ALTER DATABASE regress_db2 OWNER TO regress_dbowner;
SELECT injection_points_attach('parallel-vacuum-worker-start', 'wait');
});
$node->safe_psql(
'regress_db2', qq{
CREATE TABLE tbl_fddb (a int, b int)
WITH (autovacuum_parallel_workers = 1, autovacuum_vacuum_threshold = 1);
INSERT INTO tbl_fddb SELECT g, g FROM generate_series(1, 100) g;
CREATE INDEX tbl_fddb_a ON tbl_fddb (a);
CREATE INDEX tbl_fddb_b ON tbl_fddb (b);
DELETE FROM tbl_fddb;
});
$node->wait_for_event('parallel worker', 'parallel-vacuum-worker-start');
$log_offset = -s $node->logfile;
my $psql_err = '';
$node->psql(
'postgres',
'DROP DATABASE regress_db2 WITH (FORCE)',
connstr => $node->connstr('postgres') . ' user=regress_dbowner',
stderr => \$psql_err);
is($psql_err, '', 'no error from DROP DATABASE FORCE');
ok( $node->log_contains(
qr/FATAL: .*terminating background worker "parallel worker" due to administrator command/,
$log_offset),
'DROP DATABASE FORCE terminates parallel autovacuum workers');
# No wakeup, the held worker is already gone.
$node->safe_psql('postgres',
"SELECT injection_points_detach('parallel-vacuum-worker-start')");
$node->stop;
done_testing();