Re: Avoid orphaned objects dependencies, take 3

public inbox for [email protected]  
help / color / mirror / Atom feed

From: Bertrand Drouvot <[email protected]>
To: Robert Haas <[email protected]>
Cc: Jeff Davis <[email protected]>
Cc: Roman Eskin <[email protected]>
Cc: Michael Paquier <[email protected]>
Cc: Alexander Lakhin <[email protected]>
Cc: [email protected]
Cc: Tom Lane <[email protected]>
Subject: Re: Avoid orphaned objects dependencies, take 3
Date: Mon, 18 May 2026 12:14:34 +0000
Message-ID: <agsCqlLTytZCudMv@bdtpg> (raw)
In-Reply-To: <CA+TgmobtzkG9HzGtDXbgb3KVE46qHuSvGcuKHgAGQ1aejCoLLw@mail.gmail.com>
References: <[email protected]>
	<aC8VT2OjOyPJuKv/@ip-10-97-1-34.eu-west-3.compute.internal>
	<CA+TgmobxpkwwjfM4EkZxyKpJ3S0CqQPXPkqQn4YmCHBeMDypdQ@mail.gmail.com>
	<aD2u/GR/[email protected]>
	<CA+TgmoZh8yXoQ2AF-VFSTswhin0o+BZ78AaOsWZskCW1GHBd6g@mail.gmail.com>
	<[email protected]>
	<[email protected]>
	<ad/awXLnMuc6Rf5M@bdtpg>
	<afCXPVNuvpf5ECGZ@bdtpg>
	<CA+TgmobtzkG9HzGtDXbgb3KVE46qHuSvGcuKHgAGQ1aejCoLLw@mail.gmail.com>

Hi,

On Wed, May 13, 2026 at 04:20:21PM -0400, Robert Haas wrote:
> On Tue, Apr 28, 2026 at 7:17 AM Bertrand Drouvot
> <[email protected]> wrote:
> > 0003: Add Assert guard to detect permission check before lock regressions
> >
> > Add instrumentation under USE_ASSERT_CHECKING to detect cases where object_aclcheck()
> > is called on a referenced object before a lock is held on it, which would widen
> > the TOCTOU window between the permission check and the dependency recording.
> 
> I really like the idea of having some kind of cross-check system that
> can detect future (or current) coding mistakes.

Thanks for the feedback! BTW, it detected a new one due to 4793fc41f82, so v21
attached does fix it to make the CI green.

> But what I wonder
> about this mechanism is: should we instead be insisting that we take a
> lock and check permissions on every dependency? Is it an error to
> record a dependency on an object without any sort of permissions
> check?

I'm not sure. For example, currently, without execute privilege on myfunc(), one
could create a view like:

CREATE VIEW v1 AS SELECT myfunc(a) FROM t1;

so that the dependency is recorded.

The execution permission is checked when the view is queried. I don't think this
example is a bug, so that I'm doubtful about insisting that we take a lock and
check permissions on every dependency.

> Also, I think the mechanism might not be entirely safe. ProcessUtility
> can result in executing user-defined functions which could
> theoretically run other DDL and then it seems like this code would get
> confused.

The assert fires when an aclcheck was tracked and the lock is not held. Since
the locks are transaction-scoped, any lock acquired during the statement persists,
so the assert passes regardless of nesting. So that looks not possible to get false
positives (assert fires) due to user-defined functions running other DDL.

The concern would be false negatives (missed detection) due to the reset wiping
the outer DDL's entries.

I believe, that's theoretically possible only if:

- The outer DDL has a P1 bug (aclcheck without lock on object X)
- A user-defined function happens to run DDL that also aclchecks and locks the same
object X
- The user-defined function DDL's lock satisfies the assert for the outer DDL

But then, the bug would be caught in normal testing without the user-defined
function being present.

So I'm not sure how this code could get confused. Do you have an example of what
you have in mind?

Regards,

-- 
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com

view thread (50+ messages)  latest in thread

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: [email protected]
  Cc: [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected]
  Subject: Re: Avoid orphaned objects dependencies, take 3
  In-Reply-To: <agsCqlLTytZCudMv@bdtpg>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox