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: 3038e353cf ("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>
This commit is contained in:
Takashi Sakamoto
2026-08-11 21:09:28 +09:00
parent a563a7cb64
commit 05bfb1327d
2 changed files with 21 additions and 13 deletions

View File

@@ -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)
{

View File

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