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 <nihaal@cse.iitm.ac.in>
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 <o-takashi@sakamocchi.jp>

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);
 }