On Tue, Jul 9, 2024 at 19:58 Junwang Zhao <zhjw...@gmail.com> wrote: > On Tue, Jul 9, 2024 at 6:18 PM Amit Langote <amitlangot...@gmail.com> > wrote: > > > > Hi Junwang, > > > > On Mon, Jul 8, 2024 at 7:08 PM Junwang Zhao <zhjw...@gmail.com> wrote: > > > On Mon, Jul 8, 2024 at 3:22 PM Nitin Jadhav > > > <nitinjadhavpostg...@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. >
