From: Alexander Korotkov Date: Thu, 14 May 2026 09:25:19 +0000 (+0300) Subject: Prevent access to other sessions' temp tables X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=ce146621f7860d2e19c509f1466feca3bf777678;p=thirdparty%2Fpostgresql.git Prevent access to other sessions' temp tables Commit b7b0f3f2724 ("Use streaming I/O in sequential scans") routed sequential scans through read_stream_next_buffer(), bypassing the RELATION_IS_OTHER_TEMP() check in ReadBufferExtended(). As a result, a superuser can attempt to read or modify temp tables of other sessions through the read-stream path. When the query plan uses no index, SELECT/UPDATE/DELETE/MERGE silently see no rows / report zero affected rows, and COPY produces an empty output -- because the buffer manager has no visibility into the owning session's local buffers and silently returns nothing. Any query plan that uses, for instance, a btree index still errors out via the existing check in ReadBufferExtended(), which is reached from hio.c and nbtree respectively, but this is incidental. Fix by enforcing RELATION_IS_OTHER_TEMP() at the three additional buffer-manager entry points: - read_stream_begin_impl() rejects the read at stream setup time, covering sequential and bitmap scans that go through the read-stream path. - ReadBuffer_common() becomes the canonical place for the check, consolidating the existing one previously kept in ReadBufferExtended(). All ReadBufferExtended() callers go through ReadBuffer_common(), so the consolidation is behavior-preserving. - StartReadBuffersImpl() catches direct callers of StartReadBuffers() that bypass both of the above. This is currently defense-in-depth, but documents the contract for future code. The companion test in src/test/modules/test_misc was added in the preceding commit; this commit updates the assertions for SELECT, UPDATE, DELETE, MERGE, and COPY (which previously documented the bug as silent success) to expect the new error. Author: Jim Jones Author: Daniil Davydov <3danissimo@gmail.com> Co-authored-by: Alexander Korotkov Reviewed-by: Michael Paquier Reviewed-by: Soumya S Murali Reviewed-by: Tom Lane Discussion: https://postgr.es/m/CAJDiXghdFcZ8%3Dnh4G69te7iRr3Q0uFyXxb3ZdG09_GTNZXwH0g%40mail.gmail.com Backpatch-through: 17 --- diff --git a/src/backend/storage/aio/read_stream.c b/src/backend/storage/aio/read_stream.c index 2374b4cd507..a318539e56c 100644 --- a/src/backend/storage/aio/read_stream.c +++ b/src/backend/storage/aio/read_stream.c @@ -776,6 +776,16 @@ read_stream_begin_impl(int flags, uint32 max_possible_buffer_limit; Oid tablespace_id; + /* + * 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 (rel && RELATION_IS_OTHER_TEMP(rel)) + ereport(ERROR, + (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), + errmsg("cannot access temporary tables of other sessions"))); + /* * Decide how many I/Os we will allow to run at the same time. This * number also affects how far we look ahead for opportunities to start diff --git a/src/backend/storage/buffer/bufmgr.c b/src/backend/storage/buffer/bufmgr.c index 1878efb4aa9..cc398db124d 100644 --- a/src/backend/storage/buffer/bufmgr.c +++ b/src/backend/storage/buffer/bufmgr.c @@ -791,7 +791,7 @@ PrefetchBuffer(Relation reln, ForkNumber forkNum, BlockNumber blockNum) if (RelationUsesLocalBuffers(reln)) { - /* see comments in ReadBufferExtended */ + /* see comments in ReadBuffer_common */ if (RELATION_IS_OTHER_TEMP(reln)) ereport(ERROR, (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), @@ -928,19 +928,10 @@ 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. + * miss. The other-session temp-relation check is enforced by + * ReadBuffer_common(). */ buf = ReadBuffer_common(reln, RelationGetSmgr(reln), 0, forkNum, blockNum, mode, strategy); @@ -1292,6 +1283,18 @@ ReadBuffer_common(Relation rel, SMgrRelation smgr, char smgr_persistence, int flags; char persistence; + /* + * 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. This is the canonical place for the check, + * covering the ReadBufferExtended() entry point and any other caller that + * supplies a Relation. + */ + if (rel && RELATION_IS_OTHER_TEMP(rel)) + ereport(ERROR, + (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), + errmsg("cannot access temporary tables of other sessions"))); + /* * Backward compatibility path, most code should use ExtendBufferedRel() * instead, as acquiring the extension lock inside ExtendBufferedRel() @@ -1382,6 +1385,12 @@ StartReadBuffersImpl(ReadBuffersOperation *operation, Assert(*nblocks > 0); Assert(*nblocks <= MAX_IO_COMBINE_LIMIT); + /* see comments in ReadBuffer_common */ + if (operation->rel && RELATION_IS_OTHER_TEMP(operation->rel)) + ereport(ERROR, + (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), + errmsg("cannot access temporary tables of other sessions"))); + if (operation->persistence == RELPERSISTENCE_TEMP) { io_context = IOCONTEXT_NORMAL; diff --git a/src/include/utils/rel.h b/src/include/utils/rel.h index e2e389d544f..fa07ebf8ff7 100644 --- a/src/include/utils/rel.h +++ b/src/include/utils/rel.h @@ -666,17 +666,12 @@ RelationCloseSmgr(Relation relation) * * Reading another session's temp-table data through never works right: * the owning session keeps the data in its private local buffer pool, - * which we cannot access. The macro is therefore used at the buffer-manager - * level to reject such accesses, and by command-level code (TRUNCATE, - * ALTER TABLE, VACUUM, CLUSTER, REINDEX, ...) for command-specific error - * messages. - * - * Currenlty buffer manager checks include only ReadBufferExtended(), and - * PrefetchBuffer(); while ReadBuffer_common(), read_stream_begin_impl(), and - * StartReadBuffersImpl() are not covered. As a result, read paths that - * bypass ReadBufferExtended() -- notably sequential scans that go through - * the read-stream API -- silently return no rows when targeted at another - * session's temp table instead of failing. + * which we cannot access. Existing buffer-manager entry points + * (ReadBuffer_common(), StartReadBuffersImpl(), read_stream_begin_impl(), + * and PrefetchBuffer()) already enforce this; any new buffer-access entry + * points must do the same. Command-level code (TRUNCATE, ALTER TABLE, + * VACUUM, CLUSTER, REINDEX, ...) additionally uses this macro for + * command-specific error messages. * * Beware of multiple eval of argument */ diff --git a/src/test/modules/test_misc/t/013_temp_obj_multisession.pl b/src/test/modules/test_misc/t/013_temp_obj_multisession.pl index bfcba42acd0..5f3cc7d2fc5 100644 --- a/src/test/modules/test_misc/t/013_temp_obj_multisession.pl +++ b/src/test/modules/test_misc/t/013_temp_obj_multisession.pl @@ -55,25 +55,20 @@ my ($stdout, $stderr); # DML and SELECT have to read the table's data and therefore go through # the buffer manager. With no index on the table, the planner cannot # use index access, so SELECT/UPDATE/DELETE/MERGE/COPY all run through -# the read-stream path. -# -# XXX: in current code, the read-stream path bypasses the -# RELATION_IS_OTHER_TEMP() check, so these commands silently see no -# rows / report zero affected rows -- the visible symptom of the bug -# this test suite documents. A follow-up patch will route the check -# through read_stream_begin_impl() and these assertions will be -# updated to expect "cannot access temporary tables of other sessions". +# the read-stream path and are caught by read_stream_begin_impl(). $node->psql( 'postgres', "SELECT val FROM $tempschema.foo;", - stdout => \$stdout, stderr => \$stderr); -is($stderr, '', 'SELECT (currently no error -- bug to be fixed)'); +like( + $stderr, + qr/cannot access temporary tables of other sessions/, + 'SELECT (seqscan via read_stream)'); # INSERT goes through hio.c which calls ReadBufferExtended() to find a -# page with free space; that hits the existing check before any data is -# written. This case currently errors as expected. +# page with free space; that hits the existing check before any data +# is written. $node->psql( 'postgres', "INSERT INTO $tempschema.foo VALUES (73);", @@ -87,21 +82,21 @@ $node->psql( 'postgres', "UPDATE $tempschema.foo SET val = NULL;", stderr => \$stderr); -is($stderr, '', 'UPDATE (currently no error -- bug to be fixed)'); +like($stderr, qr/cannot access temporary tables of other sessions/, 'UPDATE'); $node->psql('postgres', "DELETE FROM $tempschema.foo;", stderr => \$stderr); -is($stderr, '', 'DELETE (currently no error -- bug to be fixed)'); +like($stderr, qr/cannot access temporary tables of other sessions/, 'DELETE'); $node->psql( 'postgres', "MERGE INTO $tempschema.foo USING (VALUES (42)) AS s(val) " . "ON foo.val = s.val WHEN MATCHED THEN DELETE;", stderr => \$stderr); -is($stderr, '', 'MERGE (currently no error -- bug to be fixed)'); +like($stderr, qr/cannot access temporary tables of other sessions/, 'MERGE'); $node->psql('postgres', "COPY $tempschema.foo TO STDOUT;", stderr => \$stderr); -is($stderr, '', 'COPY (currently no error -- bug to be fixed)'); +like($stderr, qr/cannot access temporary tables of other sessions/, 'COPY'); # DDL and maintenance commands have their own command-specific checks # (older than the buffer-manager check above), so they fail with