On Tue, Jul 9, 2024 at 7:39 PM Amit Langote <amitlangot...@gmail.com> wrote: > > 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.
got it, thanks~ -- Regards Junwang Zhao
