[PATCH 0/3] firewire: core: add KUnit tests for tree building

[PATCH 0/3] firewire: core: add KUnit tests for tree building

[Takashi Sakamoto]

Hi,

The core function builds internal node tree by parsing self ID sequence
at bus reset. Abdun Nihaal points to a memory leak in this process at
encountering a failure in the middle of self ID sequence enumeration[1].
This issue is not yet solved, and some unit tests make it easier to work
for it, mainly to avoid some regressions when refactoring codes.

This patchset adds some KUnit tests for the purpose. Especially, the
tests added at the last patch includes actual cases to hit the memory
leak.

In my development environment (Ubuntu 26.04, amd64), I can see the
memory leak by enabling and redirecting some tracepoints events to kmsg
on the qemu process for KUnit tests execution:

```
$ python3 tools/testing/kunit/kunit.py run \
  --arch=x86_64 \
  --cross_compile=x86_64-linux-gnu- \
  --kunitconfig=drivers/firewire/ \
  --kconfig_add=CONFIG_FTRACE=y \
  --kernel_args=tp_printk=1 \
  --kernel_args=trace_event=kmem:kmalloc,kmem:kfree \
  --kernel_args=hash_pointers=never \
  --raw_output=all
```

In the KUnit tests for successful cases, the allocated memory objects
are released successfully in the test side, like:

```
kmalloc: call_site=build_tree+0x203/0x5d0 ptr=ffff8e10413f0b80 bytes_req=48 bytes_alloc=64 gfp_flags=GFP_ATOMIC|__GFP_ZERO node=-1 accounted=false
kmalloc: call_site=build_tree+0x203/0x5d0 ptr=ffff8e10413f0b40 bytes_req=64 bytes_alloc=64 gfp_flags=GFP_ATOMIC|__GFP_ZERO node=-1 accounted=false
kfree: call_site=for_each_fw_node+0x17c/0x230 ptr=ffff8e10413f0b40
kfree: call_site=for_each_fw_node+0x17c/0x230 ptr=ffff8e10413f0b80
```

For the issued cases, we can see no kfree events corresponding to the
allocated memory objects, like: 

```
kmalloc: call_site=build_tree+0x203/0x5d0 ptr=ffff8e10413f0940 bytes_req=64 bytes_alloc=64 gfp_flags=GFP_ATOMIC|__GFP_ZERO node=-1 accounted=false
kmalloc: call_site=build_tree+0x203/0x5d0 ptr=ffff8e10413f0900 bytes_req=48 bytes_alloc=64 gfp_flags=GFP_ATOMIC|__GFP_ZERO node=-1 accounted=false
kmalloc: call_site=build_tree+0x203/0x5d0 ptr=ffff8e10413f08c0 bytes_req=64 bytes_alloc=64 gfp_flags=GFP_ATOMIC|__GFP_ZERO node=-1 accounted=false
kmalloc: call_site=build_tree+0x203/0x5d0 ptr=ffff8e10413f0880 bytes_req=64 bytes_alloc=64 gfp_flags=GFP_ATOMIC|__GFP_ZERO node=-1 accounted=false
firewire_core node_tree_test_invalid_parent_port_count.dummy-device: parent port inconsistency for node 3: parent_count=2
...
```

The memory objects remain in the build_tree() function and should be
released internally. This is the addressing issue.

[1] https://lore.kernel.org/lkml/20260801110915.82561-1-nihaal@cse.iitm.ac.in/


Takashi Sakamoto (3):
  firewire: core: add KUnit test skeleton for node tree
  firewire: core: add KUnit tests for successful tree building
  firewire: core: add KUnit tests for failure of tree building

 drivers/firewire/.kunitconfig     |   1 +
 drivers/firewire/Kconfig          |  15 +
 drivers/firewire/core-topology.c  |   4 +
 drivers/firewire/node-tree-test.c | 611 ++++++++++++++++++++++++++++++
 4 files changed, 631 insertions(+)
 create mode 100644 drivers/firewire/node-tree-test.c


base-commit: a5367912ba81dfb3180fce89b545b009e57f17a5
-- 
2.53.0

[PATCH 1/3] firewire: core: add KUnit test skeleton for node tree

[Takashi Sakamoto]

Some issues have been reported in node tree management. Refactoring
the topology-related code in the core is required. Adding unit tests
would help ensure that the refactoring does not introduce regressions.

This commit adds a KUnit test skeleton for this purpose.

Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
---
 drivers/firewire/.kunitconfig     |  1 +
 drivers/firewire/Kconfig          | 15 +++++++++++++++
 drivers/firewire/core-topology.c  |  4 ++++
 drivers/firewire/node-tree-test.c | 19 +++++++++++++++++++
 4 files changed, 39 insertions(+)
 create mode 100644 drivers/firewire/node-tree-test.c

diff --git a/drivers/firewire/.kunitconfig b/drivers/firewire/.kunitconfig
index 21b7e9eef63d..7406acb00478 100644
--- a/drivers/firewire/.kunitconfig
+++ b/drivers/firewire/.kunitconfig
@@ -6,3 +6,4 @@ CONFIG_FIREWIRE_KUNIT_DEVICE_ATTRIBUTE_TEST=y
 CONFIG_FIREWIRE_KUNIT_PACKET_SERDES_TEST=y
 CONFIG_FIREWIRE_KUNIT_SELF_ID_SEQUENCE_HELPER_TEST=y
 CONFIG_FIREWIRE_KUNIT_OHCI_SERDES_TEST=y
+CONFIG_FIREWIRE_KUNIT_NODE_TREE_TEST=y
diff --git a/drivers/firewire/Kconfig b/drivers/firewire/Kconfig
index a5f5e250223a..b5abe00accc9 100644
--- a/drivers/firewire/Kconfig
+++ b/drivers/firewire/Kconfig
@@ -81,6 +81,21 @@ config FIREWIRE_KUNIT_SELF_ID_SEQUENCE_HELPER_TEST
 	  For more information on KUnit and unit tests in general, refer
 	  to the KUnit documentation in Documentation/dev-tools/kunit/.
 
+config FIREWIRE_KUNIT_NODE_TREE_TEST
+	tristate "KUnit tests for node tree" if !KUNIT_ALL_TESTS
+	depends on FIREWIRE && KUNIT
+	default KUNIT_ALL_TESTS
+	help
+	  This builds the KUnit tests for node tree.
+
+	  KUnit tests run during boot and output the results to the debug
+	  log in TAP format (https://testanything.org/). Only useful for
+	  kernel devs running KUnit test harness and are not for inclusion
+	  into a production build.
+
+	  For more information on KUnit and unit tests in general, refer
+	  to the KUnit documentation in Documentation/dev-tools/kunit/.
+
 config FIREWIRE_OHCI
 	tristate "OHCI-1394 controllers"
 	depends on PCI && FIREWIRE
diff --git a/drivers/firewire/core-topology.c b/drivers/firewire/core-topology.c
index df2ac0dab106..1d3a4419f554 100644
--- a/drivers/firewire/core-topology.c
+++ b/drivers/firewire/core-topology.c
@@ -507,3 +507,7 @@ void fw_core_handle_bus_reset(struct fw_card *card, int node_id, int generation,
 	}
 }
 EXPORT_SYMBOL(fw_core_handle_bus_reset);
+
+#ifdef CONFIG_FIREWIRE_KUNIT_NODE_TREE_TEST
+#include "node-tree-test.c"
+#endif
diff --git a/drivers/firewire/node-tree-test.c b/drivers/firewire/node-tree-test.c
new file mode 100644
index 000000000000..fca71dd99b25
--- /dev/null
+++ b/drivers/firewire/node-tree-test.c
@@ -0,0 +1,19 @@
+// SPDX-License-Identifier: GPL-2.0-only
+//
+// node-tree-test.c - An application of Kunit to test node tree.
+//
+// Copyright (c) 2026 Takashi Sakamoto
+//
+// This file can not be built independently since it is intentionally included in core-topology.c.
+
+#include <kunit/test.h>
+
+static struct kunit_case node_tree_test_cases[] = {
+	{}
+};
+
+static struct kunit_suite node_tree_test_suite = {
+	.name = "firewire-node-tree",
+	.test_cases = node_tree_test_cases,
+};
+kunit_test_suite(node_tree_test_suite);
-- 
2.53.0

[PATCH 2/3] firewire: core: add KUnit tests for successful tree building

[Takashi Sakamoto]

After a bus reset, self ID sequence is captured by 1394 OHCI hardware and
passed to software through SelfID DMA context. The core parses the sequence
to build an internal cache of the node tree for the current generation
of the bus. This is the first step in managing resources on the bus.

The tree is build by the build_tree() function. This commit adds KUnit
tests for the function, covering several successful scenarios.

Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
---
 drivers/firewire/node-tree-test.c | 508 ++++++++++++++++++++++++++++++
 1 file changed, 508 insertions(+)

diff --git a/drivers/firewire/node-tree-test.c b/drivers/firewire/node-tree-test.c
index fca71dd99b25..6f404df3d50e 100644
--- a/drivers/firewire/node-tree-test.c
+++ b/drivers/firewire/node-tree-test.c
@@ -7,13 +7,521 @@
 // This file can not be built independently since it is intentionally included in core-topology.c.
 
 #include <kunit/test.h>
+#include <kunit/test-bug.h>
+#include <kunit/device.h>
+
+struct private_data {
+	struct fw_card *card;
+	unsigned int release_count;
+};
+
+static int node_tree_test_init(struct kunit *test)
+{
+	struct private_data *data;
+
+	data = kunit_kzalloc(test, sizeof(*data), GFP_KERNEL);
+	KUNIT_ASSERT_NOT_NULL(test, data);
+
+	data->card = kunit_kzalloc(test, sizeof(struct fw_card), GFP_KERNEL);
+	KUNIT_ASSERT_NOT_NULL(test, data->card);
+
+	data->card->device = kunit_device_register(test, "dummy-device");
+	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, data->card->device);
+
+	test->priv = data;
+
+	return 0;
+}
+
+static void node_tree_test_exit(struct kunit *test)
+{
+	struct private_data *data = test->priv;
+
+	kunit_device_unregister(test, data->card->device);
+	kunit_kfree(test, data->card);
+	kunit_kfree(test, data);
+}
+
+static void release_fw_node(struct fw_card *card, struct fw_node *node, struct fw_node *parent)
+{
+	struct private_data *data = kunit_get_current_test()->priv;
+
+	fw_node_put(node);
+	++data->release_count;
+}
+
+static void node_tree_test_two_nodes(struct kunit *test)
+{
+	//       root
+	// ++============++
+	// ||    phy 1   ||
+	// ||  P0 P1 P2  ||
+	// ++===|==|==|==++
+	//            |
+	//      +-----+
+	//      |
+	// ++===|==x==x==++
+	// ||  P0 P1 P2  ||
+	// ||    phy 0   ||
+	// ++============++
+	//
+	static const u32 self_id_sequence[] = {
+		0x80000080,
+		0x8100005e,
+	};
+	struct private_data *data = test->priv;
+	struct fw_card *card = data->card;
+
+	card->node_id = LOCAL_BUS | 0x01;
+
+	card->local_node = build_tree(card, self_id_sequence, ARRAY_SIZE(self_id_sequence), 123);
+	KUNIT_EXPECT_NOT_NULL(test, card->local_node);
+	KUNIT_EXPECT_PTR_EQ(test, card->local_node, card->root_node);
+
+	struct fw_node *node = card->root_node;
+	KUNIT_EXPECT_EQ(test, node->node_id, LOCAL_BUS | 0x01);
+	KUNIT_EXPECT_EQ(test, node->port_count, 3);
+	KUNIT_EXPECT_NULL(test, node->ports[0]);
+	KUNIT_EXPECT_NULL(test, node->ports[1]);
+	KUNIT_EXPECT_NOT_NULL(test, node->ports[2]);
+
+	struct fw_node *parent = node;
+	node = parent->ports[2];
+	KUNIT_EXPECT_EQ(test, node->node_id, LOCAL_BUS | 0x00);
+	KUNIT_EXPECT_EQ(test, node->port_count, 1);
+	KUNIT_EXPECT_PTR_EQ(test, node->ports[0], parent);
+
+	++card->color;
+	for_each_fw_node(card, card->root_node, release_fw_node);
+	KUNIT_EXPECT_EQ(test, data->release_count, 2);
+}
+
+static void node_tree_test_two_nodes_1394a(struct kunit *test)
+{
+	//         root
+	// ++===============++
+	// ||      phy 0    ||
+	// ||  P0 P1 P2 P3  ||
+	// ++===|==|==|==|==++
+	//            |
+	//         +--+
+	//         |
+	// ++===|==|==|==|==|==++
+	// ||  P0 P1 P2 P3 P4  ||
+	// ||       phy 1      ||
+	// ++==================++
+	//
+	// NOTE: Just for Self-ID Packets Zero and One.
+	static const u32 self_id_sequence[] = {
+		0x80000065, 0x80814000,
+		0x8100005d, 0x81810000,
+	};
+	struct private_data *data = test->priv;
+	struct fw_card *card = data->card;
+
+	card->node_id = LOCAL_BUS | 0x01;
+
+	card->local_node = build_tree(card, self_id_sequence, ARRAY_SIZE(self_id_sequence), 123);
+	KUNIT_EXPECT_NOT_NULL(test, card->local_node);
+	KUNIT_EXPECT_PTR_EQ(test, card->local_node, card->root_node);
+
+	struct fw_node *node = card->root_node;
+	KUNIT_EXPECT_EQ(test, node->node_id, LOCAL_BUS | 0x01);
+	KUNIT_EXPECT_EQ(test, node->port_count, 4);
+	KUNIT_EXPECT_NULL(test, node->ports[0]);
+	KUNIT_EXPECT_NULL(test, node->ports[1]);
+	KUNIT_EXPECT_NOT_NULL(test, node->ports[2]);
+	KUNIT_EXPECT_NULL(test, node->ports[3]);
+
+	struct fw_node *parent = node;
+	node = parent->ports[2];
+	KUNIT_EXPECT_EQ(test, node->node_id, LOCAL_BUS | 0x00);
+	KUNIT_EXPECT_EQ(test, node->port_count, 5);
+	KUNIT_EXPECT_NULL(test, node->ports[0]);
+	KUNIT_EXPECT_PTR_EQ(test, node->ports[1], parent);
+	KUNIT_EXPECT_NULL(test, node->ports[2]);
+	KUNIT_EXPECT_NULL(test, node->ports[3]);
+	KUNIT_EXPECT_NULL(test, node->ports[4]);
+
+	++card->color;
+	for_each_fw_node(card, card->root_node, release_fw_node);
+	KUNIT_EXPECT_EQ(test, data->release_count, 2);
+}
+
+static void node_tree_test_three_nodes_case0(struct kunit *test)
+{
+	//       root
+	// ++============++
+	// ||    phy 2   ||
+	// ||  P0 P1 P2  ||
+	// ++===|==|==|==++
+	//      |     |
+	//      +--+  +-----------------+
+	//         |                    |
+	// ++===|==|==x==++  ++===|==|==|==++
+	// ||  P0 P1 P2  ||  ||  P0 P1 P2  ||
+	// ||    phy 0   ||  ||    phy 1   ||
+	// ++============++  ++============++
+	//
+	static const u32 self_id_sequence[] = {
+		0x80000060,
+		0x81000058,
+		0x820000dc,
+	};
+	struct private_data *data = test->priv;
+	struct fw_card *card = data->card;
+
+	card->node_id = LOCAL_BUS | 0x02;
+
+	card->local_node = build_tree(card, self_id_sequence, ARRAY_SIZE(self_id_sequence), 123);
+	KUNIT_EXPECT_NOT_NULL(test, card->local_node);
+	KUNIT_EXPECT_PTR_EQ(test, card->local_node, card->root_node);
+
+	struct fw_node *node = card->root_node;
+	KUNIT_EXPECT_EQ(test, node->node_id, LOCAL_BUS | 0x02);
+	KUNIT_EXPECT_EQ(test, node->port_count, 3);
+	KUNIT_EXPECT_NOT_NULL(test, node->ports[0]);
+	KUNIT_EXPECT_NULL(test, node->ports[1]);
+	KUNIT_EXPECT_NOT_NULL(test, node->ports[2]);
+
+	struct fw_node *parent = node;
+	node = parent->ports[0];
+	KUNIT_EXPECT_EQ(test, node->node_id, LOCAL_BUS | 0x00);
+	KUNIT_EXPECT_EQ(test, node->port_count, 2);
+	KUNIT_EXPECT_NULL(test, node->ports[0]);
+	KUNIT_EXPECT_PTR_EQ(test, node->ports[1], parent);
+
+	node = parent->ports[2];
+	KUNIT_EXPECT_EQ(test, node->node_id, LOCAL_BUS | 0x01);
+	KUNIT_EXPECT_EQ(test, node->port_count, 3);
+	KUNIT_EXPECT_NULL(test, node->ports[0]);
+	KUNIT_EXPECT_NULL(test, node->ports[1]);
+	KUNIT_EXPECT_PTR_EQ(test, node->ports[2], parent);
+
+	++card->color;
+	for_each_fw_node(card, card->root_node, release_fw_node);
+	KUNIT_EXPECT_EQ(test, data->release_count, 3);
+}
+
+static void node_tree_test_three_nodes_case1(struct kunit *test)
+{
+	//       root
+	// ++============++
+	// ||    phy 2   ||
+	// ||  P0 P1 P2  ||
+	// ++===|==|==x==++
+	//         |
+	//         |  +-----------+
+	//         |  |           |
+	// ++===|==|==|==++  ++===|==x==x==++
+	// ||  P0 P1 P2  ||  ||  P0 P1 P2  ||
+	// ||    phy 1   ||  ||    phy 0   ||
+	// ++============++  ++============++
+	//
+	static const u32 self_id_sequence[] = {
+		0x80000080,
+		0x8100006c,
+		0x82000070,
+	};
+	struct private_data *data = test->priv;
+	struct fw_card *card = data->card;
+
+	card->node_id = LOCAL_BUS | 0x02;
+
+	card->local_node = build_tree(card, self_id_sequence, ARRAY_SIZE(self_id_sequence), 123);
+	KUNIT_EXPECT_NOT_NULL(test, card->local_node);
+	KUNIT_EXPECT_PTR_EQ(test, card->local_node, card->root_node);
+
+	struct fw_node *node = card->root_node;
+	KUNIT_EXPECT_EQ(test, node->node_id, LOCAL_BUS | 0x02);
+	KUNIT_EXPECT_EQ(test, node->port_count, 2);
+	KUNIT_EXPECT_NULL(test, node->ports[0]);
+	KUNIT_EXPECT_NOT_NULL(test, node->ports[1]);
+
+	struct fw_node *parent = node;
+	node = parent->ports[1];
+	KUNIT_EXPECT_EQ(test, node->node_id, LOCAL_BUS | 0x01);
+	KUNIT_EXPECT_EQ(test, node->port_count, 3);
+	KUNIT_EXPECT_NULL(test, node->ports[0]);
+	KUNIT_EXPECT_PTR_EQ(test, node->ports[1], parent);
+	KUNIT_EXPECT_NOT_NULL(test, node->ports[2]);
+
+	parent = node;
+	node = parent->ports[2];
+	KUNIT_EXPECT_EQ(test, node->node_id, LOCAL_BUS | 0x00);
+	KUNIT_EXPECT_EQ(test, node->port_count, 1);
+	KUNIT_EXPECT_PTR_EQ(test, node->ports[0], parent);
+
+	++card->color;
+	for_each_fw_node(card, card->root_node, release_fw_node);
+	KUNIT_EXPECT_EQ(test, data->release_count, 3);
+}
+
+static void node_tree_test_four_nodes_case0(struct kunit *test)
+{
+	//       root
+	// ++============++
+	// ||    phy 3   ||
+	// ||  P0 P1 P2  ||
+	// ++===|==|==|==++
+	//         |
+	//         |  +-----------+  +--------------+
+	//         |  |           |  |              |
+	// ++===|==|==|==++  ++===|==|==x==++  ++===|==x==x==++
+	// ||  P0 P1 P2  ||  ||  P0 P1 P2  ||  ||  P0 P1 P2  ||
+	// ||    phy 2   ||  ||    phy 1   ||  ||    phy 0   ||
+	// ++============++  ++============++  ++============++
+	//
+	static const u32 self_id_sequence[] = {
+		0x80000080,
+		0x810000b0,
+		0x8200006c,
+		0x83000074,
+	};
+	struct private_data *data = test->priv;
+	struct fw_card *card = data->card;
+
+	card->node_id = LOCAL_BUS | 0x03;
+
+	card->local_node = build_tree(card, self_id_sequence, ARRAY_SIZE(self_id_sequence), 123);
+	KUNIT_EXPECT_NOT_NULL(test, card->local_node);
+	KUNIT_EXPECT_PTR_EQ(test, card->local_node, card->root_node);
+
+	struct fw_node *node = card->root_node;
+	KUNIT_EXPECT_EQ(test, node->node_id, LOCAL_BUS | 0x03);
+	KUNIT_EXPECT_EQ(test, node->port_count, 3);
+	KUNIT_EXPECT_NULL(test, node->ports[0]);
+	KUNIT_EXPECT_NOT_NULL(test, node->ports[1]);
+	KUNIT_EXPECT_NULL(test, node->ports[2]);
+
+	struct fw_node *parent = node;
+	node = parent->ports[1];
+	KUNIT_EXPECT_EQ(test, node->node_id, LOCAL_BUS | 0x02);
+	KUNIT_EXPECT_EQ(test, node->port_count, 3);
+	KUNIT_EXPECT_NULL(test, node->ports[0]);
+	KUNIT_EXPECT_PTR_EQ(test, node->ports[1], parent);
+	KUNIT_EXPECT_NOT_NULL(test, node->ports[2]);
+
+	parent = node;
+	node = parent->ports[2];
+	KUNIT_EXPECT_EQ(test, node->node_id, LOCAL_BUS | 0x01);
+	KUNIT_EXPECT_EQ(test, node->port_count, 2);
+	KUNIT_EXPECT_PTR_EQ(test, node->ports[0], parent);
+	KUNIT_EXPECT_NOT_NULL(test, node->ports[1]);
+
+	parent = node;
+	node = parent->ports[1];
+	KUNIT_EXPECT_EQ(test, node->node_id, LOCAL_BUS | 0x00);
+	KUNIT_EXPECT_EQ(test, node->port_count, 1);
+	KUNIT_EXPECT_PTR_EQ(test, node->ports[0], parent);
+
+	++card->color;
+	for_each_fw_node(card, card->root_node, release_fw_node);
+	KUNIT_EXPECT_EQ(test, data->release_count, 4);
+}
+
+static void node_tree_test_four_nodes_case1(struct kunit *test)
+{
+	//       root
+	// ++============++
+	// ||    phy 3   ||
+	// ||  P0 P1 P2  ||
+	// ++===|==|==x==++
+	//      |
+	//      |  +--------------------------------+
+	//      |  |  +-----------+                 |
+	// ++===|==|==|==++  ++===|==x==x==++  ++===|==|==|==++
+	// ||  P0 P1 P2  ||  ||  P0 P1 P2  ||  ||  P0 P1 P2  ||
+	// ||    phy 2   ||  ||    phy 1   ||  ||    phy 0   ||
+	// ++============++  ++============++  ++============++
+	//
+	static const u32 self_id_sequence[] = {
+		0x80000094,
+		0x81000080,
+		0x820000bc,
+		0x830000d0,
+	};
+	struct private_data *data = test->priv;
+	struct fw_card *card = data->card;
+
+	card->node_id = LOCAL_BUS | 0x03;
+
+	card->local_node = build_tree(card, self_id_sequence, ARRAY_SIZE(self_id_sequence), 123);
+	KUNIT_EXPECT_NOT_NULL(test, card->local_node);
+	KUNIT_EXPECT_PTR_EQ(test, card->local_node, card->root_node);
+
+	struct fw_node *node = card->root_node;
+	KUNIT_EXPECT_EQ(test, node->node_id, LOCAL_BUS | 0x03);
+	KUNIT_EXPECT_EQ(test, node->port_count, 2);
+	KUNIT_EXPECT_NOT_NULL(test, node->ports[0]);
+	KUNIT_EXPECT_NULL(test, node->ports[1]);
+
+	struct fw_node *parent = node;
+	node = parent->ports[0];
+	KUNIT_EXPECT_EQ(test, node->node_id, LOCAL_BUS | 0x02);
+	KUNIT_EXPECT_EQ(test, node->port_count, 3);
+	KUNIT_EXPECT_PTR_EQ(test, node->ports[0], parent);
+	KUNIT_EXPECT_NOT_NULL(test, node->ports[1]);
+	KUNIT_EXPECT_NOT_NULL(test, node->ports[2]);
+
+	parent = node;
+	node = parent->ports[2];
+	KUNIT_EXPECT_EQ(test, node->node_id, LOCAL_BUS | 0x01);
+	KUNIT_EXPECT_EQ(test, node->port_count, 1);
+	KUNIT_EXPECT_PTR_EQ(test, node->ports[0], parent);
+
+	node = parent->ports[1];
+	KUNIT_EXPECT_EQ(test, node->node_id, LOCAL_BUS | 0x00);
+	KUNIT_EXPECT_EQ(test, node->port_count, 3);
+	KUNIT_EXPECT_PTR_EQ(test, node->ports[0], parent);
+	KUNIT_EXPECT_NULL(test, node->ports[1]);
+	KUNIT_EXPECT_NULL(test, node->ports[2]);
+
+	++card->color;
+	for_each_fw_node(card, card->root_node, release_fw_node);
+	KUNIT_EXPECT_EQ(test, data->release_count, 4);
+}
+
+static void node_tree_test_four_nodes_case2(struct kunit *test)
+{
+	//       root
+	// ++============++
+	// ||    phy 3   ||
+	// ||  P0 P1 P2  ||
+	// ++===|==|==|==++
+	//      |     |
+	//      |     +-----------------------------+
+	//      |  +--------------+                 |
+	// ++===|==|==x==++  ++===|==|==|==++  ++===|==x==x==++
+	// ||  P0 P1 P2  ||  ||  P0 P1 P2  ||  ||  P0 P1 P2  ||
+	// ||    phy 1   ||  ||    phy 0   ||  ||    phy 2   ||
+	// ++============++  ++============++  ++============++
+	//
+	static const u32 self_id_sequence[] = {
+		0x80000094,
+		0x810000b0,
+		0x82000080,
+		0x830000dc,
+	};
+	struct private_data *data = test->priv;
+	struct fw_card *card = data->card;
+
+	card->node_id = LOCAL_BUS | 0x03;
+
+	card->local_node = build_tree(card, self_id_sequence, ARRAY_SIZE(self_id_sequence), 123);
+	KUNIT_EXPECT_NOT_NULL(test, card->local_node);
+	KUNIT_EXPECT_PTR_EQ(test, card->local_node, card->root_node);
+
+	struct fw_node *node = card->root_node;
+	KUNIT_EXPECT_EQ(test, node->node_id, LOCAL_BUS | 0x03);
+	KUNIT_EXPECT_EQ(test, node->port_count, 3);
+	KUNIT_EXPECT_NOT_NULL(test, node->ports[0]);
+	KUNIT_EXPECT_NULL(test, node->ports[1]);
+	KUNIT_EXPECT_NOT_NULL(test, node->ports[2]);
+
+	struct fw_node *parent = node;
+	node = parent->ports[2];
+	KUNIT_EXPECT_EQ(test, node->node_id, LOCAL_BUS | 0x02);
+	KUNIT_EXPECT_EQ(test, node->port_count, 1);
+	KUNIT_EXPECT_PTR_EQ(test, node->ports[0], parent);
+
+	node = parent->ports[0];
+	KUNIT_EXPECT_EQ(test, node->node_id, LOCAL_BUS | 0x01);
+	KUNIT_EXPECT_EQ(test, node->port_count, 2);
+	KUNIT_EXPECT_PTR_EQ(test, node->ports[0], parent);
+	KUNIT_EXPECT_NOT_NULL(test, node->ports[1]);
+
+	parent = node;
+	node = parent->ports[1];
+	KUNIT_EXPECT_EQ(test, node->node_id, LOCAL_BUS | 0x00);
+	KUNIT_EXPECT_EQ(test, node->port_count, 3);
+	KUNIT_EXPECT_PTR_EQ(test, node->ports[0], parent);
+	KUNIT_EXPECT_NULL(test, node->ports[1]);
+	KUNIT_EXPECT_NULL(test, node->ports[2]);
+
+	++card->color;
+	for_each_fw_node(card, card->root_node, release_fw_node);
+	KUNIT_EXPECT_EQ(test, data->release_count, 4);
+}
+
+static void node_tree_test_four_nodes_case3(struct kunit *test)
+{
+	//       root
+	// ++============++
+	// ||    phy 3   ||
+	// ||  P0 P1 P2  ||
+	// ++===|==|==|==++
+	//      |  |  +--------------------------------+
+	//      |  +--------------------+              |
+	//      |                       |              |
+	// ++===|==|==x==++  ++===|==|==|==++  ++===|==|==x==++
+	// ||  P0 P1 P2  ||  ||  P0 P1 P2  ||  ||  P0 P1 P2  ||
+	// ||    phy 0   ||  ||    phy 1   ||  ||    phy 2   ||
+	// ++============++  ++============++  ++============++
+	//
+	static const u32 self_id_sequence[] = {
+		0x80000090,
+		0x81000058,
+		0x82000060,
+		0x830000fc,
+	};
+	struct private_data *data = test->priv;
+	struct fw_card *card = data->card;
+
+	card->node_id = LOCAL_BUS | 0x03;
+
+	card->local_node = build_tree(card, self_id_sequence, ARRAY_SIZE(self_id_sequence), 123);
+	KUNIT_EXPECT_NOT_NULL(test, card->local_node);
+	KUNIT_EXPECT_PTR_EQ(test, card->local_node, card->root_node);
+
+	struct fw_node *node = card->root_node;
+	KUNIT_EXPECT_EQ(test, node->node_id, LOCAL_BUS | 0x03);
+	KUNIT_EXPECT_EQ(test, node->port_count, 3);
+	KUNIT_EXPECT_NOT_NULL(test, node->ports[0]);
+	KUNIT_EXPECT_NOT_NULL(test, node->ports[1]);
+	KUNIT_EXPECT_NOT_NULL(test, node->ports[2]);
+
+	struct fw_node *parent = node;
+	node = parent->ports[2];
+	KUNIT_EXPECT_EQ(test, node->node_id, LOCAL_BUS | 0x02);
+	KUNIT_EXPECT_EQ(test, node->port_count, 2);
+	KUNIT_EXPECT_NULL(test, node->ports[0]);
+	KUNIT_EXPECT_PTR_EQ(test, node->ports[1], parent);
+
+	node = parent->ports[1];
+	KUNIT_EXPECT_EQ(test, node->node_id, LOCAL_BUS | 0x01);
+	KUNIT_EXPECT_EQ(test, node->port_count, 3);
+	KUNIT_EXPECT_NULL(test, node->ports[0]);
+	KUNIT_EXPECT_NULL(test, node->ports[1]);
+	KUNIT_EXPECT_PTR_EQ(test, node->ports[2], parent);
+
+	node = parent->ports[0];
+	KUNIT_EXPECT_EQ(test, node->node_id, LOCAL_BUS | 0x00);
+	KUNIT_EXPECT_EQ(test, node->port_count, 2);
+	KUNIT_EXPECT_PTR_EQ(test, node->ports[0], parent);
+	KUNIT_EXPECT_NULL(test, node->ports[1]);
+
+	++card->color;
+	for_each_fw_node(card, card->root_node, release_fw_node);
+	KUNIT_EXPECT_EQ(test, data->release_count, 4);
+}
 
 static struct kunit_case node_tree_test_cases[] = {
+	KUNIT_CASE(node_tree_test_two_nodes),
+	KUNIT_CASE(node_tree_test_two_nodes_1394a),
+	KUNIT_CASE(node_tree_test_three_nodes_case0),
+	KUNIT_CASE(node_tree_test_three_nodes_case1),
+	KUNIT_CASE(node_tree_test_four_nodes_case0),
+	KUNIT_CASE(node_tree_test_four_nodes_case1),
+	KUNIT_CASE(node_tree_test_four_nodes_case2),
+	KUNIT_CASE(node_tree_test_four_nodes_case3),
 	{}
 };
 
 static struct kunit_suite node_tree_test_suite = {
 	.name = "firewire-node-tree",
+	.init = node_tree_test_init,
+	.exit = node_tree_test_exit,
 	.test_cases = node_tree_test_cases,
 };
 kunit_test_suite(node_tree_test_suite);
-- 
2.53.0

[PATCH 3/3] firewire: core: add KUnit tests for failure of tree building

[Takashi Sakamoto]

Abdun Nihaal has reported a memory leak when tree building fails in the
middle of self ID sequence enumeration. This is caused by an invalid
self ID sequence and is not a common occurrence.

This commit is intended to assist in fixing the issue by adding KUnit
tests to show the cases in which the memory leak is triggered. The leak
occurs internally in the build_tree() function, therefore it cannot be
detected directly by the tests.

Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
---
 drivers/firewire/node-tree-test.c | 84 +++++++++++++++++++++++++++++++
 1 file changed, 84 insertions(+)

diff --git a/drivers/firewire/node-tree-test.c b/drivers/firewire/node-tree-test.c
index 6f404df3d50e..251c0113639f 100644
--- a/drivers/firewire/node-tree-test.c
+++ b/drivers/firewire/node-tree-test.c
@@ -506,6 +506,86 @@ static void node_tree_test_four_nodes_case3(struct kunit *test)
 	KUNIT_EXPECT_EQ(test, data->release_count, 4);
 }
 
+static void node_tree_test_invalid_extended_self_id_sequence(struct kunit *test)
+{
+	// Use the same node tree as node_tree_test_four_nodes_case1, except for the invalid
+	// content of self ID packet for the phy 3.
+	static const u32 self_id_sequence[] = {
+		0x80000094,
+		0x81000080,
+		0x820000bc,
+		0x830000d1, // Invalid.
+	};
+	struct private_data *data = test->priv;
+	struct fw_card *card = data->card;
+
+	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);
+}
+
+static void node_tree_test_invalid_phy_id(struct kunit *test)
+{
+	// Use the same node tree as node_tree_test_four_nodes_case1, except for the invalid
+	// phy ID for phy 3.
+	static const u32 self_id_sequence[] = {
+		0x80000094,
+		0x81000080,
+		0x820000bc,
+		0x8f0000d0, // Invalid.
+	};
+	struct private_data *data = test->priv;
+	struct fw_card *card = data->card;
+
+	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);
+}
+
+static void node_tree_test_invalid_child_port_count(struct kunit *test)
+{
+	// Use the same node tree as node_tree_test_four_nodes_case1, except for the invalid
+	// count of child ports for phy 3.
+	static const u32 self_id_sequence[] = {
+		0x80000094,
+		0x81000080,
+		0x820000bc,
+		0x830000fc, // Invalid.
+	};
+	struct private_data *data = test->priv;
+	struct fw_card *card = data->card;
+
+	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);
+}
+
+static void node_tree_test_invalid_parent_port_count(struct kunit *test)
+{
+	// Use the same node tree as node_tree_test_four_nodes_case1, except for the invalid
+	// count of parent ports for phy 3.
+	static const u32 self_id_sequence[] = {
+		0x80000094,
+		0x81000080,
+		0x820000bc,
+		0x830000e8, // Invalid.
+	};
+	struct private_data *data = test->priv;
+	struct fw_card *card = data->card;
+
+	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);
+}
+
 static struct kunit_case node_tree_test_cases[] = {
 	KUNIT_CASE(node_tree_test_two_nodes),
 	KUNIT_CASE(node_tree_test_two_nodes_1394a),
@@ -515,6 +595,10 @@ static struct kunit_case node_tree_test_cases[] = {
 	KUNIT_CASE(node_tree_test_four_nodes_case1),
 	KUNIT_CASE(node_tree_test_four_nodes_case2),
 	KUNIT_CASE(node_tree_test_four_nodes_case3),
+	KUNIT_CASE(node_tree_test_invalid_extended_self_id_sequence),
+	KUNIT_CASE(node_tree_test_invalid_phy_id),
+	KUNIT_CASE(node_tree_test_invalid_child_port_count),
+	KUNIT_CASE(node_tree_test_invalid_parent_port_count),
 	{}
 };
 
-- 
2.53.0

Re: [PATCH 0/3] firewire: core: add KUnit tests for tree building

[Takashi Sakamoto]

On Mon, Aug 10, 2026 at 03:41:16PM +0900, Takashi Sakamoto wrote:
> Hi,
> 
> The core function builds internal node tree by parsing self ID sequence
> at bus reset. Abdun Nihaal points to a memory leak in this process at
> encountering a failure in the middle of self ID sequence enumeration[1].
> This issue is not yet solved, and some unit tests make it easier to work
> for it, mainly to avoid some regressions when refactoring codes.
> 
> This patchset adds some KUnit tests for the purpose. Especially, the
> tests added at the last patch includes actual cases to hit the memory
> leak.
> 
> In my development environment (Ubuntu 26.04, amd64), I can see the
> memory leak by enabling and redirecting some tracepoints events to kmsg
> on the qemu process for KUnit tests execution:
> 
> ```
> $ python3 tools/testing/kunit/kunit.py run \
>   --arch=x86_64 \
>   --cross_compile=x86_64-linux-gnu- \
>   --kunitconfig=drivers/firewire/ \
>   --kconfig_add=CONFIG_FTRACE=y \
>   --kernel_args=tp_printk=1 \
>   --kernel_args=trace_event=kmem:kmalloc,kmem:kfree \
>   --kernel_args=hash_pointers=never \
>   --raw_output=all
> ```
> 
> In the KUnit tests for successful cases, the allocated memory objects
> are released successfully in the test side, like:
> 
> ```
> kmalloc: call_site=build_tree+0x203/0x5d0 ptr=ffff8e10413f0b80 bytes_req=48 bytes_alloc=64 gfp_flags=GFP_ATOMIC|__GFP_ZERO node=-1 accounted=false
> kmalloc: call_site=build_tree+0x203/0x5d0 ptr=ffff8e10413f0b40 bytes_req=64 bytes_alloc=64 gfp_flags=GFP_ATOMIC|__GFP_ZERO node=-1 accounted=false
> kfree: call_site=for_each_fw_node+0x17c/0x230 ptr=ffff8e10413f0b40
> kfree: call_site=for_each_fw_node+0x17c/0x230 ptr=ffff8e10413f0b80
> ```
> 
> For the issued cases, we can see no kfree events corresponding to the
> allocated memory objects, like: 
> 
> ```
> kmalloc: call_site=build_tree+0x203/0x5d0 ptr=ffff8e10413f0940 bytes_req=64 bytes_alloc=64 gfp_flags=GFP_ATOMIC|__GFP_ZERO node=-1 accounted=false
> kmalloc: call_site=build_tree+0x203/0x5d0 ptr=ffff8e10413f0900 bytes_req=48 bytes_alloc=64 gfp_flags=GFP_ATOMIC|__GFP_ZERO node=-1 accounted=false
> kmalloc: call_site=build_tree+0x203/0x5d0 ptr=ffff8e10413f08c0 bytes_req=64 bytes_alloc=64 gfp_flags=GFP_ATOMIC|__GFP_ZERO node=-1 accounted=false
> kmalloc: call_site=build_tree+0x203/0x5d0 ptr=ffff8e10413f0880 bytes_req=64 bytes_alloc=64 gfp_flags=GFP_ATOMIC|__GFP_ZERO node=-1 accounted=false
> firewire_core node_tree_test_invalid_parent_port_count.dummy-device: parent port inconsistency for node 3: parent_count=2
> ...
> ```
> 
> The memory objects remain in the build_tree() function and should be
> released internally. This is the addressing issue.
> 
> [1] https://lore.kernel.org/lkml/20260801110915.82561-1-nihaal@cse.iitm.ac.in/
> 
> 
> Takashi Sakamoto (3):
>   firewire: core: add KUnit test skeleton for node tree
>   firewire: core: add KUnit tests for successful tree building
>   firewire: core: add KUnit tests for failure of tree building
> 
>  drivers/firewire/.kunitconfig     |   1 +
>  drivers/firewire/Kconfig          |  15 +
>  drivers/firewire/core-topology.c  |   4 +
>  drivers/firewire/node-tree-test.c | 611 ++++++++++++++++++++++++++++++
>  4 files changed, 631 insertions(+)
>  create mode 100644 drivers/firewire/node-tree-test.c

Applied to for-next branch.


Regards

Takashi Sakamoto