Address the -Wuse-after-free warning in ATExecAttachPartition()

Started by Nitin Jadhavover 1 year ago8 messages
#1Nitin Jadhav
nitinjadhavpostgres@gmail.com
1 attachment(s)

In [1]/messages/by-id/202311151802.ngj2la66jwgi@alvherre.pgsql, Andres reported a -Wuse-after-free bug in the
ATExecAttachPartition() function. I've created a patch to address it
with pointers from Amit offlist.

The issue was that the partBoundConstraint variable was utilized after
the list_concat() function. This could potentially lead to accessing
the partBoundConstraint variable after its memory has been freed.

The issue was resolved by using the return value of the list_concat()
function, instead of using the list1 argument of list_concat(). I
copied the partBoundConstraint variable to a new variable named
partConstraint and used it for the previous references before invoking
get_proposed_default_constraint(). I confirmed that the
eval_const_expressions(), make_ands_explicit(),
map_partition_varattnos(), QueuePartitionConstraintValidation()
functions do not modify the memory location pointed to by the
partBoundConstraint variable. Therefore, it is safe to use it for the
next reference in get_proposed_default_constraint()

Attaching the patch. Please review and share the comments if any.
Thanks to Andres for spotting the bug and some off-list advice on how
to reproduce it.

[1]: /messages/by-id/202311151802.ngj2la66jwgi@alvherre.pgsql

Best Regards,
Nitin Jadhav
Azure Database for PostgreSQL
Microsoft

Attachments:

v1-0001-Address-the-Wuse-after-free-warning-in-ATExecAttachP.patchapplication/octet-stream; name=v1-0001-Address-the-Wuse-after-free-warning-in-ATExecAttachP.patchDownload
From 6c23fc9c52531cbce97db1741792f4b6a4c04c8d Mon Sep 17 00:00:00 2001
From: Nitin Jadhav <nitinjadhav@microsoft.com>
Date: Fri, 5 Jul 2024 11:51:36 +0000
Subject: [PATCH] Address the -Wuse-after-free warning in
 ATExecAttachPartition()

Address the warning in the ATExecAttachPartition() function, where
the partBoundConstraint variable was used after the list_concat()
function. This could result in the partBoundConstraint variable
being accessed after its memory has been released. The resolution
is to use the return value of the list_concat() function, rather
than using the list1 argument of list_concat().
---
 src/backend/commands/tablecmds.c | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 8fcb188323..ba3c3c0f29 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -18362,7 +18362,6 @@ ATExecAttachPartition(List **wqueue, Relation rel, PartitionCmd *cmd,
 	Relation	attachrel,
 				catalog;
 	List	   *attachrel_children;
-	List	   *partConstraint;
 	SysScanDesc scan;
 	ScanKeyData skey;
 	AttrNumber	attno;
@@ -18561,12 +18560,19 @@ ATExecAttachPartition(List **wqueue, Relation rel, PartitionCmd *cmd,
 	 * constraint as well.
 	 */
 	partBoundConstraint = get_qual_from_partbound(rel, cmd->bound);
-	partConstraint = list_concat(partBoundConstraint,
+	partBoundConstraint = list_concat(partBoundConstraint,
 								 RelationGetPartitionQual(rel));
 
 	/* Skip validation if there are no constraints to validate. */
-	if (partConstraint)
+	if (partBoundConstraint)
 	{
+		/*
+		 * Preserve the modifications applied to partBoundConstraint by storing
+		 * them in a new variable. This will be utilized later to reconstruct
+		 * the constraint of the default partition.
+		 */
+		List	*partConstraint = partBoundConstraint;
+
 		/*
 		 * Run the partition quals through const-simplification similar to
 		 * check constraints.  We skip canonicalize_qual, though, because
-- 
2.43.0

#2Junwang Zhao
zhjwpku@gmail.com
In reply to: Nitin Jadhav (#1)
Re: Address the -Wuse-after-free warning in ATExecAttachPartition()

On Mon, Jul 8, 2024 at 3:22 PM Nitin Jadhav
<nitinjadhavpostgres@gmail.com> wrote:

In [1], Andres reported a -Wuse-after-free bug in the
ATExecAttachPartition() function. I've created a patch to address it
with pointers from Amit offlist.

The issue was that the partBoundConstraint variable was utilized after
the list_concat() function. This could potentially lead to accessing
the partBoundConstraint variable after its memory has been freed.

The issue was resolved by using the return value of the list_concat()
function, instead of using the list1 argument of list_concat(). I
copied the partBoundConstraint variable to a new variable named
partConstraint and used it for the previous references before invoking
get_proposed_default_constraint(). I confirmed that the
eval_const_expressions(), make_ands_explicit(),
map_partition_varattnos(), QueuePartitionConstraintValidation()
functions do not modify the memory location pointed to by the
partBoundConstraint variable. Therefore, it is safe to use it for the
next reference in get_proposed_default_constraint()

Attaching the patch. Please review and share the comments if any.
Thanks to Andres for spotting the bug and some off-list advice on how
to reproduce it.

The patch LGTM.

Curious how to reproduce the bug ;)

[1]: /messages/by-id/202311151802.ngj2la66jwgi@alvherre.pgsql

Best Regards,
Nitin Jadhav
Azure Database for PostgreSQL
Microsoft

--
Regards
Junwang Zhao

#3Amit Langote
amitlangote09@gmail.com
In reply to: Junwang Zhao (#2)
Re: Address the -Wuse-after-free warning in ATExecAttachPartition()

Hi Junwang,

On Mon, Jul 8, 2024 at 7:08 PM Junwang Zhao <zhjwpku@gmail.com> wrote:

On Mon, Jul 8, 2024 at 3:22 PM Nitin Jadhav
<nitinjadhavpostgres@gmail.com> wrote:

In [1], Andres reported a -Wuse-after-free bug in the
ATExecAttachPartition() function. I've created a patch to address it
with pointers from Amit offlist.

The issue was that the partBoundConstraint variable was utilized after
the list_concat() function. This could potentially lead to accessing
the partBoundConstraint variable after its memory has been freed.

The issue was resolved by using the return value of the list_concat()
function, instead of using the list1 argument of list_concat(). I
copied the partBoundConstraint variable to a new variable named
partConstraint and used it for the previous references before invoking
get_proposed_default_constraint(). I confirmed that the
eval_const_expressions(), make_ands_explicit(),
map_partition_varattnos(), QueuePartitionConstraintValidation()
functions do not modify the memory location pointed to by the
partBoundConstraint variable. Therefore, it is safe to use it for the
next reference in get_proposed_default_constraint()

Attaching the patch. Please review and share the comments if any.
Thanks to Andres for spotting the bug and some off-list advice on how
to reproduce it.

The patch LGTM.

Curious how to reproduce the bug ;)

Download and apply (`git am`) Andres' patch to "add allocator
attributes" here (note it's not merged into the tree yet!):
https://github.com/anarazel/postgres/commit/99067d5c944e7bd29a8702689f470f623723f4e7.patch

Then configure using meson with -Dc_args="-Wuse-after-free=3"
--buildtype=release

Compiling with gcc-12 or gcc-13 should give you the warning that looks
as follows:

[713/2349] Compiling C object
src/backend/postgres_lib.a.p/commands_tablecmds.c.o
../src/backend/commands/tablecmds.c: In function ‘ATExecAttachPartition’:
../src/backend/commands/tablecmds.c:18593:25: warning: pointer
‘partBoundConstraint’ may be used after ‘list_concat’
[-Wuse-after-free]
18593 |
get_proposed_default_constraint(partBoundConstraint);
|
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
../src/backend/commands/tablecmds.c:18546:26: note: call to ‘list_concat’ here
18546 | partConstraint = list_concat(partBoundConstraint,
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
18547 |
RelationGetPartitionQual(rel));
|
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

--
Thanks, Amit Langote

#4Junwang Zhao
zhjwpku@gmail.com
In reply to: Amit Langote (#3)
Re: Address the -Wuse-after-free warning in ATExecAttachPartition()

On Tue, Jul 9, 2024 at 6:18 PM Amit Langote <amitlangote09@gmail.com> wrote:

Hi Junwang,

On Mon, Jul 8, 2024 at 7:08 PM Junwang Zhao <zhjwpku@gmail.com> wrote:

On Mon, Jul 8, 2024 at 3:22 PM Nitin Jadhav
<nitinjadhavpostgres@gmail.com> wrote:

In [1], Andres reported a -Wuse-after-free bug in the
ATExecAttachPartition() function. I've created a patch to address it
with pointers from Amit offlist.

The issue was that the partBoundConstraint variable was utilized after
the list_concat() function. This could potentially lead to accessing
the partBoundConstraint variable after its memory has been freed.

The issue was resolved by using the return value of the list_concat()
function, instead of using the list1 argument of list_concat(). I
copied the partBoundConstraint variable to a new variable named
partConstraint and used it for the previous references before invoking
get_proposed_default_constraint(). I confirmed that the
eval_const_expressions(), make_ands_explicit(),
map_partition_varattnos(), QueuePartitionConstraintValidation()
functions do not modify the memory location pointed to by the
partBoundConstraint variable. Therefore, it is safe to use it for the
next reference in get_proposed_default_constraint()

Attaching the patch. Please review and share the comments if any.
Thanks to Andres for spotting the bug and some off-list advice on how
to reproduce it.

The patch LGTM.

Curious how to reproduce the bug ;)

Download and apply (`git am`) Andres' patch to "add allocator
attributes" here (note it's not merged into the tree yet!):
https://github.com/anarazel/postgres/commit/99067d5c944e7bd29a8702689f470f623723f4e7.patch

Then configure using meson with -Dc_args="-Wuse-after-free=3"
--buildtype=release

Compiling with gcc-12 or gcc-13 should give you the warning that looks
as follows:

[713/2349] Compiling C object
src/backend/postgres_lib.a.p/commands_tablecmds.c.o
../src/backend/commands/tablecmds.c: In function ‘ATExecAttachPartition’:
../src/backend/commands/tablecmds.c:18593:25: warning: pointer
‘partBoundConstraint’ may be used after ‘list_concat’
[-Wuse-after-free]
18593 |
get_proposed_default_constraint(partBoundConstraint);
|
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
../src/backend/commands/tablecmds.c:18546:26: note: call to ‘list_concat’ here
18546 | partConstraint = list_concat(partBoundConstraint,
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
18547 |
RelationGetPartitionQual(rel));
|
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

--
Thanks, Amit Langote

Thanks Amit,

Good to know.

When Nitin says:

```Thanks to Andres for spotting the bug and some off-list advice on how
to reproduce it.```

I thought maybe there is some test case that can really trigger the
use-after-free bug, I might get it wrong though ;)

--
Regards
Junwang Zhao

#5Amit Langote
amitlangote09@gmail.com
In reply to: Junwang Zhao (#4)
Re: Address the -Wuse-after-free warning in ATExecAttachPartition()

On Tue, Jul 9, 2024 at 19:58 Junwang Zhao <zhjwpku@gmail.com> wrote:

On Tue, Jul 9, 2024 at 6:18 PM Amit Langote <amitlangote09@gmail.com>
wrote:

Hi Junwang,

On Mon, Jul 8, 2024 at 7:08 PM Junwang Zhao <zhjwpku@gmail.com> wrote:

On Mon, Jul 8, 2024 at 3:22 PM Nitin Jadhav
<nitinjadhavpostgres@gmail.com> wrote:

In [1], Andres reported a -Wuse-after-free bug in the
ATExecAttachPartition() function. I've created a patch to address it
with pointers from Amit offlist.

The issue was that the partBoundConstraint variable was utilized

after

the list_concat() function. This could potentially lead to accessing
the partBoundConstraint variable after its memory has been freed.

The issue was resolved by using the return value of the list_concat()
function, instead of using the list1 argument of list_concat(). I
copied the partBoundConstraint variable to a new variable named
partConstraint and used it for the previous references before

invoking

get_proposed_default_constraint(). I confirmed that the
eval_const_expressions(), make_ands_explicit(),
map_partition_varattnos(), QueuePartitionConstraintValidation()
functions do not modify the memory location pointed to by the
partBoundConstraint variable. Therefore, it is safe to use it for the
next reference in get_proposed_default_constraint()

Attaching the patch. Please review and share the comments if any.
Thanks to Andres for spotting the bug and some off-list advice on how
to reproduce it.

The patch LGTM.

Curious how to reproduce the bug ;)

Download and apply (`git am`) Andres' patch to "add allocator
attributes" here (note it's not merged into the tree yet!):

https://github.com/anarazel/postgres/commit/99067d5c944e7bd29a8702689f470f623723f4e7.patch

Then configure using meson with -Dc_args="-Wuse-after-free=3"
--buildtype=release

Compiling with gcc-12 or gcc-13 should give you the warning that looks
as follows:

[713/2349] Compiling C object
src/backend/postgres_lib.a.p/commands_tablecmds.c.o
../src/backend/commands/tablecmds.c: In function ‘ATExecAttachPartition’:
../src/backend/commands/tablecmds.c:18593:25: warning: pointer
‘partBoundConstraint’ may be used after ‘list_concat’
[-Wuse-after-free]
18593 |
get_proposed_default_constraint(partBoundConstraint);
|
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
../src/backend/commands/tablecmds.c:18546:26: note: call to

‘list_concat’ here

18546 | partConstraint = list_concat(partBoundConstraint,
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
18547 |
RelationGetPartitionQual(rel));
|
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

--
Thanks, Amit Langote

Thanks Amit,

Good to know.

When Nitin says:

```Thanks to Andres for spotting the bug and some off-list advice on how
to reproduce it.```

I thought maybe there is some test case that can really trigger the
use-after-free bug, I might get it wrong though ;)

I doubt one could write a regression test to demonstrate the use-after-free
bug, though I may of course be wrong. By “reproduce it”, I think Nitin
meant the warning that suggests that use-after-free bug may occur.

Show quoted text
#6Junwang Zhao
zhjwpku@gmail.com
In reply to: Amit Langote (#5)
Re: Address the -Wuse-after-free warning in ATExecAttachPartition()

On Tue, Jul 9, 2024 at 7:39 PM Amit Langote <amitlangote09@gmail.com> wrote:

On Tue, Jul 9, 2024 at 19:58 Junwang Zhao <zhjwpku@gmail.com> wrote:

On Tue, Jul 9, 2024 at 6:18 PM Amit Langote <amitlangote09@gmail.com> wrote:

Hi Junwang,

On Mon, Jul 8, 2024 at 7:08 PM Junwang Zhao <zhjwpku@gmail.com> wrote:

On Mon, Jul 8, 2024 at 3:22 PM Nitin Jadhav
<nitinjadhavpostgres@gmail.com> wrote:

In [1], Andres reported a -Wuse-after-free bug in the
ATExecAttachPartition() function. I've created a patch to address it
with pointers from Amit offlist.

The issue was that the partBoundConstraint variable was utilized after
the list_concat() function. This could potentially lead to accessing
the partBoundConstraint variable after its memory has been freed.

The issue was resolved by using the return value of the list_concat()
function, instead of using the list1 argument of list_concat(). I
copied the partBoundConstraint variable to a new variable named
partConstraint and used it for the previous references before invoking
get_proposed_default_constraint(). I confirmed that the
eval_const_expressions(), make_ands_explicit(),
map_partition_varattnos(), QueuePartitionConstraintValidation()
functions do not modify the memory location pointed to by the
partBoundConstraint variable. Therefore, it is safe to use it for the
next reference in get_proposed_default_constraint()

Attaching the patch. Please review and share the comments if any.
Thanks to Andres for spotting the bug and some off-list advice on how
to reproduce it.

The patch LGTM.

Curious how to reproduce the bug ;)

Download and apply (`git am`) Andres' patch to "add allocator
attributes" here (note it's not merged into the tree yet!):
https://github.com/anarazel/postgres/commit/99067d5c944e7bd29a8702689f470f623723f4e7.patch

Then configure using meson with -Dc_args="-Wuse-after-free=3"
--buildtype=release

Compiling with gcc-12 or gcc-13 should give you the warning that looks
as follows:

[713/2349] Compiling C object
src/backend/postgres_lib.a.p/commands_tablecmds.c.o
../src/backend/commands/tablecmds.c: In function ‘ATExecAttachPartition’:
../src/backend/commands/tablecmds.c:18593:25: warning: pointer
‘partBoundConstraint’ may be used after ‘list_concat’
[-Wuse-after-free]
18593 |
get_proposed_default_constraint(partBoundConstraint);
|
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
../src/backend/commands/tablecmds.c:18546:26: note: call to ‘list_concat’ here
18546 | partConstraint = list_concat(partBoundConstraint,
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
18547 |
RelationGetPartitionQual(rel));
|
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

--
Thanks, Amit Langote

Thanks Amit,

Good to know.

When Nitin says:

```Thanks to Andres for spotting the bug and some off-list advice on how
to reproduce it.```

I thought maybe there is some test case that can really trigger the
use-after-free bug, I might get it wrong though ;)

I doubt one could write a regression test to demonstrate the use-after-free bug, though I may of course be wrong. By “reproduce it”, I think Nitin meant the warning that suggests that use-after-free bug may occur.

got it, thanks~

--
Regards
Junwang Zhao

#7Amit Langote
amitlangote09@gmail.com
In reply to: Junwang Zhao (#2)
1 attachment(s)
Re: Address the -Wuse-after-free warning in ATExecAttachPartition()

Hi,

On Mon, Jul 8, 2024 at 7:08 PM Junwang Zhao <zhjwpku@gmail.com> wrote:

On Mon, Jul 8, 2024 at 3:22 PM Nitin Jadhav
<nitinjadhavpostgres@gmail.com> wrote:

In [1], Andres reported a -Wuse-after-free bug in the
ATExecAttachPartition() function. I've created a patch to address it
with pointers from Amit offlist.

The issue was that the partBoundConstraint variable was utilized after
the list_concat() function. This could potentially lead to accessing
the partBoundConstraint variable after its memory has been freed.

The issue was resolved by using the return value of the list_concat()
function, instead of using the list1 argument of list_concat(). I
copied the partBoundConstraint variable to a new variable named
partConstraint and used it for the previous references before invoking
get_proposed_default_constraint(). I confirmed that the
eval_const_expressions(), make_ands_explicit(),
map_partition_varattnos(), QueuePartitionConstraintValidation()
functions do not modify the memory location pointed to by the
partBoundConstraint variable. Therefore, it is safe to use it for the
next reference in get_proposed_default_constraint()

Attaching the patch. Please review and share the comments if any.
Thanks to Andres for spotting the bug and some off-list advice on how
to reproduce it.

The patch LGTM.

I reviewed the faulty code in ATExecAttachPartition() that this patch
aims to address, and it seems that the fix suggested by Alvaro in the
original report [1] might be more appropriate. It also allows us to
resolve a minor issue related to how partBoundConstraint is handled
later in the function. Specifically, the constraint checked against
the default partition, if one exists on the parent table, should not
include the negated version of the parent's constraint, which the
current code mistakenly does. This occurs because the list_concat()
operation modifies the partBoundConstraint when combining it with the
parent table’s partition constraint. Although this doesn't cause a
live bug, the inclusion of the redundant parent constraint introduces
unnecessary complexity in the combined constraint expression. This can
cause slight delays in evaluating the constraint, as the redundant
check always evaluates to false for rows in the default partition.
Ultimately, the constraint failure is still determined by the negated
version of the partition constraint of the table being attached.

I'm inclined to apply the attached patch only to the master branch as
the case for applying this to back-branches doesn't seem all that
strong to me. Thoughts?

--
Thanks, Amit Langote

Attachments:

v2-0001-Fix-expression-list-handling-in-ATExecAttachParti.patchapplication/octet-stream; name=v2-0001-Fix-expression-list-handling-in-ATExecAttachParti.patchDownload
From 46ac7531201986d62f7dd6d84a304139ef6fd9d4 Mon Sep 17 00:00:00 2001
From: Amit Langote <amitlan@postgresql.org>
Date: Tue, 1 Oct 2024 15:03:02 +0900
Subject: [PATCH v2] Fix expression list handling in ATExecAttachPartition()
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

This commit resolves two issues related to the manipulation of
partition constraint expression list in ATExecAttachPartition().

First, the current use of list_concat() to combine the partition's
constraint (retrieved via get_qual_from_partbound()) with the parent
table’s partition constraint can lead to memory safety issues. After
calling list_concat(), the original constraint (partBoundConstraint)
might no longer be safe to access, as list_concat() may free or alter
it.

Second, there is a logical error when constructing the constraint
expression used to validate against the default partition. The current
approach mistakenly adds a negated version of the parent table’s
partition constraint. This is incorrect, as the default partition’s
constraint must include the parent table’s partition constraint
without negation. Although this does not result in a live bug (since
the negated constraint always evaluates to false and becomes
redundant), it is logically wrong.

To address these issues, list_concat() is replaced with
list_concat_copy(), ensuring that partBoundConstraint remains
unchanged and can be safely reused when constructing the validation
constraint for the default partition.

This change preserves memory safety and prevents the unnecessary
inclusion of redundant constraints.

Nitin Jadhav also posted a patch to address the issue memory safety
issue, but I decided to implement the solution suggested by Alvaro
Herrera in the first discussion because it allows to address the
second issue as well.

Reported-by: Andres Freund <andres@anarazel.de>
Reported-by: Nitin Jadhav <nitinjadhavpostgres@gmail.com>
Reviewed-by: Junwang Zhao <zhjwpku@gmail.com>
Discussion: https://postgr.es/m/20231115165737.zeulb575cgrbqo74@awork3.anarazel.de
Discussion: https://postgr.es/m/CAMm1aWbmYHM3bqtjyMQ-a+4Ub=dgsb_2E3_up2cn=UGdHNrGTg@mail.gmail.com
---
 src/backend/commands/tablecmds.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 87232e0137..0e9b87d499 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -18719,8 +18719,14 @@ ATExecAttachPartition(List **wqueue, Relation rel, PartitionCmd *cmd,
 	 * constraint as well.
 	 */
 	partBoundConstraint = get_qual_from_partbound(rel, cmd->bound);
-	partConstraint = list_concat(partBoundConstraint,
-								 RelationGetPartitionQual(rel));
+
+	/*
+	 * Use list_concat_copy() to avoid modifying partBoundConstraint in place,
+	 * since it’s needed later to construct the constraint expression for
+	 * validating against the default partition, if any.
+	 */
+	partConstraint = list_concat_copy(partBoundConstraint,
+									  RelationGetPartitionQual(rel));
 
 	/* Skip validation if there are no constraints to validate. */
 	if (partConstraint)
-- 
2.43.0

#8Amit Langote
amitlangote09@gmail.com
In reply to: Amit Langote (#7)
Re: Address the -Wuse-after-free warning in ATExecAttachPartition()

On Tue, Oct 1, 2024 at 3:23 PM Amit Langote <amitlangote09@gmail.com> wrote:

Hi,

On Mon, Jul 8, 2024 at 7:08 PM Junwang Zhao <zhjwpku@gmail.com> wrote:

On Mon, Jul 8, 2024 at 3:22 PM Nitin Jadhav
<nitinjadhavpostgres@gmail.com> wrote:

In [1], Andres reported a -Wuse-after-free bug in the
ATExecAttachPartition() function. I've created a patch to address it
with pointers from Amit offlist.

The issue was that the partBoundConstraint variable was utilized after
the list_concat() function. This could potentially lead to accessing
the partBoundConstraint variable after its memory has been freed.

The issue was resolved by using the return value of the list_concat()
function, instead of using the list1 argument of list_concat(). I
copied the partBoundConstraint variable to a new variable named
partConstraint and used it for the previous references before invoking
get_proposed_default_constraint(). I confirmed that the
eval_const_expressions(), make_ands_explicit(),
map_partition_varattnos(), QueuePartitionConstraintValidation()
functions do not modify the memory location pointed to by the
partBoundConstraint variable. Therefore, it is safe to use it for the
next reference in get_proposed_default_constraint()

Attaching the patch. Please review and share the comments if any.
Thanks to Andres for spotting the bug and some off-list advice on how
to reproduce it.

The patch LGTM.

I reviewed the faulty code in ATExecAttachPartition() that this patch
aims to address, and it seems that the fix suggested by Alvaro in the
original report [1] might be more appropriate. It also allows us to
resolve a minor issue related to how partBoundConstraint is handled
later in the function. Specifically, the constraint checked against
the default partition, if one exists on the parent table, should not
include the negated version of the parent's constraint, which the
current code mistakenly does. This occurs because the list_concat()
operation modifies the partBoundConstraint when combining it with the
parent table’s partition constraint. Although this doesn't cause a
live bug, the inclusion of the redundant parent constraint introduces
unnecessary complexity in the combined constraint expression. This can
cause slight delays in evaluating the constraint, as the redundant
check always evaluates to false for rows in the default partition.
Ultimately, the constraint failure is still determined by the negated
version of the partition constraint of the table being attached.

I'm inclined to apply the attached patch only to the master branch as
the case for applying this to back-branches doesn't seem all that
strong to me. Thoughts?

I've pushed this to the master branch now.

Thanks Nitin for the report.

--
Thanks, Amit Langote