double free when deleting nested XML elements using // (descendant-or-self) #319
Comments
GeoffWilliams
pushed a commit
to GeoffWilliams/puppetlabs-tomcat
that referenced
this issue
Nov 24, 2015
lutter
added a commit
to lutter/augeas
that referenced
this issue
Nov 24, 2015
In a tree like /files/1/2, when we execute 'rm /files//*', the path expression matches /files/1 and /files/1/2. When tree_rm goes to delete these two nodes, it first deletes (frees) /files/1 and all its descendents. By the time we try to delete /files/1/2, the pointer we have to that is no longer valid and we end up causing a double-free. With this change, we make sure we only delete a node if none of its ancestors is being deleted beforehand in the same operation - deleting a node, and one of its ancestors afterwards is fine as the pointer to the ancestor is still valid. Fixes hercules-team#319 Special shoutout to Geoff Williams for finding, diagnosing and filing a great bug report about this.
|
Thanks for the excellent and thorough bug report. Having that made fixing this bug really easy. I'll leave the PR open for a couple days to give you chance to test it - if you do, lmk if it really does fix the issue. |
|
NP! PR fixes issue |
lutter
added a commit
to lutter/augeas
that referenced
this issue
Nov 26, 2015
In a tree like /files/1/2, when we execute 'rm /files//*', the path expression matches /files/1 and /files/1/2. When tree_rm goes to delete these two nodes, it first deletes (frees) /files/1 and all its descendents. By the time we try to delete /files/1/2, the pointer we have to that is no longer valid and we end up causing a double-free. With this change, we make sure we only delete a node if none of its ancestors is being deleted beforehand in the same operation - deleting a node, and one of its ancestors afterwards is fine as the pointer to the ancestor is still valid. Fixes hercules-team#319 Special shoutout to Geoff Williams for finding, diagnosing and filing a great bug report about this.
xbezdick
added a commit
to xbezdick/openstack-puppet-modules
that referenced
this issue
Dec 15, 2015
70a1b729140ee803724933edf839f5d7ae4daf39 Merge pull request redhat-openstack#113 from puppetlabs/1.3.x e93da60ae443dc30763a463475edf6cd58eef7d8 Merge pull request redhat-openstack#111 from GeoffWilliams/augeas_segfault_workaround 6c5c77e7cf959abdd1ace4294bdb5728b9e27e06 Merge pull request redhat-openstack#110 from tphoney/release_1.3.3 259ed084aca3360d7dfc86e54072fb0e06b0cbfd 1.3.3 release prep 957dfdc5884a1ebe29dc296d8616339d20ffaae9 delete realm nodes in depth-first order to workaround augeas segfault (hercules-team/augeas#319) 501ad90d4f5391f79ec37f192a4dbc9ab4779e30 Merge pull request redhat-openstack#107 from joshbeard/purge_connector_readme f277c2fa1f84f5e4c9325c7929e7f4024e558c14 Document 'purge_connectors' cf4e55790aeefd8c558681d9d475e84a3b52b9cc Merge pull request redhat-openstack#105 from joshbeard/purge_connectors ed4f2e98ab10256d5fad53468a9904e3be1ddff3 Merge pull request redhat-openstack#106 from DavidS/add-package-install-options c6d42c40deca6025899df8579dd6cc61a6aac645 (MAINT) enable install options in package resource 23eeb156c63f8870c3b1ac398c9842dda33512ae Specify port when purging connectors 34d866bf2f455cf771262e147c5bd5722690e008 Merge pull request redhat-openstack#104 from puppetlabs/1.3.x Change-Id: I084ab5906e10e01a9d1d6f488c4421b7bf5c4d3e
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Overview
Double free from libaugeas with lens
Xml.lnswhen deleting nested XML elements collected using//(descendant-or-self).Software versions
background
Augeas is being used to remove all
Realmelements from Tomcat'sserver.xmlfile using theruby-augeasAPI like this:The operation itself seems to succeed but there is an error from
libaugeaswhen the library attempts to free the memory being used.Originally this problem was encountered invoking augeas through Puppet but I was able to reproduce the problem using
augtool, see testcase.External bug report: https://tickets.puppetlabs.com/browse/MODULES-1721
Error details
The above Ruby function enters libaugeas in
aug_rm()and results in the following backtrace when running under puppet:Once I'd developed a stand-alone testcase I was able to add some print statements and step through the code with GDB and I found that:
free_tree_node()whenfree()is used against a dangling pointerfree_tree_node(del)and recompile the library code works as expected but will leak memoryRealmelements are built, one for each present in the input XML filetree_rm()making a call totree_unlink_raw(del[i])free_tree_node()and the pointer value matches an element inside one of the previous treesNarrative (added
printfstatements)Root cause
Inside the XML file, I see the following
Realmelements:It seems the parser has mapped the exact same memory for elements of the
struct treemember being passed totree_unlink_raw()which makes sense because the elements are nestedTestcase
assuming
testcase.xmlpresent on system at/vagrant/testcase.xmland with a freshly compiled libaugeas from git, free of any source modifications:In the second testcase, I've un-nested the elements and Augeas is able to remove them with no problems:
How to fix this?
I'm not immediately clear on how this can be fixed being new to the internals of Augeas.
Looking at the nesting of the elements I'm trying to delete, the pointer structure I'm seeing makes sense since its the exact same XML nodes inside one another. I had been working under the assumption that the code was copying an object by reference instead of cloning it but clearly the parsing code is correct and not at fault.
It looks like the trees passed for deletion in
tree_rm()need to be de-duplicated somehow, so that we don't try to delete nodes that are contained within a parent tree that has already been deleted - but I'm not sure of the best way to do this, can anyone help?Thanks!
testcase.xml.txt
testcase2.xml.txt
The text was updated successfully, but these errors were encountered: