Prevent access to other sessions' empty temp tables

Commit ce146621 ensures that ERROR is raised if a session tries to read
pages of another session's temp table.  But there is a corner case where
the other session's temp table is empty -- in this case the INSERT
command bypasses our checks and executes without any errors.

Such behavior is inconsistent and erroneous: it leaves an invalid buffer
in the temp buffers pool.  Since the buffer was created for another
session's temp table, we get an error "no such file or directory" when
trying to flush it.

This commit fixes it by adding a RELATION_IS_OTHER_TEMP check in the
relation-extension path.

Backpatch to 16, because it is the first release after 31966b151e6, which
introduced a separate local relation extension function
ExtendBufferedRelLocal(), which lacks of RELATION_IS_OTHER_TEMP() check.
As this fix introduces more checks to 013_temp_obj_multisession.pl, backpatch
the whole test script to 16.

Discussion: https://postgr.es/m/CAJDiXgiX2XZBHDNo%2BzBbvku%2BtchrUurvPRaN1_40mEQ1_sG90g%40mail.gmail.com
Author: Daniil Davydov <3danissimo@gmail.com>
Reviewed-by: Jim Jones <jim.jones@uni-muenster.de>
Reviewed-by: Imran Zaheer <imran.zhir@gmail.com>
Reviewed-by: ZizhuanLiu X-MAN <44973863@qq.com>
Backpatch-through: 16

diff --git a/src/backend/storage/buffer/bufmgr.c b/src/backend/storage/buffer/bufmgr.c
index f79a8fa5da21b31278e5dd8ce6a8331cf07cdf24..9ab282a76d1af210dec9491781838b978de23553 100644 (file)
--- a/src/backend/storage/buffer/bufmgr.c
+++ b/src/backend/storage/buffer/bufmgr.c
@@ -2767,9 +2767,23 @@ ExtendBufferedRelCommon(BufferManagerRelation bmr,
                                                                                 extend_by);
 
        if (bmr.relpersistence == RELPERSISTENCE_TEMP)
+       {
+               /*
+                * Reject attempts to extend non-local temporary relations; we have no
+                * ability to transfer about-to-be-created local buffers into the
+                * owning session's local buffers.  This is the canonical place for
+                * the check, covering any attempt to extend a non-local temporary
+                * relation.
+                */
+               if (bmr.rel && RELATION_IS_OTHER_TEMP(bmr.rel))
+                       ereport(ERROR,
+                                       (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+                                        errmsg("cannot access temporary tables of other sessions")));
+
                first_block = ExtendBufferedRelLocal(bmr, fork, flags,
                                                                                         extend_by, extend_upto,
                                                                                         buffers, &extend_by);
+       }
        else
                first_block = ExtendBufferedRelShared(bmr, fork, strategy, flags,
                                                                                          extend_by, extend_upto,

diff --git a/src/include/utils/rel.h b/src/include/utils/rel.h
index fa07ebf8ff7c7dfef163fce1fe6dfa32ef679b9a..89c159b133fad2a0661b21dcc097b09055e3d3e0 100644 (file)
--- a/src/include/utils/rel.h
+++ b/src/include/utils/rel.h
@@ -668,10 +668,10 @@ RelationCloseSmgr(Relation relation)
  * the owning session keeps the data in its private local buffer pool,
  * 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.
+ * PrefetchBuffer() and ExtendBufferedRelCommon()) 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 5f3cc7d2fc5b989de9b382e34e17dfd26b37003a..ff6f23ef3b168ce36177ead079600385494ac99e 100644 (file)
--- a/src/test/modules/test_misc/t/013_temp_obj_multisession.pl
+++ b/src/test/modules/test_misc/t/013_temp_obj_multisession.pl
@@ -36,6 +36,10 @@ my $psql1 = $node->background_psql('postgres');
 # masked by an index scan that would hit ReadBuffer_common from nbtree.
 $psql1->query_safe(q(CREATE TEMP TABLE foo AS SELECT 42 AS val;));
 
+# Also create an empty table, so read path go straight through the
+# extend-relation entry point.
+$psql1->query_safe(q(CREATE TEMP TABLE empty_foo (val INT);));
+
 # Resolve the owner's temp schema so the probing session can refer to
 # the table by a fully-qualified name.
 my $tempschema = $node->safe_psql(
@@ -66,6 +70,18 @@ like(
        qr/cannot access temporary tables of other sessions/,
        'SELECT (seqscan via read_stream)');
 
+# INSERT into empty table goes through hio.c which calls RelationAddBlocks() to
+# extend the table; that hits the check before new pages are created for the
+# table.
+$node->psql(
+       'postgres',
+       "INSERT INTO $tempschema.empty_foo VALUES (42);",
+       stderr => \$stderr);
+like(
+       $stderr,
+       qr/cannot access temporary tables of other sessions/,
+       'INSERT (caught via hio.c)');
+
 # 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.