Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
Pre-existing issues: - [High] Memory leak in `build_tree()` where dynamically allocated `fw_node` subtrees on the local `stack` list are leaked when returning `NULL` on error paths. - [High] Unconditional dereference of `card->root_node` in `fw_core_handle_bus_reset()` leading to a kernel panic if `build_tree()` fails on the first bus reset. -- commit 814723ea200ca545cf7dcf5b3f733df13cc3c3f2 Author: Kaitao Cheng <[email protected]> firewire: core: Open-code topology list walk This commit open-codes the topology list walk in for_each_fw_node() to defer looking up the next node until after children are appended. It preserves breadth-first traversal semantics while preparing the code for a future list iterator update. > diff --git a/drivers/firewire/core-topology.c > b/drivers/firewire/core-topology.c > index bb2d2db30795e..df2ac0dab1060 100644 > --- a/drivers/firewire/core-topology.c > +++ b/drivers/firewire/core-topology.c [ ... ] drivers/firewire/core-topology.c:build_tree() { ... if (child_port_count > stack_depth) { fw_err(card, "topology stack underflow\n"); return NULL; } ... } [Severity: High] This is a pre-existing issue, but does this code leak the dynamically allocated fw_node subtrees on the local stack list when returning NULL? Since the stack list goes out of scope on this error path, it seems the attached nodes are abandoned without being freed. drivers/firewire/core-topology.c:fw_core_handle_bus_reset() { ... scoped_guard(spinlock, &card->topology_map.lock) { update_topology_map(card->topology_map.buffer, sizeof(card->topology_map.buffer), card->root_node->node_id, self_ids, self_id_count); } ... } [Severity: High] This is a pre-existing issue, but can this sequence cause a panic? If build_tree() fails and returns NULL during the first bus reset, card->root_node would be left as NULL. Would passing card->root_node->node_id unconditionally to update_topology_map() then result in a NULL pointer dereference? -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=2
