Re: Fix bug with accessing to temporary tables of other sessions
Daniil Davydov <3danissimo@gmail.com>
From: Daniil Davydov <3danissimo@gmail.com>
To: Jim Jones <jim.jones@uni-muenster.de>
Cc: Soumya S Murali <soumyamurali.work@gmail.com>,
Tom Lane <tgl@sss.pgh.pa.us>, Stepan Neretin <slpmcf@gmail.com>,
PostgreSQL Hackers <pgsql-hackers@postgresql.org>
Date: 2026-04-09T11:46:09Z
Lists: pgsql-hackers
Hi,
On Wed, Apr 8, 2026 at 9:04 PM Jim Jones <jim.jones@uni-muenster.de> wrote:
>
> 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().
>
Yeah, I agree with your conclusions.
> 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?
>
Thank you very much for the patch! A few comments:
1) Right now, read stream seems like an appropriate place for this restriction.
But actually the StartReadBuffers is not "binded" to the read stream logic. I
mean that if someone calls it bypassing the "read_stream_begin_relation"
function (it is OK to do so), then our restriction will be violated again.
I think that it will be more reliable to add the restriction directly to the
StartReadBuffers. Also, we can add an assertion ("relation is not other temp
table") to the PinBufferForBlock. What do you think?
2) If we decide to leave restriction in the "read_stream_begin_relation"
function, I would suggest adding a "rel != NULL" check here (read_stream.c):
+ if (RELATION_IS_OTHER_TEMP(rel))
+ ereport(ERROR,
+ (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+ errmsg("cannot access temporary relations of other
sessions")));
3) The "rel != NULL" checks may use the "RelationIsValid" macro, which seems
more pretty to me.
> Daniil, feel free to revert it to your last patch if you disagree with
> this approach.
This approach looks good to me after Tom's explanations.
--
Best regards,
Daniil Davydov
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