Re: [PATCH v2 02/14] firewire: core: Open-code topology list walk

[sashiko-bot@kernel.org]

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 <chengkaitao@kylinos.cn>

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/20260609064122.95825-1-kaitao.cheng@linux.dev?part=2