Horton
Menu

Thread

Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →

  1. Fix some near-bugs related to ResourceOwner function arguments

  1. Small bugs regarding resowner handling in aio.c, catcache.c

    Matthias van de Meent <boekewurm+postgres@gmail.com> — 2025-12-09T23:33:23Z

    Hi Heikki, Andres,

Whilst looking through catcache.c's code I noticed this piece of code:

ReleaseCatCacheListWithOwner(CatCList *list, ResourceOwner resowner)
{
    [...]
    if (resowner)
        ResourceOwnerForgetCatCacheRef(CurrentResourceOwner, &ct->tuple);

Note how the resowner argument is ignored in favour of
CurrentResourceOwner; and that probably wasn't what the author
intended.

So I looked around a bit, and found 2 more similar instances: one more
in catcache.c, and one in aio.c.  I can't guarantee that there are no
other comparable issues, but at least I could not immediately find any
functions that ignore their ResourceOwner argument in favour of
CurrentResourceOwner.

The issue in aio.c is externally visible, so it might trigger issues.
However, every caller in the tree uses CurrentResourceOwner, so only
extensions could trigger this bug.
The two issues in catcache.c are benign: No external code can trigger
it with a different resource owner; and while no internal code would
trigger it either, it's better to use the provided resowner, so that
future callers won't activate the bug.

Attached a fix for all 3 cases.


Kind regards,

Matthias van de Meent
Databricks (https://www.databricks.com)

  2. Re: Small bugs regarding resowner handling in aio.c, catcache.c

    Heikki Linnakangas <hlinnaka@iki.fi> — 2025-12-10T09:51:35Z

    On 10/12/2025 01:33, Matthias van de Meent wrote:
> Hi Heikki, Andres,
> 
> Whilst looking through catcache.c's code I noticed this piece of code:
> 
> ReleaseCatCacheListWithOwner(CatCList *list, ResourceOwner resowner)
> {
>      [...]
>      if (resowner)
>          ResourceOwnerForgetCatCacheRef(CurrentResourceOwner, &ct->tuple);
> 
> Note how the resowner argument is ignored in favour of
> CurrentResourceOwner; and that probably wasn't what the author
> intended.
> 
> So I looked around a bit, and found 2 more similar instances: one more
> in catcache.c, and one in aio.c.  I can't guarantee that there are no
> other comparable issues, but at least I could not immediately find any
> functions that ignore their ResourceOwner argument in favour of
> CurrentResourceOwner.
> 
> The issue in aio.c is externally visible, so it might trigger issues.
> However, every caller in the tree uses CurrentResourceOwner, so only
> extensions could trigger this bug.
> The two issues in catcache.c are benign: No external code can trigger
> it with a different resource owner; and while no internal code would
> trigger it either, it's better to use the provided resowner, so that
> future callers won't activate the bug.
> 
> Attached a fix for all 3 cases.

Pushed and backpatched, thanks!

- Heikki

  3. Re: Small bugs regarding resowner handling in aio.c, catcache.c

    Andres Freund <andres@anarazel.de> — 2025-12-10T15:04:13Z

    Hi,

On 2025-12-10 11:51:35 +0200, Heikki Linnakangas wrote:
> On 10/12/2025 01:33, Matthias van de Meent wrote:
> > Hi Heikki, Andres,
> > 
> > Whilst looking through catcache.c's code I noticed this piece of code:
> > 
> > ReleaseCatCacheListWithOwner(CatCList *list, ResourceOwner resowner)
> > {
> >      [...]
> >      if (resowner)
> >          ResourceOwnerForgetCatCacheRef(CurrentResourceOwner, &ct->tuple);
> > 
> > Note how the resowner argument is ignored in favour of
> > CurrentResourceOwner; and that probably wasn't what the author
> > intended.
> > 
> > So I looked around a bit, and found 2 more similar instances: one more
> > in catcache.c, and one in aio.c.  I can't guarantee that there are no
> > other comparable issues, but at least I could not immediately find any
> > functions that ignore their ResourceOwner argument in favour of
> > CurrentResourceOwner.
> > 
> > The issue in aio.c is externally visible, so it might trigger issues.
> > However, every caller in the tree uses CurrentResourceOwner, so only
> > extensions could trigger this bug.
> > The two issues in catcache.c are benign: No external code can trigger
> > it with a different resource owner; and while no internal code would
> > trigger it either, it's better to use the provided resowner, so that
> > future callers won't activate the bug.
> > 
> > Attached a fix for all 3 cases.
> 
> Pushed and backpatched, thanks!

Thanks for finding/fixing this and merging the change!

Greetings,

Andres Freund