From 6bdf78c18342a960f2c014ac861beaaf7cca397a Mon Sep 17 00:00:00 2001 From: Shihao Zhong Date: Fri, 11 Sep 2026 12:26:11 +0200 Subject: [PATCH v8 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 hide these details from 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() lacked this check. Add it, for consistency with the sibling functions. Like pg_stat_activity, this also hides the statistics of processes owned by no role, such as autovacuum workers or the WAL writer, from callers without these privileges. pg_stat_get_backend_subxact() reads the backend status entry, so it checks the role recorded there, like its sibling functions. The last three report cumulative statistics that may come from a snapshot, taken while an older backend held the same proc number, so the role recorded in the backend status entry may not be the one that owns the statistics. Store the OID of the role that owns the backend in PgStat_Backend when its entry is created, and check the caller against that, based on the statistics data alone. 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. This changes the output of existing functions for callers lacking the required privileges, so no backpatch is done. Author: Shihao Zhong Author: Jim Jones Reviewed-by: Michael Paquier Discussion: https://postgr.es/m/CAGRkXqTBZ+zbVuDC8xGEB6Btj61hsui5H5nGqzFBDyOXc=4bjQ@mail.gmail.com --- doc/src/sgml/monitoring.sgml | 25 +++++++++ src/backend/utils/activity/backend_status.c | 2 +- src/backend/utils/activity/pgstat_backend.c | 15 +++++- src/backend/utils/adt/pgstatfuncs.c | 13 +++-- src/include/pgstat.h | 3 +- src/include/utils/pgstat_internal.h | 2 + src/test/regress/expected/stats.out | 59 +++++++++++++++++++++ src/test/regress/sql/stats.sql | 45 ++++++++++++++++ 8 files changed, 156 insertions(+), 8 deletions(-) diff --git a/doc/src/sgml/monitoring.sgml b/doc/src/sgml/monitoring.sgml index 62dadf3e86c..78f224bf9e8 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 details of a session's activity are only shown if + 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/backend_status.c b/src/backend/utils/activity/backend_status.c index d685fc5cd87..fdeae791227 100644 --- a/src/backend/utils/activity/backend_status.c +++ b/src/backend/utils/activity/backend_status.c @@ -460,7 +460,7 @@ pgstat_bestart_final(void) /* Create the backend statistics entry */ if (pgstat_tracks_backend_bktype(MyBackendType)) - pgstat_create_backend(MyProcNumber); + pgstat_create_backend(MyProcNumber, userid); /* Update app name to current GUC setting */ if (application_name) diff --git a/src/backend/utils/activity/pgstat_backend.c b/src/backend/utils/activity/pgstat_backend.c index b54789eac35..c118ca2d3f2 100644 --- a/src/backend/utils/activity/pgstat_backend.c +++ b/src/backend/utils/activity/pgstat_backend.c @@ -394,9 +394,14 @@ pgstat_backend_flush_cb(bool nowait) /* * Create backend statistics entry for proc number. + * + * "userid" is the OID of the role that owns the backend, or InvalidOid for a + * process that has no session user. It is stored in the entry so that the + * functions reporting these statistics can check whether their caller is + * allowed to see them, based on the statistics data alone. */ void -pgstat_create_backend(ProcNumber procnum) +pgstat_create_backend(ProcNumber procnum, Oid userid) { PgStat_EntryRef *entry_ref; PgStatShared_Backend *shstatent; @@ -411,7 +416,9 @@ pgstat_create_backend(ProcNumber procnum) */ memset(&shstatent->stats, 0, sizeof(shstatent->stats)); shstatent->stats.pid = MyProcPid; + shstatent->stats.userid = userid; shstatent->pid = MyProcPid; + shstatent->userid = userid; pgstat_unlock_entry(entry_ref); MemSet(&PendingBackendStats, 0, sizeof(PgStat_BackendPending)); @@ -485,6 +492,10 @@ pgstat_backend_reset_timestamp_cb(PgStatShared_Common *header, TimestampTz ts) shstatent->stats.stat_reset_timestamp = ts; - /* a reset zeroes the whole entry, so restore the PID of its owner */ + /* + * A reset zeroes the whole entry, so restore the PID and the owner of the + * backend. + */ shstatent->stats.pid = shstatent->pid; + shstatent->stats.userid = shstatent->userid; } diff --git a/src/backend/utils/adt/pgstatfuncs.c b/src/backend/utils/adt/pgstatfuncs.c index 64b6f60516c..1e6702794ed 100644 --- a/src/backend/utils/adt/pgstatfuncs.c +++ b/src/backend/utils/adt/pgstatfuncs.c @@ -838,7 +838,9 @@ pg_stat_get_backend_subxact(PG_FUNCTION_ARGS) 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); @@ -1682,7 +1684,8 @@ pg_stat_get_backend_io(PG_FUNCTION_ARGS) pid = PG_GETARG_INT32(0); backend_stats = pgstat_fetch_stat_backend_by_pid(pid, &bktype); - if (!backend_stats) + /* Report the details of a session only to a caller allowed to see them */ + if (!backend_stats || !HAS_PGSTAT_PERMISSIONS(backend_stats->userid)) return (Datum) 0; bktype_stats = &backend_stats->io_stats; @@ -1775,7 +1778,8 @@ pg_stat_get_backend_wal(PG_FUNCTION_ARGS) pid = PG_GETARG_INT32(0); backend_stats = pgstat_fetch_stat_backend_by_pid(pid, NULL); - if (!backend_stats) + /* Report the details of a session only to a caller allowed to see them */ + if (!backend_stats || !HAS_PGSTAT_PERMISSIONS(backend_stats->userid)) PG_RETURN_NULL(); bktype_stats = backend_stats->wal_counters; @@ -1866,7 +1870,8 @@ pg_stat_get_backend_lock(PG_FUNCTION_ARGS) pid = PG_GETARG_INT32(0); backend_stats = pgstat_fetch_stat_backend_by_pid(pid, NULL); - if (!backend_stats) + /* Report the details of a session only to a caller allowed to see them */ + if (!backend_stats || !HAS_PGSTAT_PERMISSIONS(backend_stats->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 eb66ae79ee9..410baf8aa18 100644 --- a/src/include/pgstat.h +++ b/src/include/pgstat.h @@ -592,6 +592,7 @@ typedef struct PgStat_WalStats typedef struct PgStat_Backend { int pid; /* PID of the backend owning these stats */ + Oid userid; /* role owning the backend, or InvalidOid */ TimestampTz stat_reset_timestamp; PgStat_BktypeIO io_stats; PgStat_WalCounters wal_counters; @@ -674,7 +675,7 @@ extern PgStat_Backend *pgstat_fetch_stat_backend(ProcNumber procNumber); extern PgStat_Backend *pgstat_fetch_stat_backend_by_pid(int pid, BackendType *bktype); extern bool pgstat_tracks_backend_bktype(BackendType bktype); -extern void pgstat_create_backend(ProcNumber procnum); +extern void pgstat_create_backend(ProcNumber procnum, Oid userid); /* * Functions in pgstat_bgwriter.c diff --git a/src/include/utils/pgstat_internal.h b/src/include/utils/pgstat_internal.h index 0ed2830d4a0..02b2eedb32b 100644 --- a/src/include/utils/pgstat_internal.h +++ b/src/include/utils/pgstat_internal.h @@ -537,7 +537,9 @@ typedef struct PgStatShared_Backend PgStatShared_Common header; PgStat_Backend stats; + /* copies of the fields of "stats" to restore after a reset */ int pid; + Oid userid; } PgStatShared_Backend; /* diff --git a/src/test/regress/expected/stats.out b/src/test/regress/expected/stats.out index 8b15471248b..fc1870bfd4b 100644 --- a/src/test/regress/expected/stats.out +++ b/src/test/regress/expected/stats.out @@ -1141,6 +1141,65 @@ 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 +SELECT current_user AS regress_stat_backend_owner \gset +CREATE ROLE regress_stat_backend_role; +-- a role with privileges of the role that owns this backend sees them +GRANT :"regress_stat_backend_owner" 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; +REVOKE :"regress_stat_backend_owner" FROM regress_stat_backend_role; +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..17c8e2f5231 100644 --- a/src/test/regress/sql/stats.sql +++ b/src/test/regress/sql/stats.sql @@ -535,6 +535,51 @@ 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 +SELECT current_user AS regress_stat_backend_owner \gset +CREATE ROLE regress_stat_backend_role; +-- a role with privileges of the role that owns this backend sees them +GRANT :"regress_stat_backend_owner" 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; +REVOKE :"regress_stat_backend_owner" FROM regress_stat_backend_role; +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.37.1 (Apple Git-137.1)