From af1e5e7fb321c3a132bdc2b04804ba8e0bb29bb1 Mon Sep 17 00:00:00 2001 From: Jim Jones Date: Fri, 11 Sep 2026 12:26:11 +0200 Subject: [PATCH v3 2/2] Make per-backend statistics functions respect statistics permissions The per-backend statistics functions that report the details of a session call HAS_PGSTAT_PERMISSIONS() first, so that they report nothing to a caller that is neither a superuser, nor has privileges of pg_read_all_stats, nor is a member of the role that owns the session. pg_stat_get_backend_subxact(), pg_stat_get_backend_io(), pg_stat_get_backend_wal() and pg_stat_get_backend_lock() had no such check and reported their statistics to any caller. Add the check the sibling functions use. The last three look up a backend by PID and have no backend status entry at hand, so pgstat_fetch_stat_backend_by_pid() gains an optional "userid" output argument, next to the existing "bktype" one, returning the OID of the role that owns the backend. The permission rule was documented for the dynamic statistics views but not for these functions, so state it above the per-backend function table and on the three functions listed among the additional statistics functions. While on it, correct the name of the subxact_overflowed column in the tuple descriptor built by pg_stat_get_backend_subxact(). Author: Shihao Zhong Author: Jim Jones Reviewed-by: Michael Paquier Discussion: https://www.postgresql.org/message-id/flat/CAGRkXqTBZ%2BzbVuDC8xGEB6Btj61hsui5H5nGqzFBDyOXc%3D4bjQ%40mail.gmail.com --- doc/src/sgml/monitoring.sgml | 25 ++++++++++ src/backend/utils/activity/pgstat_backend.c | 12 ++++- src/backend/utils/adt/pgstatfuncs.c | 24 ++++++--- src/include/pgstat.h | 3 +- src/test/regress/expected/stats.out | 54 +++++++++++++++++++++ src/test/regress/sql/stats.sql | 40 +++++++++++++++ 6 files changed, 147 insertions(+), 11 deletions(-) diff --git a/doc/src/sgml/monitoring.sgml b/doc/src/sgml/monitoring.sgml index 49bf6b51c49..32168bc6f70 100644 --- a/doc/src/sgml/monitoring.sgml +++ b/doc/src/sgml/monitoring.sgml @@ -5799,6 +5799,11 @@ description | Waiting for a newly initialized WAL file to reach durable storage the background writer, the startup process and the autovacuum launcher as they are already visible in the pg_stat_io view and there is only one of each. + + + This function returns no rows unless the caller is a superuser, has + privileges of the pg_read_all_stats role, or is a + member of the role that owns the backend. @@ -5834,6 +5839,11 @@ description | Waiting for a newly initialized WAL file to reach durable storage The function does not return lock statistics for the checkpointer, the background writer, the startup process and the autovacuum launcher. + + + This function returns no rows unless the caller is a superuser, has + privileges of the pg_read_all_stats role, or is a + member of the role that owns the backend. @@ -5853,6 +5863,11 @@ description | Waiting for a newly initialized WAL file to reach durable storage The function does not return WAL statistics for the checkpointer, the background writer, the startup process and the autovacuum launcher. + + + This function returns NULL unless the caller is a superuser, has + privileges of the pg_read_all_stats role, or is a + member of the role that owns the backend. @@ -6211,6 +6226,16 @@ FROM pg_stat_get_backend_idset() AS backendid; + + These functions are security restricted in the same way as + pg_stat_activity. The existence of a session and + its general properties, such as its session user and database, are visible + to all users, but the functions that report the details of a session's + activity return NULL unless the caller is a superuser, has privileges of the + pg_read_all_stats + role, or is a member of the role that owns the session. + + Per-Backend Statistics Functions diff --git a/src/backend/utils/activity/pgstat_backend.c b/src/backend/utils/activity/pgstat_backend.c index b736b2ccc6f..59bc7e699b5 100644 --- a/src/backend/utils/activity/pgstat_backend.c +++ b/src/backend/utils/activity/pgstat_backend.c @@ -140,10 +140,12 @@ pgstat_fetch_stat_backend(ProcNumber procNumber) * * This routine includes sanity checks to ensure that the backend exists and * is running. "bktype" can be optionally defined to return the BackendType - * of the backend whose statistics are returned. + * of the backend whose statistics are returned. "userid" can be optionally + * defined to return the OID of the role that owns the backend, for callers + * that need to check whether they are allowed to report its statistics. */ PgStat_Backend * -pgstat_fetch_stat_backend_by_pid(int pid, BackendType *bktype) +pgstat_fetch_stat_backend_by_pid(int pid, BackendType *bktype, Oid *userid) { PGPROC *proc; PgBackendStatus *beentry; @@ -153,6 +155,8 @@ pgstat_fetch_stat_backend_by_pid(int pid, BackendType *bktype) proc = BackendPidGetProc(pid); if (bktype) *bktype = B_INVALID; + if (userid) + *userid = InvalidOid; /* this could be an auxiliary process */ if (!proc) @@ -177,6 +181,8 @@ pgstat_fetch_stat_backend_by_pid(int pid, BackendType *bktype) if (bktype) *bktype = beentry->st_backendType; + if (userid) + *userid = beentry->st_userid; /* * Retrieve the entry. Note that "beentry" may be freed depending on the @@ -187,6 +193,8 @@ pgstat_fetch_stat_backend_by_pid(int pid, BackendType *bktype) { if (bktype) *bktype = B_INVALID; + if (userid) + *userid = InvalidOid; return NULL; } diff --git a/src/backend/utils/adt/pgstatfuncs.c b/src/backend/utils/adt/pgstatfuncs.c index 081cd006666..f9fc3b65b2a 100644 --- a/src/backend/utils/adt/pgstatfuncs.c +++ b/src/backend/utils/adt/pgstatfuncs.c @@ -832,13 +832,15 @@ pg_stat_get_backend_subxact(PG_FUNCTION_ARGS) tupdesc = CreateTemplateTupleDesc(PG_STAT_GET_SUBXACT_COLS); TupleDescInitEntry(tupdesc, (AttrNumber) 1, "subxact_count", INT4OID, -1, 0); - TupleDescInitEntry(tupdesc, (AttrNumber) 2, "subxact_overflow", + TupleDescInitEntry(tupdesc, (AttrNumber) 2, "subxact_overflowed", BOOLOID, -1, 0); TupleDescFinalize(tupdesc); BlessTupleDesc(tupdesc); - if ((local_beentry = pgstat_get_local_beentry_by_proc_number(procNumber)) != NULL) + /* Report the details of a session only to a caller allowed to see them */ + if ((local_beentry = pgstat_get_local_beentry_by_proc_number(procNumber)) != NULL && + HAS_PGSTAT_PERMISSIONS(local_beentry->backendStatus.st_userid)) { /* Fill values and NULLs */ values[0] = Int32GetDatum(local_beentry->backend_subxact_count); @@ -1673,6 +1675,7 @@ pg_stat_get_backend_io(PG_FUNCTION_ARGS) ReturnSetInfo *rsinfo; BackendType bktype; int pid; + Oid userid; PgStat_Backend *backend_stats; PgStat_BktypeIO *bktype_stats; @@ -1680,9 +1683,10 @@ pg_stat_get_backend_io(PG_FUNCTION_ARGS) rsinfo = (ReturnSetInfo *) fcinfo->resultinfo; pid = PG_GETARG_INT32(0); - backend_stats = pgstat_fetch_stat_backend_by_pid(pid, &bktype); + backend_stats = pgstat_fetch_stat_backend_by_pid(pid, &bktype, &userid); - if (!backend_stats) + /* Report the details of a session only to a caller allowed to see them */ + if (!backend_stats || !HAS_PGSTAT_PERMISSIONS(userid)) return (Datum) 0; bktype_stats = &backend_stats->io_stats; @@ -1769,13 +1773,15 @@ Datum pg_stat_get_backend_wal(PG_FUNCTION_ARGS) { int pid; + Oid userid; PgStat_Backend *backend_stats; PgStat_WalCounters bktype_stats; pid = PG_GETARG_INT32(0); - backend_stats = pgstat_fetch_stat_backend_by_pid(pid, NULL); + backend_stats = pgstat_fetch_stat_backend_by_pid(pid, NULL, &userid); - if (!backend_stats) + /* Report the details of a session only to a caller allowed to see them */ + if (!backend_stats || !HAS_PGSTAT_PERMISSIONS(userid)) PG_RETURN_NULL(); bktype_stats = backend_stats->wal_counters; @@ -1857,6 +1863,7 @@ Datum pg_stat_get_backend_lock(PG_FUNCTION_ARGS) { int pid; + Oid userid; ReturnSetInfo *rsinfo; PgStat_Backend *backend_stats; @@ -1864,9 +1871,10 @@ pg_stat_get_backend_lock(PG_FUNCTION_ARGS) rsinfo = (ReturnSetInfo *) fcinfo->resultinfo; pid = PG_GETARG_INT32(0); - backend_stats = pgstat_fetch_stat_backend_by_pid(pid, NULL); + backend_stats = pgstat_fetch_stat_backend_by_pid(pid, NULL, &userid); - if (!backend_stats) + /* Report the details of a session only to a caller allowed to see them */ + if (!backend_stats || !HAS_PGSTAT_PERMISSIONS(userid)) return (Datum) 0; pg_stat_lock_build_tuples(rsinfo, backend_stats->lock_stats.stats, diff --git a/src/include/pgstat.h b/src/include/pgstat.h index 187d82c96fe..4c3dcc03df5 100644 --- a/src/include/pgstat.h +++ b/src/include/pgstat.h @@ -671,7 +671,8 @@ extern void pgstat_count_backend_lock_fastpath_exceeded(uint8 locktag_type); extern PgStat_Backend *pgstat_fetch_stat_backend(ProcNumber procNumber); extern PgStat_Backend *pgstat_fetch_stat_backend_by_pid(int pid, - BackendType *bktype); + BackendType *bktype, + Oid *userid); extern bool pgstat_tracks_backend_bktype(BackendType bktype); extern void pgstat_create_backend(ProcNumber procnum); diff --git a/src/test/regress/expected/stats.out b/src/test/regress/expected/stats.out index 8b15471248b..a1ad1e86f64 100644 --- a/src/test/regress/expected/stats.out +++ b/src/test/regress/expected/stats.out @@ -1141,6 +1141,60 @@ WHERE pg_stat_get_backend_pid(beid) = pg_backend_pid(); t (1 row) +-- The per-backend statistics functions report the details of a session only +-- to a caller that is allowed to see them: a superuser, a role with +-- privileges of pg_read_all_stats, or the role that owns the session. +SELECT beid FROM pg_stat_get_backend_idset() beid +WHERE pg_stat_get_backend_pid(beid) = pg_backend_pid() \gset +CREATE ROLE regress_stat_backend_role; +-- the role that owns this backend sees the statistics +SELECT (SELECT subxact_count IS NOT NULL + FROM pg_stat_get_backend_subxact(:beid)) AS subxact, + (SELECT count(*) > 0 + FROM pg_stat_get_backend_io(pg_backend_pid())) AS io, + (SELECT count(*) > 0 + FROM pg_stat_get_backend_lock(pg_backend_pid())) AS locks, + (SELECT wal_records IS NOT NULL + FROM pg_stat_get_backend_wal(pg_backend_pid())) AS wal; + subxact | io | locks | wal +---------+----+-------+----- + t | t | t | t +(1 row) + +SET ROLE regress_stat_backend_role; +-- an unrelated role sees nothing +SELECT (SELECT subxact_count IS NOT NULL + FROM pg_stat_get_backend_subxact(:beid)) AS subxact, + (SELECT count(*) > 0 + FROM pg_stat_get_backend_io(pg_backend_pid())) AS io, + (SELECT count(*) > 0 + FROM pg_stat_get_backend_lock(pg_backend_pid())) AS locks, + (SELECT wal_records IS NOT NULL + FROM pg_stat_get_backend_wal(pg_backend_pid())) AS wal; + subxact | io | locks | wal +---------+----+-------+----- + f | f | f | f +(1 row) + +RESET ROLE; +-- but a role with privileges of pg_read_all_stats sees them again +GRANT pg_read_all_stats TO regress_stat_backend_role; +SET ROLE regress_stat_backend_role; +SELECT (SELECT subxact_count IS NOT NULL + FROM pg_stat_get_backend_subxact(:beid)) AS subxact, + (SELECT count(*) > 0 + FROM pg_stat_get_backend_io(pg_backend_pid())) AS io, + (SELECT count(*) > 0 + FROM pg_stat_get_backend_lock(pg_backend_pid())) AS locks, + (SELECT wal_records IS NOT NULL + FROM pg_stat_get_backend_wal(pg_backend_pid())) AS wal; + subxact | io | locks | wal +---------+----+-------+----- + t | t | t | t +(1 row) + +RESET ROLE; +DROP ROLE regress_stat_backend_role; ----- -- Test that resetting stats works for reset timestamp ----- diff --git a/src/test/regress/sql/stats.sql b/src/test/regress/sql/stats.sql index 674637e172b..bd9b56aaa6f 100644 --- a/src/test/regress/sql/stats.sql +++ b/src/test/regress/sql/stats.sql @@ -535,6 +535,46 @@ SELECT (current_schemas(true))[1] = ('pg_temp_' || beid::text) AS match FROM pg_stat_get_backend_idset() beid WHERE pg_stat_get_backend_pid(beid) = pg_backend_pid(); +-- The per-backend statistics functions report the details of a session only +-- to a caller that is allowed to see them: a superuser, a role with +-- privileges of pg_read_all_stats, or the role that owns the session. +SELECT beid FROM pg_stat_get_backend_idset() beid +WHERE pg_stat_get_backend_pid(beid) = pg_backend_pid() \gset +CREATE ROLE regress_stat_backend_role; +-- the role that owns this backend sees the statistics +SELECT (SELECT subxact_count IS NOT NULL + FROM pg_stat_get_backend_subxact(:beid)) AS subxact, + (SELECT count(*) > 0 + FROM pg_stat_get_backend_io(pg_backend_pid())) AS io, + (SELECT count(*) > 0 + FROM pg_stat_get_backend_lock(pg_backend_pid())) AS locks, + (SELECT wal_records IS NOT NULL + FROM pg_stat_get_backend_wal(pg_backend_pid())) AS wal; +SET ROLE regress_stat_backend_role; +-- an unrelated role sees nothing +SELECT (SELECT subxact_count IS NOT NULL + FROM pg_stat_get_backend_subxact(:beid)) AS subxact, + (SELECT count(*) > 0 + FROM pg_stat_get_backend_io(pg_backend_pid())) AS io, + (SELECT count(*) > 0 + FROM pg_stat_get_backend_lock(pg_backend_pid())) AS locks, + (SELECT wal_records IS NOT NULL + FROM pg_stat_get_backend_wal(pg_backend_pid())) AS wal; +RESET ROLE; +-- but a role with privileges of pg_read_all_stats sees them again +GRANT pg_read_all_stats TO regress_stat_backend_role; +SET ROLE regress_stat_backend_role; +SELECT (SELECT subxact_count IS NOT NULL + FROM pg_stat_get_backend_subxact(:beid)) AS subxact, + (SELECT count(*) > 0 + FROM pg_stat_get_backend_io(pg_backend_pid())) AS io, + (SELECT count(*) > 0 + FROM pg_stat_get_backend_lock(pg_backend_pid())) AS locks, + (SELECT wal_records IS NOT NULL + FROM pg_stat_get_backend_wal(pg_backend_pid())) AS wal; +RESET ROLE; +DROP ROLE regress_stat_backend_role; + ----- -- Test that resetting stats works for reset timestamp ----- -- 2.55.0