From 05bfb1327dc5fb61528bab31cd8f0c1e4bddec23 Mon Sep 17 00:00:00 2001 From: Takashi Sakamoto Date: Tue, 11 Aug 2026 21:09:28 +0900 Subject: [PATCH] firewire: core: fix memory leak in error path of build_tree() In the error path of build_tree(), node instances can remain in the local linked list when the function returns. Whenever an invalid value is detected in the self ID sequence, each allocated node instance is either an entry in the linked list or an entry in the ports array of its parent node. Therefore, the allocate node instances can be safely released by traversing the linked list from its head. Release the remaining node instances with for_each_fw_node() before returning to the caller. Fixes: 3038e353cfaf ("firewire: Add core firewire stack.") Reported-by: Abdun Nihaal Link: https://lore.kernel.org/all/20260727095955.104972-1-nihaal@cse.iitm.ac.in/ Link: https://lore.kernel.org/r/20260811120928.700577-4-o-takashi@sakamocchi.jp Signed-off-by: Takashi Sakamoto --- drivers/firewire/core-topology.c | 30 +++++++++++++++++++++--------- drivers/firewire/node-tree-test.c | 4 ---- 2 files changed, 21 insertions(+), 13 deletions(-) diff --git a/drivers/firewire/core-topology.c b/drivers/firewire/core-topology.c index e032497b2594..ee6b54f89859 100644 --- a/drivers/firewire/core-topology.c +++ b/drivers/firewire/core-topology.c @@ -88,6 +88,17 @@ static inline struct fw_node *fw_node(struct list_head *l) return list_entry(l, struct fw_node, link); } +typedef void (*fw_node_callback_t)(struct fw_card *card, struct fw_node *node, + struct fw_node *parent); + +static void for_each_fw_node(struct fw_card *card, struct fw_node *root, + fw_node_callback_t callback); + +static void free_fw_node(struct fw_card *card, struct fw_node *node, struct fw_node *parent) +{ + kfree(node); +} + /* * This function builds the tree representation of the topology given * by the self IDs from the latest bus reset. During the construction @@ -134,7 +145,7 @@ static struct fw_node *build_tree(struct fw_card *card, const u32 *sid, int self if (PTR_ERR(self_id_sequence) != -ENODATA) { fw_err(card, "inconsistent extended self IDs: %ld\n", PTR_ERR(self_id_sequence)); - return NULL; + goto error; } break; } @@ -168,18 +179,18 @@ static struct fw_node *build_tree(struct fw_card *card, const u32 *sid, int self (enumerator.quadlet_count > 0 && parent_port_count != 1)) { fw_err(card, "parent port inconsistency for node %d: parent_count=%d\n", phy_id, parent_port_count); - return NULL; + goto error; } if (phy_id != phy_packet_self_id_get_phy_id(self_id_sequence[0])) { fw_err(card, "PHY ID mismatch in self ID: %d != %d\n", phy_id, phy_packet_self_id_get_phy_id(self_id_sequence[0])); - return NULL; + goto error; } if (child_port_count > stack_depth) { fw_err(card, "topology stack underflow\n"); - return NULL; + goto error; } /* @@ -197,7 +208,7 @@ static struct fw_node *build_tree(struct fw_card *card, const u32 *sid, int self node = fw_node_create(self_id_sequence[0], total_port_count, card->color); if (node == NULL) { fw_err(card, "out of memory while building topology\n"); - return NULL; + goto error; } if (phy_id == (card->node_id & 0x3f)) @@ -256,12 +267,13 @@ static struct fw_node *build_tree(struct fw_card *card, const u32 *sid, int self card->beta_repeaters_present = beta_repeaters_present; return local_node; +error: + ++card->color; + list_for_each_entry_safe(node, child, &stack, link) + for_each_fw_node(card, node, free_fw_node); + return NULL; } -typedef void (*fw_node_callback_t)(struct fw_card * card, - struct fw_node * node, - struct fw_node * parent); - static void for_each_fw_node(struct fw_card *card, struct fw_node *root, fw_node_callback_t callback) { diff --git a/drivers/firewire/node-tree-test.c b/drivers/firewire/node-tree-test.c index 251c0113639f..5e6bb58537e9 100644 --- a/drivers/firewire/node-tree-test.c +++ b/drivers/firewire/node-tree-test.c @@ -521,7 +521,6 @@ static void node_tree_test_invalid_extended_self_id_sequence(struct kunit *test) card->node_id = LOCAL_BUS | 0x03; - // TODO: Memory leak. card->local_node = build_tree(card, self_id_sequence, ARRAY_SIZE(self_id_sequence), 123); KUNIT_EXPECT_NULL(test, card->local_node); } @@ -541,7 +540,6 @@ static void node_tree_test_invalid_phy_id(struct kunit *test) card->node_id = LOCAL_BUS | 0x03; - // TODO: Memory leak. card->local_node = build_tree(card, self_id_sequence, ARRAY_SIZE(self_id_sequence), 123); KUNIT_EXPECT_NULL(test, card->local_node); } @@ -561,7 +559,6 @@ static void node_tree_test_invalid_child_port_count(struct kunit *test) card->node_id = LOCAL_BUS | 0x03; - // TODO: Memory leak. card->local_node = build_tree(card, self_id_sequence, ARRAY_SIZE(self_id_sequence), 123); KUNIT_EXPECT_NULL(test, card->local_node); } @@ -581,7 +578,6 @@ static void node_tree_test_invalid_parent_port_count(struct kunit *test) card->node_id = LOCAL_BUS | 0x03; - // TODO: Memory leak. card->local_node = build_tree(card, self_id_sequence, ARRAY_SIZE(self_id_sequence), 123); KUNIT_EXPECT_NULL(test, card->local_node); }