Обсуждение: We broke the defense against accessing other sessions' temp tables

Поиск
Список
Период
Сортировка

We broke the defense against accessing other sessions' temp tables

От
Tom Lane
Дата:
In session 1:

regression=# create temp table tt(f1 int);
CREATE TABLE
regression=# insert into tt values(44);
INSERT 0 1
regression=# \d tt
               Table "pg_temp_77.tt"
 Column |  Type   | Collation | Nullable | Default 
--------+---------+-----------+----------+---------
 f1     | integer |           |          | 

In session 2:

regression=# select * from pg_temp_77.tt; -- use the schema shown in session 1
 f1 
----
(0 rows)

Branches before 17 show the expected failure

ERROR:  cannot access temporary tables of other sessions

but that's gone missing :-(.  Apparently, we refactored things enough
that ReadBufferExtended is not used for a simple seqscan, and since
that's where the check for an other-session temp table is, we get
no error and instead bogus results.  I did not bother to bisect to
find a culprit commit, but given that the failure manifests in 17,
I'm betting the stream I/O stuff is at fault.

I experimented with moving the check into PinBufferForBlock, and
that seems to work correctly, but I wonder if someone who's messed
with this code more recently than I would prefer a different
solution.

            regards, tom lane

diff --git a/src/backend/storage/buffer/bufmgr.c b/src/backend/storage/buffer/bufmgr.c
index fe470de63f2..e27d74a24c2 100644
--- a/src/backend/storage/buffer/bufmgr.c
+++ b/src/backend/storage/buffer/bufmgr.c
@@ -655,7 +655,7 @@ PrefetchBuffer(Relation reln, ForkNumber forkNum, BlockNumber blockNum)

     if (RelationUsesLocalBuffers(reln))
     {
-        /* see comments in ReadBufferExtended */
+        /* see comments in PinBufferForBlock */
         if (RELATION_IS_OTHER_TEMP(reln))
             ereport(ERROR,
                     (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
@@ -807,16 +807,6 @@ ReadBufferExtended(Relation reln, ForkNumber forkNum, BlockNumber blockNum,
 {
     Buffer        buf;

-    /*
-     * Reject attempts to read non-local temporary relations; we would be
-     * likely to get wrong data since we have no visibility into the owning
-     * session's local buffers.
-     */
-    if (RELATION_IS_OTHER_TEMP(reln))
-        ereport(ERROR,
-                (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
-                 errmsg("cannot access temporary tables of other sessions")));
-
     /*
      * Read the buffer, and update pgstat counters to reflect a cache hit or
      * miss.
@@ -1128,6 +1118,18 @@ PinBufferForBlock(Relation rel,

     if (persistence == RELPERSISTENCE_TEMP)
     {
+        /*
+         * Reject attempts to read non-local temporary relations; we would be
+         * likely to get wrong data since we have no visibility into the
+         * owning session's local buffers.  We don't expect to get here
+         * without a relcache entry (see ReadBufferWithoutRelcache).
+         */
+        Assert(rel);
+        if (RELATION_IS_OTHER_TEMP(rel))
+            ereport(ERROR,
+                    (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+                     errmsg("cannot access temporary tables of other sessions")));
+
         io_context = IOCONTEXT_NORMAL;
         io_object = IOOBJECT_TEMP_RELATION;
     }

Re: We broke the defense against accessing other sessions' temp tables

От
Bruce Momjian
Дата:
On Sun, Sep 21, 2025 at 01:32:59PM -0400, Tom Lane wrote:
> but that's gone missing :-(.  Apparently, we refactored things enough
> that ReadBufferExtended is not used for a simple seqscan, and since
> that's where the check for an other-session temp table is, we get
> no error and instead bogus results.  I did not bother to bisect to
> find a culprit commit, but given that the failure manifests in 17,
> I'm betting the stream I/O stuff is at fault.
> 
> I experimented with moving the check into PinBufferForBlock, and
> that seems to work correctly, but I wonder if someone who's messed
> with this code more recently than I would prefer a different
> solution.

Given that this problem exists in PG 17, I assume this will not affect
the PG 18 release this week.

-- 
  Bruce Momjian  <bruce@momjian.us>        https://momjian.us
  EDB                                      https://enterprisedb.com

  Do not let urgent matters crowd out time for investment in the future.



Re: We broke the defense against accessing other sessions' temp tables

От
Tom Lane
Дата:
Bruce Momjian <bruce@momjian.us> writes:
> Given that this problem exists in PG 17, I assume this will not affect
> the PG 18 release this week.

Yeah, if it were 18-only we'd need a powwow about whether to break
release freeze.  But since it's been there since 17 and nobody
noticed, I'm content to wait till after 18.0 to fix it.

I believe you need to be superuser to reach the bug at all, otherwise
permissions checks should stop you earlier.  This makes it less urgent
than it would otherwise be.

            regards, tom lane



Re: We broke the defense against accessing other sessions' temp tables

От
Bruce Momjian
Дата:
On Sun, Sep 21, 2025 at 01:44:11PM -0400, Tom Lane wrote:
> Bruce Momjian <bruce@momjian.us> writes:
> > Given that this problem exists in PG 17, I assume this will not affect
> > the PG 18 release this week.
> 
> Yeah, if it were 18-only we'd need a powwow about whether to break
> release freeze.  But since it's been there since 17 and nobody
> noticed, I'm content to wait till after 18.0 to fix it.
> 
> I believe you need to be superuser to reach the bug at all, otherwise
> permissions checks should stop you earlier.  This makes it less urgent
> than it would otherwise be.

Oh, good to know.

-- 
  Bruce Momjian  <bruce@momjian.us>        https://momjian.us
  EDB                                      https://enterprisedb.com

  Do not let urgent matters crowd out time for investment in the future.



Re: We broke the defense against accessing other sessions' temp tables

От
Jelte Fennema-Nio
Дата:
On Sun, 21 Sept 2025 at 19:44, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Yeah, if it were 18-only we'd need a powwow about whether to break
> release freeze.  But since it's been there since 17 and nobody
> noticed, I'm content to wait till after 18.0 to fix it.

People definitely noticed[1], but I agree that there's no reason to
rush this fix out before the 18 release.

Patch looks good, but obviously needs some tests.

[1]: https://www.postgresql.org/message-id/flat/CAMEv5_syU0ZopE-2Wr8A8QksqrCyYT2hW06Rgw4RSPdyJO-%3Dfw%40mail.gmail.com



Re: We broke the defense against accessing other sessions' temp tables

От
"David G. Johnston"
Дата:
On Sun, Sep 21, 2025 at 12:19 PM Jelte Fennema-Nio <postgres@jeltef.nl> wrote:
On Sun, 21 Sept 2025 at 19:44, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Yeah, if it were 18-only we'd need a powwow about whether to break
> release freeze.  But since it's been there since 17 and nobody
> noticed, I'm content to wait till after 18.0 to fix it.

People definitely noticed[1], but I agree that there's no reason to
rush this fix out before the 18 release.

Patch looks good, but obviously needs some tests.

[1]: https://www.postgresql.org/message-id/flat/CAMEv5_syU0ZopE-2Wr8A8QksqrCyYT2hW06Rgw4RSPdyJO-%3Dfw%40mail.gmail.com


I'm presuming this report is also about the same underlying issue.


It too has a patch included.

David J.

Re: We broke the defense against accessing other sessions' temp tables

От
Jim Jones
Дата:
Hi

On 9/21/25 21:39, David G. Johnston wrote:
> On Sun, Sep 21, 2025 at 12:19 PM Jelte Fennema-Nio <postgres@jeltef.nl
> <mailto:postgres@jeltef.nl>> wrote:
> 
>     On Sun, 21 Sept 2025 at 19:44, Tom Lane <tgl@sss.pgh.pa.us
>     <mailto:tgl@sss.pgh.pa.us>> wrote:
>     > Yeah, if it were 18-only we'd need a powwow about whether to break
>     > release freeze.  But since it's been there since 17 and nobody
>     > noticed, I'm content to wait till after 18.0 to fix it.
> 
>     People definitely noticed[1], but I agree that there's no reason to
>     rush this fix out before the 18 release.
> 
>     Patch looks good, but obviously needs some tests.

Here a few tests:

== 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_81.tmp"
 Column |  Type   | Collation | Nullable | Default
--------+---------+-----------+----------+---------
 val    | integer |           |          |


== session 2 ==

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

-- previously returned O rows
postgres=# SELECT * FROM pg_temp_81.tmp;
ERROR:  cannot access temporary tables of other sessions
-- previously returned DELETE 0
postgres=# DELETE FROM pg_temp_81.tmp;
ERROR:  cannot access temporary tables of other sessions
postgres=# TRUNCATE TABLE pg_temp_81.tmp;
ERROR:  cannot truncate temporary tables of other sessions
-- previously returned UPDATE 0
postgres=# UPDATE pg_temp_81.tmp SET val = NULL;
ERROR:  cannot access temporary tables of other sessions
postgres=# INSERT INTO pg_temp_81.tmp VALUES (0);
ERROR:  cannot access temporary tables of other sessions
postgres=# COPY pg_temp_81.tmp TO '/tmp/x';
ERROR:  cannot access temporary tables of other sessions
postgres=# ALTER TABLE pg_temp_81.tmp ADD COLUMN foo text;
ERROR:  cannot alter temporary tables of other sessions

Is ALTER TABLE ... RENAME a loophole? I tested this in PostgreSQL 15.14
and the result was the same:

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

== session 1 ==

postgres=# \d tmp
Did not find any relation named "tmp".
postgres=# \d foo
              Table "pg_temp_81.foo"
 Column |  Type   | Collation | Nullable | Default
--------+---------+-----------+----------+---------
 val    | integer |           |          |


I also noticed that it is possible to LOCK a temp table from another
session (as superuser).

== 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_86.tmp"
 Column |  Type   | Collation | Nullable | Default
--------+---------+-----------+----------+---------
 val    | integer |           |          |


== session 2 ==

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

postgres=# BEGIN;
BEGIN
postgres=*# LOCK TABLE pg_temp_86.tmp IN ACCESS EXCLUSIVE MODE;
LOCK TABLE
postgres=*#

Doesn't it mean that one session can trivially DoS another session's
private temp table?

== session 1 ==

postgres=# SELECT locktype, relation::regclass, mode, granted, pid
FROM pg_locks
WHERE relation = 'pg_temp_86.tmp'::regclass::oid;
 locktype | relation |        mode         | granted |   pid
----------+----------+---------------------+---------+---------
 relation | tmp      | AccessExclusiveLock | t       | 3286840
(1 row)

postgres=# SELECT * FROM tmp;
^CCancel request sent
ERROR:  canceling statement due to user request
CONTEXT:  waiting for AccessShareLock on relation 47632 of database 5
postgres=#


I didn't test all possible LOCK modes, but I suspect they all work.

== session 2 ==

postgres=# BEGIN;
BEGIN
postgres=*# LOCK TABLE pg_temp_86.tmp IN SHARE MODE;
LOCK TABLE
postgres=*#

== session 1 ==

postgres=# SELECT locktype, relation::regclass, mode, granted, pid
FROM pg_locks
WHERE relation = 'pg_temp_86.tmp'::regclass::oid;
 locktype | relation |   mode    | granted |   pid
----------+----------+-----------+---------+---------
 relation | tmp      | ShareLock | t       | 3289767
(1 row)

postgres=# SELECT * FROM tmp;
 val
-----
  42
(1 row)

As expected with non-superusers it returns a permission denied for the
temp schema:

== session 2 ==

postgres=# CREATE USER u1;
CREATE ROLE
postgres=# SET ROLE u1;
SET
postgres=> BEGIN;
BEGIN
postgres=*> LOCK TABLE pg_temp_86.tmp IN ACCESS EXCLUSIVE MODE;
ERROR:  permission denied for schema pg_temp_86
postgres=!>



Best regards, Jim



Re: We broke the defense against accessing other sessions' temp tables

От
Jim Jones
Дата:

On 9/22/25 10:14, Jim Jones wrote:
> I also noticed that it is possible to LOCK a temp table from another
> session (as superuser).

It gets even stranger if the owner's session closes while there is a
LOCK in a different session:

== session 1 ==

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

postgres=# SELECT pg_backend_pid();
 pg_backend_pid
----------------
        3520132
(1 row)

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


== session 2 ==

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

postgres=# BEGIN;
BEGIN
postgres=*# LOCK TABLE pg_temp_6.foo IN ACCESS EXCLUSIVE MODE;
LOCK TABLE
postgres=*# SELECT pg_backend_pid();
 pg_backend_pid
----------------
        3520608
(1 row)

== session 1 ==

postgres=# SELECT locktype, relation::regclass, mode, granted, pid
FROM pg_locks
WHERE relation = 'pg_temp_6.foo'::regclass::oid;
 locktype | relation |        mode         | granted |   pid
----------+----------+---------------------+---------+---------
 relation | foo      | AccessExclusiveLock | t       | 3520608
(1 row)

postgres=# \q

== session 3 ==

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

postgres=# SELECT pg_backend_pid();
 pg_backend_pid
----------------
        3521711
(1 row)

postgres=# SELECT locktype, relation::regclass, mode, granted, pid
FROM pg_locks
WHERE relation = 'pg_temp_6.foo'::regclass::oid;
 locktype |   relation    |        mode         | granted |   pid
----------+---------------+---------------------+---------+---------
 relation | pg_temp_6.foo | AccessExclusiveLock | t       | 3520608
 relation | pg_temp_6.foo | AccessExclusiveLock | f       | 3520132
(2 rows)

== session 4 ==

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

postgres=# SELECT pid, wait_event_type, wait_event, state
FROM pg_stat_activity
WHERE pid = 3520132;
   pid   | wait_event_type | wait_event | state
---------+-----------------+------------+-------
 3520132 | Lock            | relation   | idle
(1 row)


pid 3520132 lives as long as session 2 holds a lock on pg_temp_6.foo.


Best, Jim