From: Jeff Davis <pgsql@j-davis.com>
To: Michael Paquier <michael@paquier.xyz>
Cc: Kyotaro Horiguchi <horikyota.ntt@gmail.com>
Cc: bossartn@amazon.com
Cc: pgsql-hackers@postgresql.org
Subject: Re: Allow pg_signal_backend members to use pg_log_backend_memory_stats().
Date: Mon, 25 Oct 2021 13:42:07 -0700
Message-ID: <66fbd8a7b7b0037c9e1df48b41941aa587c67f4d.camel@j-davis.com> (raw)
In-Reply-To: <YXZYVw7JF0mNtPue@paquier.xyz>
References: <e5cf6684d17c8d1ef4904ae248605ccd6da03e72.camel@j-davis.com>
<33F34F0C-BB16-48DE-B125-95D340A54AE8@amazon.com>
<d87b3a4fa3dacb103a8f3f50751ec9e638b2e2b1.camel@j-davis.com>
<20211025.115333.563958443822411952.horikyota.ntt@gmail.com>
<ab46bf8a5158d27cdbb6901f49a6e38dd23e619e.camel@j-davis.com>
<YXZYVw7JF0mNtPue@paquier.xyz>
On Mon, 2021-10-25 at 16:10 +0900, Michael Paquier wrote:
> Hmm. Why don't you split the patch into two parts that can be
> discussed separately then? There would be one to remove all the
> superuser() checks you can think of, and a potential second to grant
> those function's execution to some system role.
Good idea. Attached a patch to remove the superuser check on
pg_log_backend_memory_contexts(), except in the case when trying to log
memory contexts of a superuser backend.
Using pg_signal_backend does not seem to be universally acceptable, so
I'll just drop the idea of granting to that predefined role.
Regards,
Jeff Davis
Attachments:pg-log-memory-contexts.diff (text/x-patch, 6.4K) downloaddiff --git a/doc/src/sgml/func.sgml b/doc/src/sgml/func.sgmlindex 5677032cb28..b7003fd37a3 100644--- a/doc/src/sgml/func.sgml+++ b/doc/src/sgml/func.sgml@@ -25332,7 +25332,8 @@ 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.+ Only superusers can request to log the memory contexts of superuser+ backends.
</para></entry>
</row>
diff --git a/src/backend/catalog/system_functions.sql b/src/backend/catalog/system_functions.sqlindex a416e94d371..54c93b16c4c 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.cindex 0d52613bc32..38ca0ddee73 100644--- a/src/backend/utils/adt/mcxtfuncs.c+++ b/src/backend/utils/adt/mcxtfuncs.c@@ -162,10 +162,11 @@ 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, but they will still+ * be prevented from logging the memory contexts of a superuser backend.
*
* 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 +178,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);
/*
@@ -205,6 +200,12 @@ pg_log_backend_memory_contexts(PG_FUNCTION_ARGS)
PG_RETURN_BOOL(false);
}
+ /* Only allow superusers to signal superuser-owned backends. */+ if (superuser_arg(proc->roleId) && !superuser())+ ereport(ERROR,+ (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),+ errmsg("must be a superuser to log memory contexts of superuser backend")));+
if (SendProcSignal(pid, PROCSIG_LOG_MEMORY_CONTEXT, proc->backendId) < 0)
{
/* Again, just a warning to allow loops */
diff --git a/src/include/catalog/catversion.h b/src/include/catalog/catversion.hindex 3253b8751b1..039f6338604 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 202110230
#endif
diff --git a/src/test/regress/expected/misc_functions.out b/src/test/regress/expected/misc_functions.outindex e845042d38d..07c3d3311b1 100644--- a/src/test/regress/expected/misc_functions.out+++ b/src/test/regress/expected/misc_functions.out@@ -138,14 +138,40 @@ 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)++-- still fails because it's a superuser backend+SET ROLE regress_log_memory;+SELECT pg_log_backend_memory_contexts(pg_backend_pid());+ERROR: must be a superuser to log memory contexts of superuser backend+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.sqlindex a398349afc6..577ac3d7171 100644--- a/src/test/regress/sql/misc_functions.sql+++ b/src/test/regress/sql/misc_functions.sql@@ -35,9 +35,32 @@ 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++-- still fails because it's a superuser backend+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
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: pgsql@j-davis.com, michael@paquier.xyz, horikyota.ntt@gmail.com, bossartn@amazon.com
Subject: Re: Allow pg_signal_backend members to use pg_log_backend_memory_stats().
In-Reply-To: <66fbd8a7b7b0037c9e1df48b41941aa587c67f4d.camel@j-davis.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox