| From: | Bertrand Drouvot <bertranddrouvot(dot)pg(at)gmail(dot)com> |
|---|---|
| To: | Jeff Davis <pgsql(at)j-davis(dot)com> |
| Cc: | Heikki Linnakangas <hlinnaka(at)iki(dot)fi>, Robert Haas <robertmhaas(at)gmail(dot)com>, Roman Eskin <r(dot)eskin(at)arenadata(dot)io>, Michael Paquier <michael(at)paquier(dot)xyz>, Alexander Lakhin <exclusion(at)gmail(dot)com>, pgsql-hackers(at)lists(dot)postgresql(dot)org, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> |
| Subject: | Re: Avoid orphaned objects dependencies, take 3 |
| Date: | 2026-06-05 13:09:37 |
| Message-ID: | aiLKkTC6QBt8i35P@bdtpg |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
Hi,
On Thu, Jun 04, 2026 at 10:03:22AM -0700, Jeff Davis wrote:
> On Mon, 2026-06-01 at 09:21 +0000, Bertrand Drouvot wrote:
> > The tracking array lives in a dedicated AclCheckTrackContext memory
> > context
> > (child of TopMemoryContext). The context is reset at the start of
> > each
> > top-level utility statement, which frees all prior allocations and
> > provides
> > clean lifetime management.
> >
> > Recording is gated by aclcheck_tracking_active, which is set to true
> > only
> > during top-level utility statement execution. This ensures DML and
> > queries pay
> > no cost. The flag is cleared both at normal completion of
> > ProcessUtility and in
> > AbortTransaction to handle the error path.
>
> This could use some better high-level comments in the code. Something
> like:
>
> "DDL performs ACL checks on referenced objects before acquiring a lock
> on them. The lock is acquired much later, when recording dependencies.
> Track the ACL checks, so that we can re-check them after acquiring the
> lock.
Agreed, I just re-worded a bit to add some nuance in v24 attached, as:
"
may perform ACL checks on referenced objects without first holding a lock on
them. In that case, the lock is acquired much later, when recording the
dependencies. Track the ACL checks, so that we can re-check them after acquiring
the lock while recording dependencies."
> XXX: consider refactoring so that we perform the name lookup,
> acquire the lock, and check ACLs all in unison, like
> RangeVarGetRelidExtended()."
I like the XXX, as I agree that what you mentioned in [1] is an area of improvement
but not something directly linked to the TOCTOU issue that this patch is addressing.
FWIW, with the scenario you described in [1]:
- before 2fbb21170e9, the function would be linked to a non existing schema
- with 2fbb21170e9, it produces "ERROR: referenced schema was concurrently dropped"
the current patch just "keep" the post 2fbb21170e9 behavior.
[1]: https://postgr.es/m/5315d15a42109297259d1a3264ad09e363eb98df.camel%40j-davis.com
Regards,
--
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com
| Attachment | Content-Type | Size |
|---|---|---|
| v24-0001-Recheck-permissions-after-lock-acquisition-in-de.patch | text/x-diff | 20.3 KB |
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Bertrand Drouvot | 2026-06-05 13:12:34 | Re: Avoid orphaned objects dependencies, take 3 |
| Previous Message | Tomas Vondra | 2026-06-05 12:52:35 | Re: Adding basic NUMA awareness |