Re: We broke the defense against accessing other sessions' temp tables

Thomas Munro <thomas(dot)munro(at)gmail(dot)com> writes:
> On Mon, Sep 22, 2025 at 5:33 AM Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>> I experimented with moving the check into PinBufferForBlock, and
>> that seems to work correctly, but I wonder if someone who's messed
>> with this code more recently than I would prefer a different
>> solution.

> This looks logically correct at first glance, but I guess it would be
> nice to perform the check just once for a stream instead of once per
> block. Each stream only accesses one relation for its whole life*.

Fair point.

> That leads to the idea of leaving the check where it is in
> ReadBufferExtended() for non-stream code paths, but (1) duplicating it
> in read_stream_begin_relation(), (2) documenting in a comment above
> StartReadBuffersImpl() that it's the caller's responsibility not to
> try to read buffers in other backends' temporary tables, and (3)
> turning that proposed check in PinBufferForBlock() into an assertion
> just to catch programming errors. I can try that soon if you think
> that makes sense, unless someone else wants to.

If we put this check at a higher level, there's more risk of people
forgetting the check in new call paths (which is pretty much what
happened here). So I'm a bit leery of it. Still, a backup Assert
at the PinBufferForBlock level might be good enough.

Anyway, happy to turn this over to you, I'm just the messenger.

> Thinking bigger about the scope of the check, I wondered if it could
> make sense to block other backends' relations from being opened in the
> first place. Reading the older threads, I see that that was already
> being discussed[1] by Andres and Andrey (well they didn't say "open"
> but I guess that's what they meant?) in the context of cross-process
> DROP, which would presumably need a special I-know-what-I'm-doing flag
> to escape that check.

Yeah, I'm not sure that that's worth the complication. There is no
logical difficulty in dropping someone else's temp table, only in
trying to access its contents, so I think that a check at the buffer
manager level is really pretty much the right thing.

regards, tom lane