Hi,

On Wed, Mar 25, 2026 at 1:58 AM Tom Lane <[email protected]> wrote:
>
> > Actually, v12 patch is not about a superuser rights restriction, but about
> > forbidding such operations for everyone.
>
> ... including superusers, who bypass permissions restrictions
> everywhere else.  You are going to have to contort the ACL system
> badly to make that happen at all, and I would not be surprised
> if you introduce new bugs.
>

I've never dealt with the ACL system before, so it's hard for me to assess
the scale of the problem. I am inclined to believe you on this issue.

> >> The actual problem is that the buffer manager is incapable of dealing
> >> with other sessions' temp tables, and we need to un-break the buffer
> >> manager's defense for that implementation restriction.  So I feel the
> >> correct approach is something similar to what I described here:
> >> https://www.postgresql.org/message-id/flat/2736425.1758475979%40sss.pgh.pa.us
>
> > Handling access to other-temp-tables on the buffer manager level seems to me
> > like fighting the symptom, not the cause.
>
> No, it IS the cause.  If someday someone were to reimplement buffer
> management in a way that didn't have this implementation restriction,
> we would surely not arbitrarily restrict superusers from looking at
> tables that they then would physically be able to look at.
>

OK, now I fully understand your point (I hope so) :
We want to restrict backend access to other-temp-tables just because it is
physically impossible for them to read the pages of such tables. And if some
users has enough privileges, it is OK for us to allow them to
lock/vacuum/drop/... other-temp-tables. I.e. only operations with heap pages
access must be forbidden, and the buffer manager layer is an appropriate place
for it.

> > Am I missing something?
>
> Mainly, that we had a setup that was working fine for decades,
> until somebody made holes in it with careless refactoring.
> We should fix that mistake, not introduce inconsistent-with-
> decades-of-practice permissions behavior to hide the mistake
> at an unrelated logical level.
>

Yeah, we have a few checks in the bufmgr (PrefetchBuffer, ReadBufferExtended),
but they stopped coping with their task.

> Also, we need a defense at the buffer manager level anyway, because
> otherwise C code could try to access another session's temp table
> and we'd not realize it was getting bogus answers.  (Whether such
> an attempt is a bug or not is a different discussion; but we at
> least need some logic that detects that it won't work, and the ACL
> system cannot be expected to stop C-level code from trying.)
>
> Also, we really need a patch that's simple and non-invasive enough
> to be back-patched into v17 and v18.  This proposal is not that.
>

OK

>
> > I don't actually want to use gram.y as a main solver of this issue. But
> > gram.y is setting the "relpersistence" field for the RangeVar and all
> > subsequent code is treating this value as truthful.
>
> I do kind of agree with this concern, but the v12 patch simply moves
> the untruthfulness around.  Reality is that we cannot know whether an
> unqualified-name RangeVar references a temp table until we do a
> catalog lookup, ...

Yep, Jim's example shows us that we cannot always rely on the "relpersistence"
field.

> ...so IMO we should not have a relpersistence field there
> at all.  At best it means something quite different from what it means
> elsewhere, and that's a recipe for confusion.  But changing that would
> not be a bug fix (AFAIK) but refactoring to reduce the probability of
> future bugs.
>

I agree with the idea to get rid of this field. By now I cannot say for sure
whether we can fix a bug without modifying the RangeVar structure. But I'll
try to implement proposed logic only within the bufmgr.


Thank you very much for your comments! I'll post a new patch in the near
future.

--
Best regards,
Daniil Davydov


Reply via email to