Skip to content

Commit

Permalink
Allow GRANT on pg_log_backend_memory_contexts().
Browse files Browse the repository at this point in the history
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
  • Loading branch information
jeff-davis committed Oct 26, 2021
1 parent 5fedf74 commit f0b051e
Show file tree
Hide file tree
Showing 6 changed files with 62 additions and 16 deletions.
1 change: 0 additions & 1 deletion doc/src/sgml/func.sgml
Original file line number Diff line number Diff line change
Expand Up @@ -25332,7 +25332,6 @@ SELECT collation for ('foo' COLLATE "de_DE");
(See <xref linkend="runtime-config-logging"/> for more information),
but will not be sent to the client regardless of
<xref linkend="guc-client-min-messages"/>.
Only superusers can request to log the memory contexts.
</para></entry>
</row>

Expand Down
2 changes: 2 additions & 0 deletions src/backend/catalog/system_functions.sql
Original file line number Diff line number Diff line change
Expand Up @@ -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.
--
Expand Down
14 changes: 4 additions & 10 deletions src/backend/utils/adt/mcxtfuncs.c
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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);

/*
Expand Down
2 changes: 1 addition & 1 deletion src/include/catalog/catversion.h
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,6 @@
*/

/* yyyymmddN */
#define CATALOG_VERSION_NO 202109101
#define CATALOG_VERSION_NO 202110260

#endif
33 changes: 31 additions & 2 deletions src/test/regress/expected/misc_functions.out
Original file line number Diff line number Diff line change
Expand Up @@ -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
--
Expand Down
26 changes: 24 additions & 2 deletions src/test/regress/sql/misc_functions.sql
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit f0b051e

Please sign in to comment.