agora inbox for pgsql-hackers@postgresql.org
help / color / mirror / Atom feedFrom: Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
To: shihao zhong <zhong950419@gmail.com>
Cc: Michael Paquier <michael@paquier.xyz>
Cc: Jim Jones <jim.jones@uni-muenster.de>
Cc: pgsql-hackers <pgsql-hackers@lists.postgresql.org>
Subject: Re: Add a permission check to pg_stat_get_backend_subxact()
Date: Wed, 23 Sep 2026 07:23:01 +0000
Message-ID: <arN+VT3xyiTY/iUD@bdtpg> (raw)
In-Reply-To: <CAGRkXqTLuE3PrRn7tXJ8t+7UHcTUJ9cuaFxbZkuhpAySD0RpfA@mail.gmail.com>
References: <c7a0be5d-5328-41dd-a7bf-6b964e9f7a06@uni-muenster.de>
<aqSXMjhwFFzz_zyo@paquier.xyz>
<CAGRkXqRGVNvvkY7a7X=3Bn=WgfLuFZ5iVsBf+t6HSfm2RwVC+g@mail.gmail.com>
<aqec3MF_GDsMdadl@paquier.xyz>
<arD/w47Ug1GaObfq@bdtpg>
<arJJVOo1mOKvK8US@paquier.xyz>
<arJSXjs6szWXIr0H@bdtpg>
<arJlSpeSs8yPKipu@paquier.xyz>
<arKKltvf8PpIByOF@bdtpg>
<CAGRkXqTLuE3PrRn7tXJ8t+7UHcTUJ9cuaFxbZkuhpAySD0RpfA@mail.gmail.com>
Hi,
On Tue, Sep 22, 2026 at 11:22:40PM -0400, shihao zhong wrote:
> > Do you mean adding the user ID on top of Shihao's 0002? If so, I can have
> a look,
> > unless Shihao is already planning to update the patch along those lines?
>
> Done in v8, attached.
Thanks!
I've a few comments:
> 0001 is the PID check, same as v7-0002. It goes first now because 0002
> needs a field in PgStat_Backend.
>
> 0002 stores the user ID in PgStat_Backend next to the PID when the entry
> is created, and pg_stat_get_backend_io(), wal() and lock() check the
> caller against that instead of the beentry. The reset callback restores
> both fields. pg_stat_get_backend_subxact() still checks the beentry, the
> counters it reports come from there.
=== 1
pgstat_read_current_status() first copies the activity entry and then calls
ProcNumberGetTransactionIds() separately. If the backend exits and its
ProcNumber is reused in between, the userid can belong to the old backend
while the subxact counters belong to the new one.
This race exists before the patch, but it matters for the new permission check.
I wonder if we should pass the copied PID to ProcNumberGetTransactionIds() and
validate it under ProcArrayLock, following the same idea as
pgstat_fetch_stat_backend_by_pid()?
=== 2
typedef struct PgStat_Backend
{
+ int pid; /* PID of the backend owning these stats */
TimestampTz stat_reset_timestamp;
0002 explicitly says that it is not intended for backpatching, but what about
0001? If it is backpatched to v18, adding pid here changes the offsets of all
the existing fields.
I think it would make sense to add pid at the end in the backbranches (if we
back patch it), as suggested in [1]. That would preserve the existing field
offsets, though it would still change sizeof(PgStat_Backend). FWIW, I could not
find any use of sizeof(PgStat_Backend) in a GitHub code search and there is
no padding to add the new field into.
[1]: https://wiki.postgresql.org/wiki/Committing_checklist
Regards,
--
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com
view thread (21+ messages) latest in thread
Message-ID: <arN+VT3xyiTY/iUD@bdtpg>
Permalink: ../arN+VT3xyiTY%2FiUD@bdtpg/
Also on: postgresql.org/message-id/arN+VT3xyiTY/iUD@bdtpg
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: bertranddrouvot.pg@gmail.com, zhong950419@gmail.com, michael@paquier.xyz, jim.jones@uni-muenster.de, pgsql-hackers@lists.postgresql.org
Subject: Re: Add a permission check to pg_stat_get_backend_subxact()
In-Reply-To: <arN+VT3xyiTY/iUD@bdtpg>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox