Fix bug with accessing to temporary tables of other sessions

On Mon, Apr 14, 2025 at 12:36 PM Daniil Davydov <3danissimo@gmail.com>
wrote:

Hi,

During previous commitfest this topic already has been discussed
within the "Forbid to DROP temp tables of other sessions" thread [1].
Unfortunately its name doesn't reflect the real problem, so I decided
to start a new thread, as David G. Johnston advised.

Here are the summary results of the discussion [1] :
The superuser is only allowed to DROP temporary relations of other
sessions. Other commands (like SELECT, INSERT, UPDATE, DELETE ...)
must be forbidden to him. Error message for this case will look like
this : `could not access temporary relations of other sessions`.
For now, superuser still can specify such operations because of a bug
in the code that mistakenly recognizes other session's temp table as
permanent (I've covered this topic in more detail in [2]). Attached
patch fixes this bug (targeted on
b51f86e49a7f119004c0ce5d0be89cdf98309141).

Opened issue:
Not everyone liked the idea that table's persistence can be assigned
to table during makeRangeVarXXX calls (inside gram.y).
My opinion - `As far as "pg_temp_" prefix is reserved by the postgres
kernel, we can definitely say that we have encountered a temporary
table when we see this prefix.`

I will be glad to hear your opinion.

--
Best regards,
Daniil Davydov

[1]
/messages/by-id/CAJDiXgj72Axj0d4ojKdRWG_rnkfs4uWY414NL=15sCvh7-9rwg@mail.gmail.com
[2]
/messages/by-id/CAJDiXgj+5UKLWSUT5605rJhuw438NmEKecvhFAF2nnrMsgGK3w@mail.gmail.com

Hi Daniil,

Your patch for securing cross-session temp table access is a great
improvement. The RVR_OTHER_TEMP_OK flag elegantly handles the DROP case
while keeping the main restriction in place.

For schema name validation, an exact strcmp for "pg_temp" and proper
numeric parsing for "pg_temp_X" would be more precise than the current
prefix check. This would avoid any accidental matches to similarly named
schemas.

The error message could be adjusted to emphasize permissions, like
"permission denied for cross-session temp table access". This would make
the security intent clearer to users.

I noticed the Assert assumes myTempNamespace is always valid. While
correct, a brief comment explaining why this is safe would help future
maintainers. The relpersistence logic could also be centralized in one
place for consistency.

I've added an isolation test to verify the behavior when trying to access
another backend's temp tables. It confirms the restrictions work as
intended while allowing permitted operations.

Thanks for working on this important security enhancement!

Best regards,
Stepan Neretin

Hi,

On Mon, Jul 28, 2025 at 10:43 AM Stepan Neretin <slpmcf@gmail.com> wrote:

Your patch for securing cross-session temp table access is a great improvement. The RVR_OTHER_TEMP_OK flag elegantly handles the DROP case while keeping the main restriction in place.

For schema name validation, an exact strcmp for "pg_temp" and proper numeric parsing for "pg_temp_X" would be more precise than the current prefix check. This would avoid any accidental matches to similarly named schemas.

Thanks for looking into it!

The error message could be adjusted to emphasize permissions, like "permission denied for cross-session temp table access". This would make the security intent clearer to users.

I don't think that such an error message will be more appropriate. We
want to forbid this operation not because of "permission" reasons, but
because of the danger of this operation.
Yes, some people insist that dropping other sessions' temp tables might
be useful in some cases, but it is a "last resort" solution.

Even with this patch, DROP of other session temp tables can lead to
an error. I wrote about it here [1]/messages/by-id/CAJDiXghoi-FM4d5XVZzUyiuhv8DDm9JdGOU8KC47emasqi1GUw@mail.gmail.com.

I noticed the Assert assumes myTempNamespace is always valid. While correct, a brief comment explaining why this is safe would help future maintainers.

Well, v5 patch already contains comment for this assert :
/*
* If this table was recognized as temporary, it means that we
* found it because the backend's temporary namespace was specified
* in search_path. Thus, MyTempNamespace must contain valid oid.
*/

The relpersistence logic could also be centralized in one place for consistency.

I don't see a reason to separate this logic into a new function, because
there will be no more cases when it will be useful to us.

I've added an isolation test to verify the behavior when trying to access another backend's temp tables. It confirms the restrictions work as intended while allowing permitted operations.

Some time ago I also created a test for this situation, see patch in this [2]/messages/by-id/CAJDiXgi9CWaZCVcHmvAT604RrAqDN5zpOYxZq92adqkPq5QbnQ@mail.gmail.com
message. it worked in a similar way (but covered more test cases).
It caused a mixed reaction from people, so I decided to abandon this idea.

I guess it might be a discussion point in the future, but first I'd like to
settle the core logic of the patch.

I attach a v7 patch to this letter. No changes yet, just rebased on the newest
commit in master branch.

[1]: /messages/by-id/CAJDiXghoi-FM4d5XVZzUyiuhv8DDm9JdGOU8KC47emasqi1GUw@mail.gmail.com
[2]: /messages/by-id/CAJDiXgi9CWaZCVcHmvAT604RrAqDN5zpOYxZq92adqkPq5QbnQ@mail.gmail.com

--
Best regards,
Daniil Davydov

Hi Daniil,

On 7/29/25 11:35, Daniil Davydov wrote:

I attach a v7 patch to this letter. No changes yet, just rebased on the newest
commit in master branch.

A few days ago I reviewed one patch[1]/messages/by-id/2736425.1758475979@sss.pgh.pa.us that has a significant overlap
with this one. Perhaps they should be merged?

Here my first tests and comments:

== session 1 ==

$ /usr/local/postgres-dev/bin/psql postgres
psql (19devel)
Type "help" for help.

postgres=# CREATE TEMPORARY TABLE tmp AS SELECT 42 AS val;
SELECT 1
postgres=# \d tmp
Table "pg_temp_75.tmp"
Column | Type | Collation | Nullable | Default
--------+---------+-----------+----------+---------
val | integer | | |

== session 2 ==

$ /usr/local/postgres-dev/bin/psql postgres
psql (19devel)
Type "help" for help.

-- fixed: previously accessed the table but returning 0 rows
postgres=# SELECT * FROM pg_temp_75.tmp;
ERROR: could not access temporary relations of other sessions
LINE 1: SELECT * FROM pg_temp_75.tmp;
^
-- fixed: previously returning DELETE 0
postgres=# DELETE FROM pg_temp_75.tmp;
ERROR: could not access temporary relations of other sessions
LINE 1: DELETE FROM pg_temp_75.tmp;
^
postgres=# TRUNCATE TABLE pg_temp_75.tmp;
ERROR: could not access temporary relations of other sessions

-- fixed: previously returning UPDATE 0
postgres=# UPDATE pg_temp_75.tmp SET val = NULL;
ERROR: could not access temporary relations of other sessions
LINE 1: UPDATE pg_temp_75.tmp SET val = NULL;
^
-- error message changed: previously "ERROR: cannot access temporary
tables of other sessions"
postgres=# INSERT INTO pg_temp_75.tmp VALUES (73);
ERROR: could not access temporary relations of other sessions
LINE 1: INSERT INTO pg_temp_75.tmp VALUES (73);
^
-- fixed: previously returning COPY 0
postgres=# COPY pg_temp_75.tmp TO '/tmp/foo';
ERROR: could not access temporary relations of other sessions

-- error message changed. previously "ERROR: cannot alter temporary
tables of other sessions"
postgres=# ALTER TABLE pg_temp_75.tmp ADD COLUMN foo int;
ERROR: could not access temporary relations of other sessions

-- fixed: previously[2]ALTER TABLE ... RENAME TO tests in PostgreSQL 14.19: it was possible to rename the temp table.
postgres=# ALTER TABLE pg_temp_75.tmp RENAME TO bar;
ERROR: could not access temporary relations of other sessions

-- fixed: previously[3]LOCK TABLE tests in PostgreSQL 14.19 == session 2 == postgres=# BEGIN; BEGIN postgres=*# LOCK TABLE pg_temp_4.foo IN ACCESS EXCLUSIVE MODE; LOCK TABLE postgres=*# it was possible to LOCK the temp table.
postgres=# BEGIN;
BEGIN
postgres=*# LOCK TABLE pg_temp_75.tmp IN ACCESS EXCLUSIVE MODE;
ERROR: could not access temporary relations of other sessions

DROP TABLE still works, but I guess it is the main motivation of
RVR_OTHER_TEMP_OK :)

Thanks for the patch. It's a great improvement!

Best regards, Jim

[1]: /messages/by-id/2736425.1758475979@sss.pgh.pa.us
/messages/by-id/2736425.1758475979@sss.pgh.pa.us
[2]: ALTER TABLE ... RENAME TO tests in PostgreSQL 14.19:

== session 1 ==
psql (14.19 (Debian 14.19-1.pgdg13+1))
Geben Sie »help« für Hilfe ein.

postgres=# CREATE TEMPORARY TABLE tmp AS SELECT 42 AS val;
SELECT 1
postgres=# \d tmp
Tabelle »pg_temp_4.tmp«
Spalte | Typ | Sortierfolge | NULL erlaubt? | Vorgabewert
--------+---------+--------------+---------------+-------------
val | integer | | |

== session 2 ==
psql (14.19 (Debian 14.19-1.pgdg13+1))
Geben Sie »help« für Hilfe ein.

postgres=# ALTER TABLE pg_temp_4.tmp RENAME TO foo;
ALTER TABLE

== session 1 ==

postgres=# \d tmp
Keine Relation namens »tmp« gefunden
postgres=# \d foo
Tabelle »pg_temp_4.foo«
Spalte | Typ | Sortierfolge | NULL erlaubt? | Vorgabewert
--------+---------+--------------+---------------+-------------
val | integer | | |

[3]: LOCK TABLE tests in PostgreSQL 14.19 == session 2 == postgres=# BEGIN; BEGIN postgres=*# LOCK TABLE pg_temp_4.foo IN ACCESS EXCLUSIVE MODE; LOCK TABLE postgres=*#
== session 2 ==
postgres=# BEGIN;
BEGIN
postgres=*# LOCK TABLE pg_temp_4.foo IN ACCESS EXCLUSIVE MODE;
LOCK TABLE
postgres=*#

== session 1 ==
-- * owner of the temp table
postgres=# SELECT locktype, relation::regclass, mode, granted, pid
FROM pg_locks
WHERE relation = 'pg_temp_4.foo'::regclass::oid;
locktype | relation | mode | granted | pid
----------+----------+---------------------+---------+--------
relation | foo | AccessExclusiveLock | t | 277699
(1 Zeile)

Hi,

On Thu, Sep 25, 2025 at 5:45 PM Jim Jones <jim.jones@uni-muenster.de> wrote:

A few days ago I reviewed one patch[1] that has a significant overlap
with this one. Perhaps they should be merged?

Thanks for looking into it!
I don't know what exactly is meant by merging. Maybe we should just
apply a current
patch that fixes all problems ?..

Here my first tests and comments:
....

OK, I'll replace "could not" with "cannot" in order to match previous comments.

DROP TABLE still works, but I guess it is the main motivation of
RVR_OTHER_TEMP_OK :)

Yep, motivation of this decision you can find here [1]/messages/by-id/Zx7oLCnqis3FjgCK@paquier.xyz.

I'll attach a v8 patch that fixes error messages (could not -> cannot).

--
Best regards,
Daniil Davydov

[1]: /messages/by-id/Zx7oLCnqis3FjgCK@paquier.xyz

On 9/25/25 15:15, Daniil Davydov wrote:

I don't know what exactly is meant by merging. Maybe we should just
apply a current
patch that fixes all problems ?..

Here I just wanted to bring to your attention that we have duplicate
efforts with these two patches. This one covers much more ground though ;)

OK, I'll replace "could not" with "cannot" in order to match previous comments.

Small typo (you forgot to remove one "not")

errmsg("cannot not access temporary relations of other sessions")

It should be something like:

errmsg("cannot access temporary relations from other sessions")

Best regards, Jim

Hi,

On Thu, Sep 25, 2025 at 9:04 PM Jim Jones <jim.jones@uni-muenster.de> wrote:

Small typo (you forgot to remove one "not")

errmsg("cannot not access temporary relations of other sessions")

It should be something like:

errmsg("cannot access temporary relations from other sessions")

Oh, my bad. Fixed. Thanks for noticing it.

--
Best regards,
Daniil Davydov

The code LGTM (commit message still missing though)

Given that the lack of tests allowed this bug to go undetected until
now, I'd suggest to include additional tests in this patch to prevent
similar issues in the future. Something like 0002 attached. What do you
think?

Best, Jim

Hi,

On Fri, Sep 26, 2025 at 6:19 PM Jim Jones <jim.jones@uni-muenster.de> wrote:

Given that the lack of tests allowed this bug to go undetected until
now, I'd suggest to include additional tests in this patch to prevent
similar issues in the future. Something like 0002 attached. What do you
think?

Thanks for the test! Some time ago I wrote an isolation test [1]/messages/by-id/CAJDiXgi9CWaZCVcHmvAT604RrAqDN5zpOYxZq92adqkPq5QbnQ@mail.gmail.com for
this patch, but
it looked a bit ugly, so I decided to abandon it temporarily. Your
test looks much
better. I'd prefer to keep it.

[1]: /messages/by-id/CAJDiXgi9CWaZCVcHmvAT604RrAqDN5zpOYxZq92adqkPq5QbnQ@mail.gmail.com

--
Best regards,
Daniil Davydov

Hi,

I've rebased patches on the newest master.

--
Best regards,
Daniil Davydov

Hi,

I've rebased patches on the newest master.

My apologies - in previous letter I had attached the wrong files.
Thanks Jim Jones for noticing it :)

I am attaching the correct patches to this email.

--
Best regards,
Daniil Davydov

Hi,

Posting rebased set of patches.

--
Best regards,
Daniil Davydov

Hi all,
Thank you for the updated patches.

On Mon, Mar 23, 2026 at 12:56 PM Daniil Davydov <3danissimo@gmail.com> wrote:

Hi,

Posting rebased set of patches.

I have gone through the patches and tested the patch series.
I successfully reproduced the bug before applying the patch and found
that the cross-session SELECT via schema-qualified temp table did not
raise an error; instead, it returned an empty result, indicating
inconsistent behaviour. After applying the patch, regression tests
passed successfully and new regression tests behave as expected.
Temporary tables from other sessions are now no longer visible and all
the attempts to access them result in error. Also I verified:-
Schema-qualified SELECT, INSERT / UPDATE / DELETE, JOIN queries,
Subqueries and EXPLAIN worked as expected. Same-session access also
works as expected. Overall the patch LGTM.

Regards,
Soumya

On 23/03/2026 11:22, Soumya S Murali wrote:
Overall the patch LGTM.

This is a step forward in really isolating contents of temp tables from
other sessions, but the more I think about it, the more I'm concerned
with the current approach -- I spent some time investigating this
problem a bit deeper last week.

My main concern is the usage of gram.y, as a parser is arguably fragile
for this kind of things. For instance, one can always change the
search_path and bypass this restriction:

(table t was created in a different session)

postgres=# SELECT * FROM pg_temp_81.t;
ERROR: cannot access temporary relations of other sessions
LINE 1: SELECT * FROM pg_temp_81.t;
^
postgres=# SET search_path = pg_temp_81, public;
SET
postgres=# SELECT * FROM t;
?column?
----------
(0 rows)

* See: if (relation->relpersistence == RELPERSISTENCE_TEMP) in
namespace.c for more details.

IMO, since it is an access control issue, I guess we better treat it as
such and modify aclchk.c instead.

Something like this the file attached. This breaks an unrelated test,
which is potentially a bug in REPACK ... but I'll describe it in another
thread.

Thoughts?

Best, Jim

Jim Jones <jim.jones@uni-muenster.de> writes:

This is a step forward in really isolating contents of temp tables from
other sessions, but the more I think about it, the more I'm concerned
with the current approach -- I spent some time investigating this
problem a bit deeper last week.

Yeah. I think this entire approach is wrongheaded: we do not enforce
permissions checks against superusers. Moreover, if we try to fix it
at the permissions level, it seems nearly certain that there will be
bypass paths, simply because superusers bypass so many other checks.

The actual problem is that the buffer manager is incapable of dealing
with other sessions' temp tables, and we need to un-break the buffer
manager's defense for that implementation restriction. So I feel the
correct approach is something similar to what I described here:

/messages/by-id/2736425.1758475979@sss.pgh.pa.us

I'm not wedded to that specific patch, but that is the implementation
level where the fix is needed.

regards, tom lane

Hi,

On Mon, Mar 23, 2026 at 8:31 PM Jim Jones <jim.jones@uni-muenster.de> wrote:

This is a step forward in really isolating contents of temp tables from
other sessions, but the more I think about it, the more I'm concerned
with the current approach -- I spent some time investigating this
problem a bit deeper last week.

My main concern is the usage of gram.y, as a parser is arguably fragile
for this kind of things.

I don't actually want to use gram.y as a main solver of this issue. But
gram.y is setting the "relpersistence" field for the RangeVar and all
subsequent code is treating this value as truthful. In particular, v12 patch
is relying on this field in RangeVar during the resolution of access issues,
and IMHO it's not out of line with the current code base. For instance, we are
considering RangeVar to determine whether we can perform a CREATE operation
within the specified namespace (see RangeVarAdjustRelationPersistence).

Even if we decide not to touch the gram.y in this patch, I still think that
leaving the "relpesistence" field misleading may lead to more bugs appearing
in the future. I.e. it should be fixed anyway (maybe in another thread?).

For instance, one can always change the
search_path and bypass this restriction:

(table t was created in a different session)

postgres=# SELECT * FROM pg_temp_81.t;
ERROR: cannot access temporary relations of other sessions
LINE 1: SELECT * FROM pg_temp_81.t;
^
postgres=# SET search_path = pg_temp_81, public;
SET
postgres=# SELECT * FROM t;
?column?
----------
(0 rows)

* See: if (relation->relpersistence == RELPERSISTENCE_TEMP) in
namespace.c for more details.

Yeah, you are right. It turns out that the current patch doesn't fully
protect other temp tables from superuser. The first thought that comes to me
is to forbid setting other-temp-namespaces in a search_path parameter. I know
it's starting to look ugly. But actually, such a restriction seems quite
logical.

IMO, since it is an access control issue, I guess we better treat it as
such and modify aclchk.c instead.

Something like this the file attached. This breaks an unrelated test,
which is potentially a bug in REPACK ... but I'll describe it in another
thread.

Thoughts?

Thank you for the patch! It seems much more beautiful and convenient to
maintain, but I have a little concern about it.

Actually, in your implementation we can DROP other temp tables not because we
make an exception to the general rule ("don't touch other temp tables"), but
because we just can perform such operations with every object in pg_temp_0.
If other operations with the same access checking as DROP command appear in
the future, the user will be able to perform these operations for
other-temp-tables. I.e. we will need to manually prevent such new operations
from accessing other-temp-tables. Have I gone too far in my reasoning?

BTW, your implementation allows calling a VACUUM for other-temp-tables. I think
that we should forbid that too.

On Mon, Mar 23, 2026 at 8:44 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:

Jim Jones <jim.jones@uni-muenster.de> writes:

This is a step forward in really isolating contents of temp tables from
other sessions, but the more I think about it, the more I'm concerned
with the current approach -- I spent some time investigating this
problem a bit deeper last week.

Yeah. I think this entire approach is wrongheaded: we do not enforce
permissions checks against superusers. Moreover, if we try to fix it
at the permissions level, it seems nearly certain that there will be
bypass paths, simply because superusers bypass so many other checks.

Actually, v12 patch is not about a superuser rights restriction, but about
forbidding such operations for everyone. Anyway, we have a new perl test that
will prevent adding a code that will allow superuser to (somehow) break the
protection of both mine and Jim's patches. Isn't that a sufficient guarantee
that superuser will not bypass checks that it must not bypass?

The actual problem is that the buffer manager is incapable of dealing
with other sessions' temp tables, and we need to un-break the buffer
manager's defense for that implementation restriction. So I feel the
correct approach is something similar to what I described here:

/messages/by-id/2736425.1758475979@sss.pgh.pa.us

I'm not wedded to that specific patch, but that is the implementation
level where the fix is needed.

Handling access to other-temp-tables on the buffer manager level seems to me
like fighting the symptom, not the cause. Protection of other-temp-tables is
kinda "upper-level logical restriction". At the same time, buffer manager is a
lower-level implementation which shouldn't face such upper-level issues.
Am I missing something?

--
Best regards,
Daniil Davydov

Daniil Davydov <3danissimo@gmail.com> writes:

On Mon, Mar 23, 2026 at 8:44 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:

Yeah. I think this entire approach is wrongheaded: we do not enforce
permissions checks against superusers. Moreover, if we try to fix it
at the permissions level, it seems nearly certain that there will be
bypass paths, simply because superusers bypass so many other checks.

Actually, v12 patch is not about a superuser rights restriction, but about
forbidding such operations for everyone.

... including superusers, who bypass permissions restrictions
everywhere else. You are going to have to contort the ACL system
badly to make that happen at all, and I would not be surprised
if you introduce new bugs.

The actual problem is that the buffer manager is incapable of dealing
with other sessions' temp tables, and we need to un-break the buffer
manager's defense for that implementation restriction. So I feel the
correct approach is something similar to what I described here:
/messages/by-id/2736425.1758475979@sss.pgh.pa.us

Handling access to other-temp-tables on the buffer manager level seems to me
like fighting the symptom, not the cause.

No, it IS the cause. If someday someone were to reimplement buffer
management in a way that didn't have this implementation restriction,
we would surely not arbitrarily restrict superusers from looking at
tables that they then would physically be able to look at.

Am I missing something?

Mainly, that we had a setup that was working fine for decades,
until somebody made holes in it with careless refactoring.
We should fix that mistake, not introduce inconsistent-with-
decades-of-practice permissions behavior to hide the mistake
at an unrelated logical level.

Also, we need a defense at the buffer manager level anyway, because
otherwise C code could try to access another session's temp table
and we'd not realize it was getting bogus answers. (Whether such
an attempt is a bug or not is a different discussion; but we at
least need some logic that detects that it won't work, and the ACL
system cannot be expected to stop C-level code from trying.)

Also, we really need a patch that's simple and non-invasive enough
to be back-patched into v17 and v18. This proposal is not that.

I don't actually want to use gram.y as a main solver of this issue. But
gram.y is setting the "relpersistence" field for the RangeVar and all
subsequent code is treating this value as truthful.

I do kind of agree with this concern, but the v12 patch simply moves
the untruthfulness around. Reality is that we cannot know whether an
unqualified-name RangeVar references a temp table until we do a
catalog lookup, so IMO we should not have a relpersistence field there
at all. At best it means something quite different from what it means
elsewhere, and that's a recipe for confusion. But changing that would
not be a bug fix (AFAIK) but refactoring to reduce the probability of
future bugs.

regards, tom lane

Hi,

On Wed, Mar 25, 2026 at 1:58 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:

Actually, v12 patch is not about a superuser rights restriction, but about
forbidding such operations for everyone.

... including superusers, who bypass permissions restrictions
everywhere else. You are going to have to contort the ACL system
badly to make that happen at all, and I would not be surprised
if you introduce new bugs.

I've never dealt with the ACL system before, so it's hard for me to assess
the scale of the problem. I am inclined to believe you on this issue.

The actual problem is that the buffer manager is incapable of dealing
with other sessions' temp tables, and we need to un-break the buffer
manager's defense for that implementation restriction. So I feel the
correct approach is something similar to what I described here:
/messages/by-id/2736425.1758475979@sss.pgh.pa.us

Handling access to other-temp-tables on the buffer manager level seems to me
like fighting the symptom, not the cause.

No, it IS the cause. If someday someone were to reimplement buffer
management in a way that didn't have this implementation restriction,
we would surely not arbitrarily restrict superusers from looking at
tables that they then would physically be able to look at.

OK, now I fully understand your point (I hope so) :
We want to restrict backend access to other-temp-tables just because it is
physically impossible for them to read the pages of such tables. And if some
users has enough privileges, it is OK for us to allow them to
lock/vacuum/drop/... other-temp-tables. I.e. only operations with heap pages
access must be forbidden, and the buffer manager layer is an appropriate place
for it.

Am I missing something?

Mainly, that we had a setup that was working fine for decades,
until somebody made holes in it with careless refactoring.
We should fix that mistake, not introduce inconsistent-with-
decades-of-practice permissions behavior to hide the mistake
at an unrelated logical level.

Yeah, we have a few checks in the bufmgr (PrefetchBuffer, ReadBufferExtended),
but they stopped coping with their task.

Also, we need a defense at the buffer manager level anyway, because
otherwise C code could try to access another session's temp table
and we'd not realize it was getting bogus answers. (Whether such
an attempt is a bug or not is a different discussion; but we at
least need some logic that detects that it won't work, and the ACL
system cannot be expected to stop C-level code from trying.)

Also, we really need a patch that's simple and non-invasive enough
to be back-patched into v17 and v18. This proposal is not that.

OK

I don't actually want to use gram.y as a main solver of this issue. But
gram.y is setting the "relpersistence" field for the RangeVar and all
subsequent code is treating this value as truthful.

I do kind of agree with this concern, but the v12 patch simply moves
the untruthfulness around. Reality is that we cannot know whether an
unqualified-name RangeVar references a temp table until we do a
catalog lookup, ...

Yep, Jim's example shows us that we cannot always rely on the "relpersistence"
field.

...so IMO we should not have a relpersistence field there
at all. At best it means something quite different from what it means
elsewhere, and that's a recipe for confusion. But changing that would
not be a bug fix (AFAIK) but refactoring to reduce the probability of
future bugs.

I agree with the idea to get rid of this field. By now I cannot say for sure
whether we can fix a bug without modifying the RangeVar structure. But I'll
try to implement proposed logic only within the bufmgr.

Thank you very much for your comments! I'll post a new patch in the near
future.

--
Best regards,
Daniil Davydov

Hi all,

On Wed, Mar 25, 2026 at 12:38 PM Daniil Davydov <3danissimo@gmail.com> wrote:

Hi,

On Wed, Mar 25, 2026 at 1:58 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:

Actually, v12 patch is not about a superuser rights restriction, but about
forbidding such operations for everyone.

... including superusers, who bypass permissions restrictions
everywhere else. You are going to have to contort the ACL system
badly to make that happen at all, and I would not be surprised
if you introduce new bugs.

I've never dealt with the ACL system before, so it's hard for me to assess
the scale of the problem. I am inclined to believe you on this issue.

The actual problem is that the buffer manager is incapable of dealing
with other sessions' temp tables, and we need to un-break the buffer
manager's defense for that implementation restriction. So I feel the
correct approach is something similar to what I described here:
/messages/by-id/2736425.1758475979@sss.pgh.pa.us

Handling access to other-temp-tables on the buffer manager level seems to me
like fighting the symptom, not the cause.

No, it IS the cause. If someday someone were to reimplement buffer
management in a way that didn't have this implementation restriction,
we would surely not arbitrarily restrict superusers from looking at
tables that they then would physically be able to look at.

OK, now I fully understand your point (I hope so) :
We want to restrict backend access to other-temp-tables just because it is
physically impossible for them to read the pages of such tables. And if some
users has enough privileges, it is OK for us to allow them to
lock/vacuum/drop/... other-temp-tables. I.e. only operations with heap pages
access must be forbidden, and the buffer manager layer is an appropriate place
for it.

Am I missing something?

Mainly, that we had a setup that was working fine for decades,
until somebody made holes in it with careless refactoring.
We should fix that mistake, not introduce inconsistent-with-
decades-of-practice permissions behavior to hide the mistake
at an unrelated logical level.

Yeah, we have a few checks in the bufmgr (PrefetchBuffer, ReadBufferExtended),
but they stopped coping with their task.

Also, we need a defense at the buffer manager level anyway, because
otherwise C code could try to access another session's temp table
and we'd not realize it was getting bogus answers. (Whether such
an attempt is a bug or not is a different discussion; but we at
least need some logic that detects that it won't work, and the ACL
system cannot be expected to stop C-level code from trying.)

Also, we really need a patch that's simple and non-invasive enough
to be back-patched into v17 and v18. This proposal is not that.

OK

I don't actually want to use gram.y as a main solver of this issue. But
gram.y is setting the "relpersistence" field for the RangeVar and all
subsequent code is treating this value as truthful.

I do kind of agree with this concern, but the v12 patch simply moves
the untruthfulness around. Reality is that we cannot know whether an
unqualified-name RangeVar references a temp table until we do a
catalog lookup, ...

Yep, Jim's example shows us that we cannot always rely on the "relpersistence"
field.

...so IMO we should not have a relpersistence field there
at all. At best it means something quite different from what it means
elsewhere, and that's a recipe for confusion. But changing that would
not be a bug fix (AFAIK) but refactoring to reduce the probability of
future bugs.

I agree with the idea to get rid of this field. By now I cannot say for sure
whether we can fix a bug without modifying the RangeVar structure. But I'll
try to implement proposed logic only within the bufmgr.

Thank you very much for your comments! I'll post a new patch in the near
future.

I worked on the issue of accessing temporary tables belonging to other
sessions and tried implementing the fix at the buffer manager level,
as suggested. I added checks in ReadBuffer_common() and
PrefetchBuffer() to reject access when a relation is temporary
(relpersistence = TEMP) but does not use local buffers
(!RelationUsesLocalBuffers) so that it ensures only heap page access
is blocked, while catalog lookups and other metadata operations
continue to work as before. While testing, I observed that in many
cases the query does not reach the buffer manager because name
resolution fails earlier with “relation does not exist”. However, the
added checks ensure that even if execution reaches the buffer layer,
access to other sessions’ temporary tables is safely rejected. The
change is minimal, and did not modify parser/ACL behavior and all
regression tests got passed successfully too.
Kindly review the attached patch herewith. Please let me know if this
approach aligns with expectations or if further adjustments are
needed.

Regards,
Soumya

Hi

On 08/04/2026 11:17, Soumya S Murali wrote:

I worked on the issue of accessing temporary tables belonging to other
sessions and tried implementing the fix at the buffer manager level,
as suggested. I added checks in ReadBuffer_common() and
PrefetchBuffer() to reject access when a relation is temporary
(relpersistence = TEMP) but does not use local buffers
(!RelationUsesLocalBuffers) so that it ensures only heap page access
is blocked, while catalog lookups and other metadata operations
continue to work as before. While testing, I observed that in many
cases the query does not reach the buffer manager because name
resolution fails earlier with “relation does not exist”. However, the
added checks ensure that even if execution reaches the buffer layer,
access to other sessions’ temporary tables is safely rejected. The
change is minimal, and did not modify parser/ACL behavior and all
regression tests got passed successfully too.
Kindly review the attached patch herewith. Please let me know if this
approach aligns with expectations or if further adjustments are
needed.

A few comments:

== PrefetchBuffer ==

The condition nested inside the if (RelationUsesLocalBuffers(reln))
tests the opposite of the main if !RelationUsesLocalBuffers(reln):

if (RelationUsesLocalBuffers(reln))
{
/* ACCESS DENIED CHECK */
if (reln != NULL &&
reln->rd_rel != NULL &&
reln->rd_rel->relpersistence == RELPERSISTENCE_TEMP &&
!RelationUsesLocalBuffers(reln))
{
ereport(ERROR,
(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
errmsg("cannot access temporary tables of other sessions")));
}
...
}

So it'll be always false, making the ereport unreachable.

== ReadBufferExtended ==

These conditions cancel each other out:

if (reln->rd_rel->relpersistence == RELPERSISTENCE_TEMP &&
!RelationUsesLocalBuffers(reln))

RelationUsesLocalBuffers(reln) expands to
((relation)->rd_rel->relpersistence == RELPERSISTENCE_TEMP), making the
error message unreachable. Perhaps you meant RELATION_IS_OTHER_TEMP?

== ReadBuffer_common ==

Same as in ReadBufferExtended and PrefetchBuffer.

== tests ==

You excluded the tests from the patch.

== patch version ==

You forgot to add the patch version.

Best, Jim