Re: Fix bug with accessing to temporary tables of other sessions
Jim Jones <jim.jones@uni-muenster.de>
From: Jim Jones <jim.jones@uni-muenster.de>
To: Soumya S Murali <soumyamurali.work@gmail.com>,
Daniil Davydov <3danissimo@gmail.com>
Cc: Tom Lane <tgl@sss.pgh.pa.us>, Stepan Neretin <slpmcf@gmail.com>,
PostgreSQL Hackers <pgsql-hackers@postgresql.org>
Date: 2026-04-08T14:04:35Z
Lists: pgsql-hackers
Attachments
- v15-0001-Prevent-access-to-other-sessions-temporary-table.patch (text/x-patch) patch v15-0001
- v15-0002-Test-cross-session-access-on-temporary-tables.patch (text/x-patch) patch v15-0002
Hi,
I did some digging:
It seems that b7b0f3f2724 is what actually broke it. By switching from
ReadBufferExtended() to read_stream_next_buffer(), it silently routed
all SELECT/UPDATE/DELETE/COPY away from the check that was sitting in
ReadBufferExtended().
@@ -528,25 +599,23 @@ heap_fetch_next_buffer(HeapScanDesc scan,
ScanDirection dir)
*/
CHECK_FOR_INTERRUPTS();
- if (unlikely(!scan->rs_inited))
+ /*
+ * If the scan direction is changing, reset the prefetch block
to the
+ * current block. Otherwise, we will incorrectly prefetch the blocks
+ * between the prefetch block and the current block again before
+ * prefetching blocks in the new, correct scan direction.
+ */
+ if (unlikely(scan->rs_dir != dir))
{
- scan->rs_cblock = heapgettup_initial_block(scan, dir);
+ scan->rs_prefetch_block = scan->rs_cblock;
+ read_stream_reset(scan->rs_read_stream);
+ }
- /* ensure rs_cbuf is invalid when we get
InvalidBlockNumber */
- Assert(scan->rs_cblock != InvalidBlockNumber ||
- !BufferIsValid(scan->rs_cbuf));
+ scan->rs_dir = dir;
- scan->rs_inited = true;
- }
- else
- scan->rs_cblock = heapgettup_advance_block(scan,
scan->rs_cblock,
-
dir);
-
- /* read block if valid */
- if (BlockNumberIsValid(scan->rs_cblock))
- scan->rs_cbuf = ReadBufferExtended(scan->rs_base.rs_rd,
MAIN_FORKNUM,
-
scan->rs_cblock, RBM_NORMAL,
-
scan->rs_strategy);
+ scan->rs_cbuf = read_stream_next_buffer(scan->rs_read_stream, NULL);
+ if (BufferIsValid(scan->rs_cbuf))
+ scan->rs_cblock = BufferGetBlockNumber(scan->rs_cbuf);
}
It simply builds upon 210622c60e1 (introduces StartReadBuffers ->
PinBufferForBlock) and b5a9b18cd0b (builds read_stream_begin_relation on
top of it).
So I thought we can explore the option of adding this check directly in
read_stream_begin_relation():
if (RELATION_IS_OTHER_TEMP(rel))
ereport(ERROR,
(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
errmsg("cannot access temporary relations of other sessions")));
Thoughts?
See v15 attached.
Daniil, feel free to revert it to your last patch if you disagree with
this approach.
Best, Jim
Commits
Same data as JSON:
GET /api/v1/messages/:b64id/commits
the thread's linked commits as JSON, with link sources.
API reference →
-
Prevent access to other sessions' temp tables
- 4dfae59a1d31 17 (unreleased) landed
- 1b0dd08157bf 18 (unreleased) landed
- ce146621f786 19 (unreleased) landed
-
Add tests for cross-session temp table access
- 40927d458fe1 17 (unreleased) landed
- 1cd37a7a8dc6 18 (unreleased) landed
- 1fee0e857e33 19 (unreleased) landed
-
Doc: use "an SQL" consistently rather than "a SQL"
- b51f86e49a7f 18.0 cited
-
Modify the relcache to record the temp status of both local and nonlocal
- 948d6ec90fd3 8.4.0 cited