Prevent access to other sessions' temp tables

Commit b7b0f3f272 ("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 <jim.jones@uni-muenster.de>
Author: Daniil Davydov <3danissimo@gmail.com>
Co-authored-by: Alexander Korotkov <aekorotkov@gmail.com>
Reviewed-by: Michael Paquier <michael@paquier.xyz>
Reviewed-by: Soumya S Murali <soumyamurali.work@gmail.com>
Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://postgr.es/m/CAJDiXghdFcZ8%3Dnh4G69te7iRr3Q0uFyXxb3ZdG09_GTNZXwH0g%40mail.gmail.com
Backpatch-through: 17

src/backend/storage/aio/read_stream.c

@@ -555,6 +555,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

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 ReadBuffer_common */
 		if (RELATION_IS_OTHER_TEMP(reln))
 			ereport(ERROR,
 					(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
@@ -807,19 +807,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);
@@ -1200,6 +1191,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()
@@ -1274,6 +1277,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")));
+
 	for (int i = 0; i < actual_nblocks; ++i)
 	{
 		bool		found;

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

src/test/modules/test_misc/t/013_temp_obj_multisession.pl

@@ -56,25 +56,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);",
@@ -88,21 +83,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