From 952835cbd0d42568299e204f5579df08468b3d6f Mon Sep 17 00:00:00 2001 From: Bharath Rupireddy Date: Sat, 9 Oct 2021 09:05:36 +0000 Subject: [PATCH v1] change privileges of pg_backend_memory_contexts, pg_shmem_allocations, pg_log_backend_memory_contexts In a typical production environment, the user (not necessarily a superuser) wants to analyze the memory usage via pg_backend_memory_contexts view or pg_shmem_allocations view or pg_log_backend_memory_contexts function which are accessible to only superusers. This patch allows non-superusers with a predefined role pg_read_all_stats to access them. While on this, change the way the tests use pg_log_backend_memory_contexts() Usually for functions, we don't use "SELECT-FROM-<>", we just use "SELECT-<>". --- src/backend/catalog/system_views.sql | 4 ++ src/backend/utils/adt/mcxtfuncs.c | 10 ++-- src/test/regress/expected/misc_functions.out | 2 +- src/test/regress/expected/privileges.out | 52 ++++++++++++++++++++ src/test/regress/sql/misc_functions.sql | 2 +- src/test/regress/sql/privileges.sql | 41 +++++++++++++++ 6 files changed, 106 insertions(+), 5 deletions(-) diff --git a/src/backend/catalog/system_views.sql b/src/backend/catalog/system_views.sql index 55f6e3711d..eb560955cd 100644 --- a/src/backend/catalog/system_views.sql +++ b/src/backend/catalog/system_views.sql @@ -621,13 +621,17 @@ CREATE VIEW pg_shmem_allocations AS SELECT * FROM pg_get_shmem_allocations(); REVOKE ALL ON pg_shmem_allocations FROM PUBLIC; +GRANT SELECT ON pg_shmem_allocations TO pg_read_all_stats; REVOKE EXECUTE ON FUNCTION pg_get_shmem_allocations() FROM PUBLIC; +GRANT EXECUTE ON FUNCTION pg_get_shmem_allocations() TO pg_read_all_stats; CREATE VIEW pg_backend_memory_contexts AS SELECT * FROM pg_get_backend_memory_contexts(); REVOKE ALL ON pg_backend_memory_contexts FROM PUBLIC; +GRANT SELECT ON pg_backend_memory_contexts TO pg_read_all_stats; REVOKE EXECUTE ON FUNCTION pg_get_backend_memory_contexts() FROM PUBLIC; +GRANT EXECUTE ON FUNCTION pg_get_backend_memory_contexts() TO pg_read_all_stats; -- Statistics views diff --git a/src/backend/utils/adt/mcxtfuncs.c b/src/backend/utils/adt/mcxtfuncs.c index 0d52613bc3..fdadd7398d 100644 --- a/src/backend/utils/adt/mcxtfuncs.c +++ b/src/backend/utils/adt/mcxtfuncs.c @@ -15,11 +15,13 @@ #include "postgres.h" +#include "catalog/pg_authid.h" #include "funcapi.h" #include "miscadmin.h" #include "mb/pg_wchar.h" #include "storage/proc.h" #include "storage/procarray.h" +#include "utils/acl.h" #include "utils/builtins.h" /* ---------- @@ -177,11 +179,13 @@ pg_log_backend_memory_contexts(PG_FUNCTION_ARGS) int pid = PG_GETARG_INT32(0); PGPROC *proc; - /* Only allow superusers to log memory contexts. */ - if (!superuser()) + /* + * Only superusers or members of pg_read_all_stats can log memory contexts. + */ + if (!is_member_of_role(GetUserId(), ROLE_PG_READ_ALL_STATS)) ereport(ERROR, (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE), - errmsg("must be a superuser to log memory contexts"))); + errmsg("must be superuser or a member of the pg_read_all_stats role to log memory contexts"))); proc = BackendPidGetProc(pid); diff --git a/src/test/regress/expected/misc_functions.out b/src/test/regress/expected/misc_functions.out index e845042d38..d0d584abb4 100644 --- a/src/test/regress/expected/misc_functions.out +++ b/src/test/regress/expected/misc_functions.out @@ -140,7 +140,7 @@ HINT: No function matches the given name and argument types. You might need to -- Furthermore, their contents can vary depending on the timing. However, -- we can at least verify that the code doesn't fail. -- -SELECT * FROM pg_log_backend_memory_contexts(pg_backend_pid()); +SELECT pg_log_backend_memory_contexts(pg_backend_pid()); pg_log_backend_memory_contexts -------------------------------- t diff --git a/src/test/regress/expected/privileges.out b/src/test/regress/expected/privileges.out index 83cff902f3..029c1e826f 100644 --- a/src/test/regress/expected/privileges.out +++ b/src/test/regress/expected/privileges.out @@ -2413,3 +2413,55 @@ REVOKE TRUNCATE ON lock_table FROM regress_locktable_user; -- clean up DROP TABLE lock_table; DROP USER regress_locktable_user; +-- test to check privileges of system views pg_shmem_allocations, +-- pg_backend_memory_contexts and function pg_log_backend_memory_contexts. +-- switch to superuser +\c - +CREATE ROLE regress_nosprusr_noreadallstats WITH NOSUPERUSER; +SET ROLE regress_nosprusr_noreadallstats; +-- The entire output of pg_backend_memory_contexts is not stable, +-- we test only the privileges and basic condition of TopMemoryContext. +SELECT name, ident, parent, level, total_bytes >= free_bytes + FROM pg_backend_memory_contexts WHERE level = 0; -- permission denied error +ERROR: permission denied for view pg_backend_memory_contexts +SELECT COUNT(*) >= 0 AS ok FROM pg_shmem_allocations; -- permission denied error +ERROR: permission denied for view pg_shmem_allocations +-- Memory contexts are logged and they are not returned to the function. +-- Furthermore, their contents can vary depending on the timing. However, +-- we can at least verify the privileges. +SELECT pg_log_backend_memory_contexts(pg_backend_pid()); -- permission denied error +ERROR: must be superuser or a member of the pg_read_all_stats role to log memory contexts +-- switch to superuser +\c - +CREATE ROLE regress_nosprusr_readallstats WITH NOSUPERUSER; +GRANT pg_read_all_stats TO regress_nosprusr_readallstats; +SET ROLE regress_nosprusr_readallstats; +-- The entire output of pg_backend_memory_contexts is not stable, +-- we test only the privileges and basic condition of TopMemoryContext. +SELECT name, ident, parent, level, total_bytes >= free_bytes + FROM pg_backend_memory_contexts WHERE level = 0; + name | ident | parent | level | ?column? +------------------+-------+--------+-------+---------- + TopMemoryContext | | | 0 | t +(1 row) + +SELECT COUNT(*) >= 0 AS ok FROM pg_shmem_allocations; + ok +---- + t +(1 row) + +-- Memory contexts are logged and they are not returned to the function. +-- Furthermore, their contents can vary depending on the timing. However, +-- we can at least verify the privileges. +SELECT pg_log_backend_memory_contexts(pg_backend_pid()); + pg_log_backend_memory_contexts +-------------------------------- + t +(1 row) + +-- switch to superuser +\c - +-- clean up +DROP ROLE regress_nosprusr_noreadallstats; +DROP ROLE regress_nosprusr_readallstats; diff --git a/src/test/regress/sql/misc_functions.sql b/src/test/regress/sql/misc_functions.sql index a398349afc..94bf995fe2 100644 --- a/src/test/regress/sql/misc_functions.sql +++ b/src/test/regress/sql/misc_functions.sql @@ -37,7 +37,7 @@ SELECT num_nulls(); -- Furthermore, their contents can vary depending on the timing. However, -- we can at least verify that the code doesn't fail. -- -SELECT * FROM pg_log_backend_memory_contexts(pg_backend_pid()); +SELECT pg_log_backend_memory_contexts(pg_backend_pid()); -- -- Test some built-in SRFs diff --git a/src/test/regress/sql/privileges.sql b/src/test/regress/sql/privileges.sql index 3d1a1db987..0f03f1c535 100644 --- a/src/test/regress/sql/privileges.sql +++ b/src/test/regress/sql/privileges.sql @@ -1476,3 +1476,44 @@ REVOKE TRUNCATE ON lock_table FROM regress_locktable_user; -- clean up DROP TABLE lock_table; DROP USER regress_locktable_user; + +-- test to check privileges of system views pg_shmem_allocations, +-- pg_backend_memory_contexts and function pg_log_backend_memory_contexts. + +-- switch to superuser +\c - + +CREATE ROLE regress_nosprusr_noreadallstats WITH NOSUPERUSER; +SET ROLE regress_nosprusr_noreadallstats; +-- The entire output of pg_backend_memory_contexts is not stable, +-- we test only the privileges and basic condition of TopMemoryContext. +SELECT name, ident, parent, level, total_bytes >= free_bytes + FROM pg_backend_memory_contexts WHERE level = 0; -- permission denied error +SELECT COUNT(*) >= 0 AS ok FROM pg_shmem_allocations; -- permission denied error +-- Memory contexts are logged and they are not returned to the function. +-- Furthermore, their contents can vary depending on the timing. However, +-- we can at least verify the privileges. +SELECT pg_log_backend_memory_contexts(pg_backend_pid()); -- permission denied error + +-- switch to superuser +\c - + +CREATE ROLE regress_nosprusr_readallstats WITH NOSUPERUSER; +GRANT pg_read_all_stats TO regress_nosprusr_readallstats; +SET ROLE regress_nosprusr_readallstats; +-- The entire output of pg_backend_memory_contexts is not stable, +-- we test only the privileges and basic condition of TopMemoryContext. +SELECT name, ident, parent, level, total_bytes >= free_bytes + FROM pg_backend_memory_contexts WHERE level = 0; +SELECT COUNT(*) >= 0 AS ok FROM pg_shmem_allocations; +-- Memory contexts are logged and they are not returned to the function. +-- Furthermore, their contents can vary depending on the timing. However, +-- we can at least verify the privileges. +SELECT pg_log_backend_memory_contexts(pg_backend_pid()); + +-- switch to superuser +\c - + +-- clean up +DROP ROLE regress_nosprusr_noreadallstats; +DROP ROLE regress_nosprusr_readallstats; -- 2.25.1