Hi, On Sun, Jun 21, 2026 at 3:08 PM ZizhuanLiu X-MAN <[email protected]> wrote: > > Thank you very much for your feedback, as well as the original patch and test > cases. > I have learned a lot about the internals of temporary tables from this > discussion. > > First, I would like to clarify that the buffer manager is not limited to > `bufmgr.c` alone, > but covers all source files under the `storage/buffer` directory. > > I’ve analyzed the existing architecture and the relationship between > `bufmgr.c`, > the `RELATION_IS_OTHER_TEMP` macro and `localbuf.c`, and my observations > are summarised below: > > 1. Within the `storage/buffer` module, `RELATION_IS_OTHER_TEMP` is currently > referenced > in three places, all of which reside in `bufmgr.c`; there are no usages in > `localbuf.c` at present. > > 2. Looking at the function call graph between `bufmgr.c` and `localbuf.c`, > most routines in > `localbuf.c` serve as helper functions invoked from `bufmgr.c`. (A tiny > number of calls exist > within `src/test/modules/test_io.c`, which I believe can be disregarded for > this discussion.) > > 3. Under the current design, `localbuf.c` acts as a callee layer focused > solely on implementing > core logic without introducing `RELATION_IS_OTHER_TEMP` checks, keeping this > module clean. > All access validation decisions using `RELATION_IS_OTHER_TEMP` are handled > upstream in > `bufmgr.c` as the main caller. > > 4. Placing the `RELATION_IS_OTHER_TEMP` check inside > `ExtendBufferedRelCommon()` in `bufmgr.c` > (rather than `ExtendBufferedRelLocal()` in `localbuf.c`) aligns with this > existing architectural principle: > validation rules are enforced by the caller layer. Admittedly, this approach > has a downside — if new > caller entrypoints are added in the future, each would need to remember to > perform this security check. > Fortunately, all relevant call paths currently originate from `bufmgr.c`.
OK, now I see your point, thank you. I agree that localbuf.c is a "callee layer focused solely on implementing core logic". Personally, I don't think that this fact somehow prevents us from performing some checks related to the temporary table inside localbuf.c, even if they look like some kind of "top-level" logic. But we already have an example where RELATION_IS_OTHER_TEMP is processed inside a function common to local and regular tables - PrefetchBuffer. I'll move the RELATION_IS_OTHER_TEMP into the bufmgr.c in order to be consistent with an existing pattern. Thank you for pointing it out! Please, see proposed change in the v3 patch. -- Best regards, Daniil Davydov
From 81deaaa7471a54f66a3160aeb9676b3c19c8661b Mon Sep 17 00:00:00 2001 From: Daniil Davidov <[email protected]> Date: Sun, 21 Jun 2026 15:58:27 +0700 Subject: [PATCH v3] Prevent access to other sessions' empty temp tables Commit ce146621 ensures that ERROR is raised if session tryes to read pages of other session's temp table. But there is corner case, when other temp table is empty - in this case the INSERT command will bypass our checks and executes without any errors. Such a behavior is inconsistent and erroneous, because it leaves an invalid buffer in the temp buffers pool : since buffer was created for other temp table, we will face an error "no such file or directory" while trying to flush this buffer. This commit fixes it by adding a RELATION_IS_OTHER_TEMP check in the relation-extension path. --- src/backend/storage/buffer/bufmgr.c | 14 ++++++++++++++ src/include/utils/rel.h | 8 ++++---- .../test_misc/t/013_temp_obj_multisession.pl | 16 ++++++++++++++++ 3 files changed, 34 insertions(+), 4 deletions(-) diff --git a/src/backend/storage/buffer/bufmgr.c b/src/backend/storage/buffer/bufmgr.c index d6c0cc1f6d4..e4e4971e51d 100644 --- 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 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 fa07ebf8ff7..89c159b133f 100644 --- 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 5f3cc7d2fc5..ff6f23ef3b1 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 @@ -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. -- 2.43.0
