Re: Add a permission check to pg_stat_get_backend_subxact()

From: Bertrand Drouvot <bertranddrouvot(dot)pg(at)gmail(dot)com>
To: Michael Paquier <michael(at)paquier(dot)xyz>
Cc: shihao zhong <zhong950419(at)gmail(dot)com>, Jim Jones <jim(dot)jones(at)uni-muenster(dot)de>, pgsql-hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: Add a permission check to pg_stat_get_backend_subxact()
Date: 2026-09-21 09:58:27
Message-ID: arD/w47Ug1GaObfq@bdtpg
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Hi,

On Mon, Sep 14, 2026 at 04:06:04PM +0900, Michael Paquier wrote:
> On Sat, Sep 12, 2026 at 08:43:28AM -0400, shihao zhong wrote:
> > Thanks for committing that, I will not include 0001 in the following emails.
>
> Fixed the subxact_overflow -> subxact_overflowed, as that's
> independent.
>
> > 1. The first test block ran as superuser, so the owner branch of
> > HAS_PGSTAT_PERMISSIONS() was never exercised: with "userid" forced to
> > InvalidOid the test still passed. The block now grants the test role
> > membership in the session's role instead. With that, forcing userid
> > to InvalidOid fails the test, and removing the checks fails the
> > "unrelated role" block.
> >
> > 2. The doc paragraph above the per-backend table said the functions
> > "return NULL", but activity/wait_event return "<insufficient
> > privilege>" and the SRFs return no rows. Reworded.
> >
> > 3. Commit message: noted that processes owned by no role (autovacuum
> > workers, WAL writer, ...) are now visible only to superusers and
> > pg_read_all_stats, as in pg_stat_activity, and that no backpatch is
> > done.
>
> That seems globally sensible, at quick glance. I am also adding
> Bertrand Drouvot in CC to comment about this change, as he has worked
> on three of these functions.
>
> @Bertrand, what do you think?

pg_stat_io, pg_stat_wal and pg_stat_lock expose aggregate statistics without
restrictions but as pg_stat_get_backend_io(), pg_stat_get_backend_wal() and
pg_stat_get_backend_lock() expose the stats for a particular backend, I think the
proposed patch makes sense.

One thing I noticed while looking at this is that with stats_fetch_consistency = snapshot,
pgstat_fetch_stat_backend_by_pid() could validate the PID and user from one backend
while returning cumulative statistics cached for an older backend that used the
same ProcNumber.

The race is not introduced by this patch, but the new permission check makes it
more relevant here. Worth to fix at the same time?

Regards,

--
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Andrey Borodin 2026-09-21 10:27:32 Re: [PATCH] Use bounded GIN pending-list cleanup in parallel autovacuum
Previous Message Alexandre Felipe 2026-09-21 09:56:11 Re: Restructured Shared Buffer Hash Table