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:08:55Z
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. Below is a partial summary of the call relationships among `bufmgr.c`, `RELATION_IS_OTHER_TEMP` and `localbuf.c` (this is not an exhaustive list): PrefetchBuffer() --RELATION_IS_OTHER_TEMP + PrefetchLocalBuffer() /* pass it off to localbuf.c */ --PrefetchSharedBuffer() ReadBuffer_common() RELATION_IS_OTHER_TEMP ExtendBufferedRel()==>ExtendBufferedRel()===>ExtendBufferedRelBy()==>ExtendBufferedRelCommon()==>ExtendBufferedRelCommon() PinBufferForBlock() StartReadBuffer()==>StartReadBuffersImpl() WaitReadBuffers() static ExtendBufferedRelCommon() --if bmr.relpersistence == RELPERSISTENCE_TEMP call ExtendBufferedRelLocal() /* @localbuf.c */ -- else ExtendBufferedRelShared() static StartReadBuffersImpl() RELATION_IS_OTHER_TEMP static PinBufferForBlock() if (persistence == RELPERSISTENCE_TEMP) LocalBufferAlloc() /* @localbuf.c */ else BufferAlloc() FlushRelationBuffers() PinLocalBuffer() /* @localbuf.c */ FlushLocalBuffer() /* @localbuf.c */ UnpinLocalBuffer /* @localbuf.c */ MarkBufferDirty() MarkLocalBufferDirty() /* @localbuf.c */ MarkBufferDirtyHint() MarkLocalBufferDirty() /* @localbuf.c */ BufferSetHintBits16() MarkLocalBufferDirty() /* @localbuf.c */ ZeroAndLockBuffer() StartLocalBufferIO() /* @localbuf.c */ TerminateLocalBufferIO() /* @localbuf.c */ StartBufferIO() StartLocalBufferIO() /* @localbuf.c */ buffer_readv_complete_one() TerminateLocalBufferIO() /* @localbuf.c */ 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