On 2020-08-22 21:18, Michael Paquier wrote:
Thanks for reviewing!
On Fri, Aug 21, 2020 at 11:27:06PM +0900, torikoshia wrote:
OK. Added a regression test on sysviews.sql.
(0001-Added-a-regression-test-for-pg_backend_memory_contex.patch)
Fujii-san gave us an example, but I added more simple one considering
the simplicity of other tests on that.
What you have sent in 0001 looks fine to me. A small test is much
better than nothing.
Added a patch for relocating the codes to mcxtfuncs.c.
(patches/0001-Rellocated-the-codes-for-pg_backend_memory_contexts-.patch)
The same code is moved around line-by-line.
Of course, this restriction makes pg_backend_memory_contexts hard to
use
when the user of the target session is not granted pg_monitor because
the
scope of this view is session local.
In this case, I imagine additional operations something like
temporarily
granting pg_monitor to that user.
Hmm. I am not completely sure either that pg_monitor is the best fit
here, because this view provides information about a bunch of internal
structures. Something that could easily be done though is to revoke
the access from public, and then users could just set up GRANT
permissions post-initdb, with pg_monitor as one possible choice. This
is the safest path by default, and this stuff is of a caliber similar
to pg_shmem_allocations in terms of internal contents.
I think this is a better way than what I did in
0001-Rellocated-the-codes-for-pg_backend_memory_contexts-.patch.
Attached a patch.
It seems to me that you are missing one "REVOKE ALL on
pg_backend_memory_contexts FROM PUBLIC" in patch 0003.
By the way, if that was just for me, I would remove used_bytes, which
is just a computation from the total and free numbers. I'll defer
that point to Fujii-san.
--
Michael
On 2020/08/20 2:59, Kasahara Tatsuhito wrote:
I totally agree that it's not *enough*, but in contrast to you I
think
it's a good step. Subsequently we should add a way to get any
backends
memory usage.
It's not too hard to imagine how to serialize it in a way that can be
easily deserialized by another backend. I am imagining something like
sending a procsignal that triggers (probably at CFR() time) a backend
to
write its own memory usage into pg_memusage/<pid> or something
roughly
like that.
Sounds good. Maybe we can also provide the SQL-callable function
or view to read pg_memusage/<pid>, to make the analysis easier.
+1
I'm thinking about starting a new thread to discuss exposing other
backend's memory context.
Regards,
--
Atsushi Torikoshi
NTT DATA CORPORATION
From dc4fade9111dc3f91e992c4d5af393dd5ed03270 Mon Sep 17 00:00:00 2001
From: Atsushi Torikoshi <torikos...@oss.nttdata.com>
Date: Mon, 24 Jul 2020 11:14:32 +0900
Subject: [PATCH] Previously pg_backend_memory_contexts doesn't have any
restriction and anyone could access it. However, this view contains some
internal information of the memory context. This policy could cause security
issues. This patch revokes all on pg_shmem_allocations from public and only
the superusers can access it.
---
doc/src/sgml/catalogs.sgml | 4 ++++
src/backend/catalog/system_views.sql | 3 +++
2 files changed, 7 insertions(+)
diff --git a/doc/src/sgml/catalogs.sgml b/doc/src/sgml/catalogs.sgml
index 1232b24e74..9fe260ecff 100644
--- a/doc/src/sgml/catalogs.sgml
+++ b/doc/src/sgml/catalogs.sgml
@@ -9697,6 +9697,10 @@ SCRAM-SHA-256$<replaceable><iteration count></replaceable>:<replaceable>&l
</tgroup>
</table>
+ <para>
+ By default, the <structname>pg_backend_memory_contexts</structname> view can be
+ read only by superusers.
+ </para>
</sect1>
<sect1 id="view-pg-config">
diff --git a/src/backend/catalog/system_views.sql b/src/backend/catalog/system_views.sql
index ba5a23ac25..a2d61302f9 100644
--- a/src/backend/catalog/system_views.sql
+++ b/src/backend/catalog/system_views.sql
@@ -557,6 +557,9 @@ REVOKE EXECUTE ON FUNCTION pg_get_shmem_allocations() FROM PUBLIC;
CREATE VIEW pg_backend_memory_contexts AS
SELECT * FROM pg_get_backend_memory_contexts();
+REVOKE ALL ON pg_backend_memory_contexts FROM PUBLIC;
+REVOKE EXECUTE ON FUNCTION pg_get_backend_memory_contexts() FROM PUBLIC;
+
-- Statistics views
CREATE VIEW pg_stat_all_tables AS
--
2.18.1
