Обсуждение: Potential use-after-free in partion related code

Hi,

A while back I had proposed annotations for palloc() et al that let the
compiler know about which allocators pair with what freeing functions. One
thing that allows the compiler to do is to detect use after free.

One such complaint is:

../../../../../home/andres/src/postgresql/src/backend/commands/tablecmds.c: In function ‘ATExecAttachPartition’:
../../../../../home/andres/src/postgresql/src/backend/commands/tablecmds.c:18758:25: warning: pointer
‘partBoundConstraint’may be used after ‘list_concat’ [-Wuse-after-free]
 
18758 |                         get_proposed_default_constraint(partBoundConstraint);
      |                         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
../../../../../home/andres/src/postgresql/src/backend/commands/tablecmds.c:18711:26: note: call to ‘list_concat’ here
18711 |         partConstraint = list_concat(partBoundConstraint,
      |                          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
18712 |                                                                  RelationGetPartitionQual(rel));
      |                                                                  ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~


And it seems quite right:

    partConstraint = list_concat(partBoundConstraint,
                                 RelationGetPartitionQual(rel));

At this point partBoundConstraint may not be used anymore, because
list_concat() might have reallocated.

But then a few lines later:

        /* we already hold a lock on the default partition */
        defaultrel = table_open(defaultPartOid, NoLock);
        defPartConstraint =
            get_proposed_default_constraint(partBoundConstraint);

We use partBoundConstraint again.

I unfortunately can't quickly enough identify what partConstraint,
defPartConstraint, partBoundConstraint are, so I don't don't really know what
the fix here is.

Greetings,

Andres Freund

On 2023-Nov-15, Andres Freund wrote:

>     partConstraint = list_concat(partBoundConstraint,
>                                  RelationGetPartitionQual(rel));
> 
> At this point partBoundConstraint may not be used anymore, because
> list_concat() might have reallocated.
> 
> But then a few lines later:
> 
>         /* we already hold a lock on the default partition */
>         defaultrel = table_open(defaultPartOid, NoLock);
>         defPartConstraint =
>             get_proposed_default_constraint(partBoundConstraint);
> 
> We use partBoundConstraint again.

Yeah, this is wrong if partBoundConstraint is reallocated by
list_concat.  One possible fix is to change list_concat to
list_concat_copy(), which leaves the original list unmodified.

AFAICT the bug came in with 6f6b99d1335b, which added default
partitions.

-- 
Álvaro Herrera        Breisgau, Deutschland  —  https://www.EnterpriseDB.com/
"Now I have my system running, not a byte was off the shelf;
It rarely breaks and when it does I fix the code myself.
It's stable, clean and elegant, and lightning fast as well,
And it doesn't cost a nickel, so Bill Gates can go to hell."