From 2b5004e14d5802fda3f51cbeaa0a41a84c633f62 Mon Sep 17 00:00:00 2001 From: Nathan Bossart Date: Tue, 14 Oct 2025 16:22:22 -0500 Subject: [PATCH v7 3/3] Fix privilege checks for pg_prewarm() on indexes. Author: Ayush Vatsa Co-authored-by: Nathan Bossart Reviewed-by: Tom Lane Reviewed-by: Jeff Davis Discussion: https://postgr.es/m/CACX%2BKaMz2ZoOojh0nQ6QNBYx8Ak1Dkoko%3DD4FSb80BYW%2Bo8CHQ%40mail.gmail.com Backpatch-through: 13 --- contrib/pg_prewarm/pg_prewarm.c | 47 +++++++++++++++++++++++++++++-- contrib/pg_prewarm/t/001_basic.pl | 29 ++++++++++++++++++- 2 files changed, 72 insertions(+), 4 deletions(-) diff --git a/contrib/pg_prewarm/pg_prewarm.c b/contrib/pg_prewarm/pg_prewarm.c index b968933ea8b..5b519a2c854 100644 --- a/contrib/pg_prewarm/pg_prewarm.c +++ b/contrib/pg_prewarm/pg_prewarm.c @@ -16,9 +16,11 @@ #include #include "access/relation.h" +#include "catalog/index.h" #include "fmgr.h" #include "miscadmin.h" #include "storage/bufmgr.h" +#include "storage/lmgr.h" #include "storage/read_stream.h" #include "storage/smgr.h" #include "utils/acl.h" @@ -71,6 +73,8 @@ pg_prewarm(PG_FUNCTION_ARGS) char *ttype; PrewarmType ptype; AclResult aclresult; + char relkind; + Oid privOid; /* Basic sanity checking. */ if (PG_ARGISNULL(0)) @@ -106,9 +110,43 @@ pg_prewarm(PG_FUNCTION_ARGS) forkString = text_to_cstring(forkName); forkNumber = forkname_to_number(forkString); - /* Open relation and check privileges. */ + /* + * Open relation and check privileges. If the relation is an index, we + * must check the privileges on its parent table instead. + */ + relkind = get_rel_relkind(relOid); + if (relkind == RELKIND_INDEX || + relkind == RELKIND_PARTITIONED_INDEX) + { + privOid = IndexGetRelation(relOid, true); + + /* Lock table before index to avoid deadlock. */ + if (OidIsValid(privOid)) + LockRelationOid(privOid, AccessShareLock); + } + else + privOid = relOid; + rel = relation_open(relOid, AccessShareLock); - aclresult = pg_class_aclcheck(relOid, GetUserId(), ACL_SELECT); + + /* + * It's possible that the relation with OID "privOid" was dropped and the + * OID was reused before we locked it. If that happens, we could be left + * with the wrong parent table OID, in which case we must ERROR. It's + * possible that such a race would change the outcome of + * get_rel_relkind(), too, but the worst case scenario there is that we'll + * check privileges on the index instead of its parent table, which isn't + * too terrible. + */ + if (!OidIsValid(privOid) || + (privOid != relOid && + privOid != IndexGetRelation(relOid, true))) + ereport(ERROR, + (errcode(ERRCODE_UNDEFINED_TABLE), + errmsg("could not find parent table of index \"%s\"", + RelationGetRelationName(rel)))); + + aclresult = pg_class_aclcheck(privOid, GetUserId(), ACL_SELECT); if (aclresult != ACLCHECK_OK) aclcheck_error(aclresult, get_relkind_objtype(rel->rd_rel->relkind), get_rel_name(relOid)); @@ -233,8 +271,11 @@ pg_prewarm(PG_FUNCTION_ARGS) read_stream_end(stream); } - /* Close relation, release lock. */ + /* Close relation, release locks. */ relation_close(rel, AccessShareLock); + if (privOid != relOid) + UnlockRelationOid(privOid, AccessShareLock); + PG_RETURN_INT64(blocks_done); } diff --git a/contrib/pg_prewarm/t/001_basic.pl b/contrib/pg_prewarm/t/001_basic.pl index 0a8259d3678..a77ab67d29e 100644 --- a/contrib/pg_prewarm/t/001_basic.pl +++ b/contrib/pg_prewarm/t/001_basic.pl @@ -23,7 +23,9 @@ $node->start; $node->safe_psql("postgres", "CREATE EXTENSION pg_prewarm;\n" . "CREATE TABLE test(c1 int);\n" - . "INSERT INTO test SELECT generate_series(1, 100);"); + . "INSERT INTO test SELECT generate_series(1, 100);\n" + . "CREATE INDEX test_idx ON test(c1);\n" + . "CREATE ROLE test_user LOGIN;"); # test read mode my $result = @@ -42,6 +44,31 @@ ok( ( $stdout =~ qr/^[1-9][0-9]*$/ or $stderr =~ qr/prefetch is not supported by this build/), 'prefetch mode succeeded'); +# test_user should be unable to prewarm table/index without privileges +($cmdret, $stdout, $stderr) = + $node->psql( + "postgres", "SELECT pg_prewarm('test');", + extra_params => [ '--username' => 'test_user' ]); +ok($stderr =~ /permission denied for table test/, 'pg_prewarm failed as expected'); +($cmdret, $stdout, $stderr) = + $node->psql( + "postgres", "SELECT pg_prewarm('test_idx');", + extra_params => [ '--username' => 'test_user' ]); +ok($stderr =~ /permission denied for index test_idx/, 'pg_prewarm failed as expected'); + +# test_user should be able to prewarm table/index with privileges +$node->safe_psql("postgres", "GRANT SELECT ON test TO test_user;"); +$result = + $node->safe_psql( + "postgres", "SELECT pg_prewarm('test');", + extra_params => [ '--username' => 'test_user' ]); +like($result, qr/^[1-9][0-9]*$/, 'pg_prewarm succeeded as expected'); +$result = + $node->safe_psql( + "postgres", "SELECT pg_prewarm('test_idx');", + extra_params => [ '--username' => 'test_user' ]); +like($result, qr/^[1-9][0-9]*$/, 'pg_prewarm succeeded as expected'); + # test autoprewarm_dump_now() $result = $node->safe_psql("postgres", "SELECT autoprewarm_dump_now();"); like($result, qr/^[1-9][0-9]*$/, 'autoprewarm_dump_now succeeded'); -- 2.39.5 (Apple Git-154)