Re: BUG with accessing to temporary tables of other sessions still exists
X-MAN <44973863@qq.com>
From: ZizhuanLiu X-MAN <44973863@qq.com>
To: Daniil Davydov <3danissimo@gmail.com>
Cc: Jim Jones <jim.jones@uni-muenster.de>, pgsql-hackers <pgsql-hackers@lists.postgresql.org>
Date: 2026-06-21T08:25:46Z
Lists: pgsql-hackers
>>Hi, >> >>On Sat, Jun 20, 2026 at 10:48?PM ZizhuanLiu X-MAN <44973863@qq.com> wrote: >>> >>> Just as described in the comment of RELATION_IS_OTHER_TEMP macro in rel.h, >>> existing buffer manager routines including ReadBuffer_common(), StartReadBuffersImpl() >>> and PrefetchBuffer() have already invoked this macro to check cross-session temporary >>> table access. All these functions are located in bufmgr.c. >>> >>> For code consistency, I suggest adding the same RELATION_IS_OTHER_TEMP check in >>> ExtendBufferedRelCommon() (also in bufmgr.c), right before calling ExtendBufferedRelLocal(). >> >>Thank you for your feedback! >> >>Do you mean that this check should be moved to "ExtendBufferedRelCommon" >>because this function is in the bufmgr.c file? If so, I don't see the point in >>that. Buffer manager is not only the bufmgr.c file, but also all the files in >>the "storage/buffer" folder. So we can say that localbuf.c also contains logic >>related to the buffer manager. >> >>Moreover, ExtendBufferedRelCommon should contain only the logic that is really >>common for both ordinary and temp tables. All other specific code is >>encapsulated inside the ExtendBufferedRelLocal and ExtendBufferedRelShared. It >>allows us to keep the ExtendBufferedRelCommon function pretty short. >> >>So, I suggest leaving RELATION_IS_OTHER_TEMP as it is. >> >>> Meanwhile, we should also update the comment of RELATION_IS_OTHER_TEMP >>> accordingly to keep the documentation synchronized with code changes. >> >>This comment is already updated - it now mentions the ExtendBufferedRelLocal >>function. >> >>-- >>Best regards, >>Daniil Davydov >Hi,Danii >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`. >Please feel free to point out any flaws or oversights in my reasoning. Alternatively, there is another approach: migrate all existing `RELATION_IS_OTHER_TEMP` validation logic from `bufmgr.c` into `localbuf.c`, so that `localbuf.c` takes responsibility for performing these legitimacy checks. regards, -- ZizhuanLiu (X-MAN) 44973863@qq.com
Commits
-
Prevent access to other sessions' empty temp tables
- c40819ebf954 master landed
- 8021cdceb018 19 (unreleased) landed
- 3aaefe8924f4 18 (unreleased) landed
- 4e49f68b72de 17 (unreleased) landed
- cc3fe7e2a747 16 (unreleased) landed
-
Add tests for cross-session temp table access
- 40927d458fe1 17 (unreleased) cited
-
Prevent access to other sessions' temp tables
- ce146621f786 19 (unreleased) cited