Thread

Re: Avoid orphaned objects dependencies, take 3

Bertrand Drouvot <bertranddrouvot.pg@gmail.com> — 2026-05-18T12:14:34Z

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