[PATCH v2] firewire: core: fix possible memory leak in build_tree()

[PATCH v2] firewire: core: fix possible memory leak in build_tree()

[Abdun Nihaal]

The memory allocated for node in fw_node_create() is not freed when a
parent port inconsistency error occurs. Fix that by invalidating
references to the node, and then calling fw_node_put() on it.

Fixes: 3038e353cfaf ("firewire: Add core firewire stack.")
Cc: stable@vger.kernel.org
Signed-off-by: Abdun Nihaal <nihaal@cse.iitm.ac.in>
---
Compile tested only. Issue found using static analysis

v1->v2:
- Invalidate references to the node in the child nodes,
  as suggested by Takashi Sakamoto.
- Use fw_node_put() instead of kfree() for consistency

v1 link: https://lore.kernel.org/all/20260727095955.104972-1-nihaal@cse.iitm.ac.in/

 drivers/firewire/core-topology.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/drivers/firewire/core-topology.c b/drivers/firewire/core-topology.c
index df2ac0dab106..3d0109c3ed90 100644
--- a/drivers/firewire/core-topology.c
+++ b/drivers/firewire/core-topology.c
@@ -227,6 +227,15 @@ static struct fw_node *build_tree(struct fw_card *card, const u32 *sid, int self
 		    (enumerator.quadlet_count > 0 && parent_count != 1)) {
 			fw_err(card, "parent port inconsistency for node %d: "
 			       "parent_count=%d\n", phy_id, parent_count);
+			while (h != &stack) {
+				child = fw_node(h);
+				for (i = 0; i < child->port_count; i++) {
+					if (child->ports[i] == node)
+						child->ports[i] = NULL;
+				}
+				h = h->next;
+			}
+			fw_node_put(node);
 			return NULL;
 		}
 
-- 
2.43.0

Re: [PATCH v2] firewire: core: fix possible memory leak in build_tree()

[Takashi Sakamoto]

Hi,

Thanks you for sending the patch.

On Sat, Aug 01, 2026 at 04:39:13PM +0530, Abdun Nihaal wrote:
> The memory allocated for node in fw_node_create() is not freed when a
> parent port inconsistency error occurs. Fix that by invalidating
> references to the node, and then calling fw_node_put() on it.
> 
> Fixes: 3038e353cfaf ("firewire: Add core firewire stack.")
> Cc: stable@vger.kernel.org
> Signed-off-by: Abdun Nihaal <nihaal@cse.iitm.ac.in>
> ---
> Compile tested only. Issue found using static analysis
> 
> v1->v2:
> - Invalidate references to the node in the child nodes,
>   as suggested by Takashi Sakamoto.
> - Use fw_node_put() instead of kfree() for consistency
> 
> v1 link: https://lore.kernel.org/all/20260727095955.104972-1-nihaal@cse.iitm.ac.in/
> 
>  drivers/firewire/core-topology.c | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/drivers/firewire/core-topology.c b/drivers/firewire/core-topology.c
> index df2ac0dab106..3d0109c3ed90 100644
> --- a/drivers/firewire/core-topology.c
> +++ b/drivers/firewire/core-topology.c
> @@ -227,6 +227,15 @@ static struct fw_node *build_tree(struct fw_card *card, const u32 *sid, int self
>  		    (enumerator.quadlet_count > 0 && parent_count != 1)) {
>  			fw_err(card, "parent port inconsistency for node %d: "
>  			       "parent_count=%d\n", phy_id, parent_count);
> +			while (h != &stack) {

This line would be replaced with '!list_is_head(h, &stack)' in
include/linux/list.h.

> +				child = fw_node(h);
> +				for (i = 0; i < child->port_count; i++) {
> +					if (child->ports[i] == node)
> +						child->ports[i] = NULL;
> +				}
> +				h = h->next;
> +			}
> +			fw_node_put(node);
>  			return NULL;
>  		}

After my further investigation, I realized that the memory leak still
remains if the above error is detected in the middle of selfID sequence
traversal, since the stack can include some node instances unlinked to
their parent, or the issued node instance has the references to some child
nodes which are deleted from the stack. We need more careful method to
release all of the intermediate state of tree.


Thanks

Takashi Sakamoto