Date: Mon, 18 May 2026 12:14:34 +0000
From: Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
To: Robert Haas <robertmhaas@gmail.com>
Cc: Jeff Davis <pgsql@j-davis.com>, Roman Eskin <r.eskin@arenadata.io>,
	Michael Paquier <michael@paquier.xyz>,
	Alexander Lakhin <exclusion@gmail.com>,
	pgsql-hackers@lists.postgresql.org, Tom Lane <tgl@sss.pgh.pa.us>
Subject: Re: Avoid orphaned objects dependencies, take 3
Message-ID: <agsCqlLTytZCudMv@bdtpg>
References: <298d95b5570b91a7d0ee42895e79890feefc67d6.camel@j-davis.com>
 <aC8VT2OjOyPJuKv/@ip-10-97-1-34.eu-west-3.compute.internal>
 <CA+TgmobxpkwwjfM4EkZxyKpJ3S0CqQPXPkqQn4YmCHBeMDypdQ@mail.gmail.com>
 <aD2u/GR/yaRkBJdJ@ip-10-97-1-34.eu-west-3.compute.internal>
 <CA+TgmoZh8yXoQ2AF-VFSTswhin0o+BZ78AaOsWZskCW1GHBd6g@mail.gmail.com>
 <aEJ+oNANWutG8aDv@ip-10-97-1-34.eu-west-3.compute.internal>
 <9f5f0dd4-96d5-4d90-9436-62d96e0298f9@arenadata.io>
 <ad/awXLnMuc6Rf5M@bdtpg>
 <afCXPVNuvpf5ECGZ@bdtpg>
 <CA+TgmobtzkG9HzGtDXbgb3KVE46qHuSvGcuKHgAGQ1aejCoLLw@mail.gmail.com>
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="FjYZKkuBdH+F20sI"
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: 
 <CA+TgmobtzkG9HzGtDXbgb3KVE46qHuSvGcuKHgAGQ1aejCoLLw@mail.gmail.com>
Archived-At: <https://www.postgresql.org/message-id/agsCqlLTytZCudMv%40bdtpg>
Precedence: bulk


--FjYZKkuBdH+F20sI
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: 8bit

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
> <bertranddrouvot.pg@gmail.com> 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

--FjYZKkuBdH+F20sI
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
	filename="v21-0001-Avoid-orphaned-objects-dependencies.patch"