Hi Bertrand,

> Worth to fix at the same time?

Thanks, I can reproduce it. A role with no privileges reads the IO and WAL
numbers of a superuser session under the PID of its own session.

v5-0002 fixes it by making the PID part of the object ID of the backend
stats entries. The entry of an older backend is then never found with
the PID of a newer one, and nothing needs to be checked after the fetch.
Entries are still dropped at exit, so the number of live entries is still
bounded by the number of proc numbers.

It is a separate patch as the problem has existed since v18.

v5-0001 is v4 rebased, without the tuple descriptor fix that Michael has
applied.

Thanks,
Shihao

On Mon, Sep 21, 2026 at 5:58 AM Bertrand Drouvot <
[email protected]> wrote:

> 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
>

Attachment: v5-0001-Make-per-backend-statistics-functions-respect-sta.patch
Description: Binary data

Attachment: v5-0002-Include-the-PID-in-the-key-of-backend-statistics-.patch
Description: Binary data

Reply via email to