From ab95349c90af6067bf7d8f325a07d84a757e1afd Mon Sep 17 00:00:00 2001 From: Deyan Ginev Date: Wed, 29 Jul 2026 19:00:19 -0400 Subject: [PATCH 1/2] =?UTF-8?q?feat:=20Node::free=5Fsubtree=20=E2=80=94=20?= =?UTF-8?q?immediate=20discard=20with=20wrapper=20neutralization?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit unlink_node alone never frees a doc-owned node, and set_rust_owned defers the free to the last wrapper drop — which a stray clone in a long-lived collection postpones past the owning document's xmlFreeDoc, turning the eventual xmlFreeNode into a use-after-free (observed: SIGSEGV in xmlDictOwns when a consumer's bookkeeping Vec dropped after the Document). free_subtree frees the C subtree NOW: it unlinks, walks the subtree iteratively (elements, texts, attributes), nulls the shared node_ptr of every registered wrapper (clones anywhere become inert no-ops) and clears the bookkeeping entries, then calls xmlFreeNode once. Documents and document fragments are refused. Driver: latexml-oxide's math parser discards thousands of replaced subtrees per document; without a true free they leak ~1.4 MB per formula. Co-Authored-By: Claude Fable 5 --- src/tree/node.rs | 75 ++++++++++++++++++++++++++++ tests/free_subtree_tests.rs | 99 +++++++++++++++++++++++++++++++++++++ 2 files changed, 174 insertions(+) create mode 100644 tests/free_subtree_tests.rs diff --git a/src/tree/node.rs b/src/tree/node.rs index cfc795d08..d1ed275be 100644 --- a/src/tree/node.rs +++ b/src/tree/node.rs @@ -1210,6 +1210,81 @@ impl Node { matches!(self.0.borrow().linkage, Linkage::RustOwned) } + /// Detach this node and free its entire C subtree IMMEDIATELY. + /// + /// This is the discard operation `unlink_node` alone does not provide: an + /// unlinked doc-owned node is freed by nobody (see `unlink_node`), so a + /// caller discarding a subtree for good must transfer ownership somewhere. + /// `set_rust_owned` defers the free to the last wrapper drop — but a stray + /// clone parked in a long-lived collection postpones that free past the + /// owning document's `xmlFreeDoc`, and the eventual `xmlFreeNode` then + /// reads the freed document (its dictionary) — a use-after-free. + /// + /// `free_subtree` instead frees NOW, after *neutralizing* every registered + /// Rust wrapper into the subtree: each wrapper's shared inner `node_ptr` + /// is nulled — clones held anywhere become inert (`Drop` is a no-op, the + /// null-guarded accessors return their empty defaults) — and the + /// bookkeeping entries are cleared, so a future allocation reusing one of + /// the freed addresses cannot collide with a stale wrapper. Attribute + /// nodes are neutralized along with elements and texts; namespace + /// declarations and attribute values are owned by their nodes and freed by + /// `xmlFreeNode` itself. + /// + /// Documents (and document-fragment shells) are refused — tearing a whole + /// document down belongs to `Document`'s `Drop`. + /// + /// # Safety contract (mirrors C `xmlFreeNode`) + /// + /// The subtree must be garbage to the caller. Wrappers created in an + /// earlier bookkeeping epoch (re-wrapped after an unlink cleared their + /// entry) cannot be found and neutralized here; using one after this call + /// is undefined behavior, exactly as using a freed `xmlNodePtr` in C. + pub fn free_subtree(mut self) { + if matches!( + self.get_type(), + Some(NodeType::DocumentNode) | Some(NodeType::DocumentFragNode) + ) { + return; + } + // Sever from parent/siblings (and clear self's bookkeeping entry). + self.unlink_node(); + let top = self.0.borrow().node_ptr; + if top.is_null() { + return; // already freed/neutralized through another handle + } + if let Some(doc) = self.get_docref().upgrade() { + // Walk the subtree iteratively (an explicit stack — recursion would + // risk the call stack on deep trees), neutralizing wrappers. + let mut stack: Vec = vec![top]; + while let Some(ptr) = stack.pop() { + let mut child = xmlGetFirstChild(ptr); + while !child.is_null() { + stack.push(child); + child = xmlNextSibling(child); + } + // Attribute structs share the xmlNode prefix layout through `ns`, + // but only ELEMENTS have a `properties` field — reading it through + // an attribute or text pointer would be out of bounds. + if NodeType::from_int(xmlGetNodeType(ptr)) == Some(NodeType::ElementNode) { + let mut attr = xmlGetFirstProperty(ptr); + while !attr.is_null() { + stack.push(attr as xmlNodePtr); + attr = xmlNextPropertySibling(attr); + } + } + let mut doc_borrowed = doc.borrow_mut(); + if let Some(wrapper) = doc_borrowed.get_node(ptr) { + wrapper.0.borrow_mut().node_ptr = ptr::null_mut(); + } + doc_borrowed.forget_node(ptr); + } + } + // Neutralize this handle itself (its registry entry went away with + // unlink_node, so the walk above could not reach it), then free. + self.0.borrow_mut().node_ptr = ptr::null_mut(); + unsafe { xmlFreeNode(top) }; + } + fn ptr_as_option(&self, node_ptr: xmlNodePtr) -> Option { if node_ptr.is_null() { None diff --git a/tests/free_subtree_tests.rs b/tests/free_subtree_tests.rs new file mode 100644 index 000000000..12bc66caf --- /dev/null +++ b/tests/free_subtree_tests.rs @@ -0,0 +1,99 @@ +//! Tests for `Node::free_subtree` — immediate discard of a garbage subtree +//! with wrapper neutralization, the operation `unlink_node` + +//! `set_rust_owned` cannot provide safely when stray clones survive in +//! long-lived collections. + +use libxml::parser::Parser; +use libxml::tree::{Document, Node, NodeType}; + +/// Freeing a subtree detaches it from the document and neutralizes every +/// live wrapper into it: held clones become inert (null node) instead of +/// dangling, and their later drop is a no-op — even after the document +/// itself is gone. +#[test] +fn free_subtree_neutralizes_held_wrappers() { + let parser = Parser::default(); + let doc = parser + .parse_string("text".as_bytes()) + .expect("parse"); + let root = doc.get_root_element().expect("root"); + let a = root.get_first_element_child().expect("a"); + let b = a.get_first_element_child().expect("b"); + let b_clone = b.clone(); // a stray holder, as in a bookkeeping Vec + + a.free_subtree(); + + // The document no longer contains ; survives. + let remaining = root.get_first_element_child().expect("c stays"); + assert_eq!(remaining.get_name(), "c"); + // The held descendant wrapper is inert, not dangling. + assert!(b_clone.get_type().is_none(), "neutralized wrapper has no type"); + assert_eq!(b_clone.get_name(), "", "null-guarded accessors go empty"); + // Dropping holders after the document is freed must be a no-op. + drop(doc); + drop(b_clone); +} + +/// An already-unlinked (detached) subtree is freed too — the case where the +/// caller detached first and decides to discard afterwards. +#[test] +fn free_subtree_frees_detached_tree() { + let parser = Parser::default(); + let doc = parser + .parse_string("".as_bytes()) + .expect("parse"); + let root = doc.get_root_element().expect("root"); + let mut a = root.get_first_element_child().expect("a"); + let b = a.get_first_element_child().expect("b"); + a.unlink_node(); + a.free_subtree(); + assert!(b.get_type().is_none(), "detached descendant neutralized"); + assert!(root.get_first_element_child().is_none()); +} + +/// Fresh, never-linked nodes (built trees that end up discarded) free +/// cleanly, and the neutralization covers nodes added under them. +#[test] +fn free_subtree_frees_built_tree() { + let mut doc = Document::new().expect("new doc"); + let mut root = Node::new("r", None, &doc).expect("root"); + doc.set_root_element(&root); + let mut shell = Node::new("shell", None, &doc).expect("shell"); + let mut inner = Node::new("inner", None, &doc).expect("inner"); + shell.add_child(&mut inner).expect("add"); + let inner_clone = inner.clone(); + // Never linked into the document tree. + shell.free_subtree(); + assert!(inner_clone.get_type().is_none()); + // Document tree unaffected. + assert_eq!(root.get_name(), "r"); +} + +/// Calling `free_subtree` twice through separate clones must not +/// double-free: the second call sees a neutralized (null) handle. +#[test] +fn free_subtree_second_call_is_noop() { + let parser = Parser::default(); + let doc = parser + .parse_string("".as_bytes()) + .expect("parse"); + let root = doc.get_root_element().expect("root"); + let a = root.get_first_element_child().expect("a"); + let a_clone = a.clone(); + a.free_subtree(); + a_clone.free_subtree(); // neutralized — must be a no-op +} + +/// Documents and document-fragment shells are refused. +#[test] +fn free_subtree_refuses_documents() { + let parser = Parser::default(); + let doc = parser.parse_string("".as_bytes()).expect("parse"); + let root = doc.get_root_element().expect("root"); + if let Some(doc_node) = root.get_parent() { + assert_eq!(doc_node.get_type(), Some(NodeType::DocumentNode)); + doc_node.free_subtree(); // refused + } + // Still a live document with a root. + assert_eq!(doc.get_root_element().expect("root").get_name(), "r"); +} From 7e5507b9a2d1b10e54c591d425d8be1512a12145 Mon Sep 17 00:00:00 2001 From: Deyan Ginev Date: Wed, 29 Jul 2026 19:00:47 -0400 Subject: [PATCH 2/2] release prep: 0.3.17 + changelog entry for free_subtree Co-Authored-By: Claude Fable 5 --- CHANGELOG.md | 14 ++++++++++++++ Cargo.toml | 2 +- tests/free_subtree_tests.rs | 2 +- 3 files changed, 16 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 25c4a194f..16f41c922 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,20 @@ ## [Unreleased] +## [0.3.17] (2026-07-29) + +### Added + +* `Node::free_subtree`: detach and free an entire C subtree IMMEDIATELY, + neutralizing every registered Rust wrapper into it (shared `node_ptr` + nulled — stray clones become inert no-ops; bookkeeping entries cleared so + address reuse cannot collide with a stale wrapper). This is the discard + operation `unlink_node` + `set_rust_owned` cannot provide safely when + clones survive in long-lived collections: their deferred drop can fire + `xmlFreeNode` after the owning document's `xmlFreeDoc` (use-after-free in + `xmlDictOwns`). Driver: consumers that discard thousands of replaced + subtrees per document (latexml-oxide's math parser) leaked them all. + ## [0.3.16] (2026-07-12) ### Fixed diff --git a/Cargo.toml b/Cargo.toml index d187261e4..409b09f38 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "libxml" -version = "0.3.16" +version = "0.3.17" edition = "2024" rust-version = "1.88" authors = ["Andreas Franzén ", "Deyan Ginev ","Jan Frederik Schaefer "] diff --git a/tests/free_subtree_tests.rs b/tests/free_subtree_tests.rs index 12bc66caf..e75cf5e6c 100644 --- a/tests/free_subtree_tests.rs +++ b/tests/free_subtree_tests.rs @@ -56,7 +56,7 @@ fn free_subtree_frees_detached_tree() { #[test] fn free_subtree_frees_built_tree() { let mut doc = Document::new().expect("new doc"); - let mut root = Node::new("r", None, &doc).expect("root"); + let root = Node::new("r", None, &doc).expect("root"); doc.set_root_element(&root); let mut shell = Node::new("shell", None, &doc).expect("shell"); let mut inner = Node::new("inner", None, &doc).expect("inner");