From f0b051e322d530a340e62f2ae16d99acdbcb3d05 Mon Sep 17 00:00:00 2001 From: Jeff Davis Date: Tue, 26 Oct 2021 13:13:52 -0700 Subject: [PATCH] Allow GRANT on pg_log_backend_memory_contexts(). Remove superuser check, allowing any user granted permissions on pg_log_backend_memory_contexts() to log the memory contexts of any backend. Note that this could allow a privileged non-superuser to log the memory contexts of a superuser backend, but as discussed, that does not seem to be a problem. Reviewed-by: Nathan Bossart, Bharath Rupireddy, Michael Paquier, Kyotaro Horiguchi, Andres Freund Discussion: https://postgr.es/m/e5cf6684d17c8d1ef4904ae248605ccd6da03e72.camel@j-davis.com --- doc/src/sgml/func.sgml | 1 - src/backend/catalog/system_functions.sql | 2 ++ src/backend/utils/adt/mcxtfuncs.c | 14 +++------ src/include/catalog/catversion.h | 2 +- src/test/regress/expected/misc_functions.out | 33 ++++++++++++++++++-- src/test/regress/sql/misc_functions.sql | 26 +++++++++++++-- 6 files changed, 62 insertions(+), 16 deletions(-) diff --git a/doc/src/sgml/func.sgml b/doc/src/sgml/func.sgml index 5677032cb2..4b49dff2ff 100644 --- a/doc/src/sgml/func.sgml +++ b/doc/src/sgml/func.sgml @@ -25332,7 +25332,6 @@ SELECT collation for ('foo' COLLATE "de_DE"); (See for more information), but will not be sent to the client regardless of . - Only superusers can request to log the memory contexts. diff --git a/src/backend/catalog/system_functions.sql b/src/backend/catalog/system_functions.sql index a416e94d37..54c93b16c4 100644 --- a/src/backend/catalog/system_functions.sql +++ b/src/backend/catalog/system_functions.sql @@ -699,6 +699,8 @@ REVOKE EXECUTE ON FUNCTION pg_ls_dir(text) FROM public; REVOKE EXECUTE ON FUNCTION pg_ls_dir(text,boolean,boolean) FROM public; +REVOKE EXECUTE ON FUNCTION pg_log_backend_memory_contexts(integer) FROM PUBLIC; + -- -- We also set up some things as accessible to standard roles. -- diff --git a/src/backend/utils/adt/mcxtfuncs.c b/src/backend/utils/adt/mcxtfuncs.c index 0d52613bc3..6ddbf70b30 100644 --- a/src/backend/utils/adt/mcxtfuncs.c +++ b/src/backend/utils/adt/mcxtfuncs.c @@ -162,10 +162,10 @@ pg_get_backend_memory_contexts(PG_FUNCTION_ARGS) * pg_log_backend_memory_contexts * Signal a backend process to log its memory contexts. * - * Only superusers are allowed to signal to log the memory contexts - * because allowing any users to issue this request at an unbounded - * rate would cause lots of log messages and which can lead to - * denial of service. + * By default, only superusers are allowed to signal to log the memory + * contexts because allowing any users to issue this request at an unbounded + * rate would cause lots of log messages and which can lead to denial of + * service. Additional roles can be permitted with GRANT. * * On receipt of this signal, a backend sets the flag in the signal * handler, which causes the next CHECK_FOR_INTERRUPTS() to log the @@ -177,12 +177,6 @@ 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()) - ereport(ERROR, - (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE), - errmsg("must be a superuser to log memory contexts"))); - proc = BackendPidGetProc(pid); /* diff --git a/src/include/catalog/catversion.h b/src/include/catalog/catversion.h index 3253b8751b..468491825a 100644 --- a/src/include/catalog/catversion.h +++ b/src/include/catalog/catversion.h @@ -53,6 +53,6 @@ */ /* yyyymmddN */ -#define CATALOG_VERSION_NO 202109101 +#define CATALOG_VERSION_NO 202110260 #endif diff --git a/src/test/regress/expected/misc_functions.out b/src/test/regress/expected/misc_functions.out index e845042d38..71d316cad3 100644 --- a/src/test/regress/expected/misc_functions.out +++ b/src/test/regress/expected/misc_functions.out @@ -138,14 +138,43 @@ HINT: No function matches the given name and argument types. You might need to -- -- 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 that the code doesn't fail. +-- we can at least verify that the code doesn't fail, and that the +-- permissions are set properly. -- -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 (1 row) +CREATE ROLE regress_log_memory; +SELECT has_function_privilege('regress_log_memory', + 'pg_log_backend_memory_contexts(integer)', 'EXECUTE'); -- no + has_function_privilege +------------------------ + f +(1 row) + +GRANT EXECUTE ON FUNCTION pg_log_backend_memory_contexts(integer) + TO regress_log_memory; +SELECT has_function_privilege('regress_log_memory', + 'pg_log_backend_memory_contexts(integer)', 'EXECUTE'); -- yes + has_function_privilege +------------------------ + t +(1 row) + +SET ROLE regress_log_memory; +SELECT pg_log_backend_memory_contexts(pg_backend_pid()); + pg_log_backend_memory_contexts +-------------------------------- + t +(1 row) + +RESET ROLE; +REVOKE EXECUTE ON FUNCTION pg_log_backend_memory_contexts(integer) + FROM regress_log_memory; +DROP ROLE regress_log_memory; -- -- Test some built-in SRFs -- diff --git a/src/test/regress/sql/misc_functions.sql b/src/test/regress/sql/misc_functions.sql index a398349afc..8c23874b3f 100644 --- a/src/test/regress/sql/misc_functions.sql +++ b/src/test/regress/sql/misc_functions.sql @@ -35,9 +35,31 @@ SELECT num_nulls(); -- -- 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 that the code doesn't fail. +-- we can at least verify that the code doesn't fail, and that the +-- permissions are set properly. -- -SELECT * FROM pg_log_backend_memory_contexts(pg_backend_pid()); + +SELECT pg_log_backend_memory_contexts(pg_backend_pid()); + +CREATE ROLE regress_log_memory; + +SELECT has_function_privilege('regress_log_memory', + 'pg_log_backend_memory_contexts(integer)', 'EXECUTE'); -- no + +GRANT EXECUTE ON FUNCTION pg_log_backend_memory_contexts(integer) + TO regress_log_memory; + +SELECT has_function_privilege('regress_log_memory', + 'pg_log_backend_memory_contexts(integer)', 'EXECUTE'); -- yes + +SET ROLE regress_log_memory; +SELECT pg_log_backend_memory_contexts(pg_backend_pid()); +RESET ROLE; + +REVOKE EXECUTE ON FUNCTION pg_log_backend_memory_contexts(integer) + FROM regress_log_memory; + +DROP ROLE regress_log_memory; -- -- Test some built-in SRFs