[PATCH net] net: hsr: free learned nodes on device setup failure

[PATCH net] net: hsr: free learned nodes on device setup failure

[Xin Xie]

hsr_dev_finalize() can fail after a lower-device RX handler has
already been registered (slave A is added before the failable slave B
and interlink adds). RX handlers run in softirq regardless of the
master's state, so frames received in that window can learn dynamic
nodes into node_db, and the error unwind never releases them.

Free both owned dynamic databases in the unwind, mirroring
hsr_dellink(). proxy_node_db is provably empty on every current error
exit (only interlink RX feeds it, and the interlink add is the last
failable step) and is freed for symmetry. The order is safe:
hsr_del_port() unregisters each RX handler with synchronize_net()
before hsr_del_nodes() runs, which removes remaining entries with
list_del_rcu() and defers their release with call_rcu() for readers
already under RCU.

Fixes: 81ba6afd6e64 ("net/hsr: Switch from dev_add_pack() to netdev_rx_handler_register()")
Signed-off-by: Xin Xie <xiexinet@gmail.com>
---
 net/hsr/hsr_device.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/net/hsr/hsr_device.c b/net/hsr/hsr_device.c
index 5555b71ab19b..9c3078dd38c2 100644
--- a/net/hsr/hsr_device.c
+++ b/net/hsr/hsr_device.c
@@ -820,6 +820,8 @@ int hsr_dev_finalize(struct net_device *hsr_dev, struct net_device *slave[2],
 	hsr_del_ports(hsr);
 err_add_master:
 	hsr_del_self_node(hsr);
+	hsr_del_nodes(&hsr->node_db);
+	hsr_del_nodes(&hsr->proxy_node_db);
 
 	if (unregister)
 		unregister_netdevice(hsr_dev);
-- 
2.43.0

Re: [PATCH net] net: hsr: free learned nodes on device setup failure

[Hangbin Liu]

On Sat, Aug 08, 2026 at 01:08:14PM +0200, Xin Xie wrote:
> hsr_dev_finalize() can fail after a lower-device RX handler has
> already been registered (slave A is added before the failable slave B
> and interlink adds). RX handlers run in softirq regardless of the
> master's state, so frames received in that window can learn dynamic
> nodes into node_db, and the error unwind never releases them.
> 
> Free both owned dynamic databases in the unwind, mirroring
> hsr_dellink(). proxy_node_db is provably empty on every current error
> exit (only interlink RX feeds it, and the interlink add is the last
> failable step) and is freed for symmetry. The order is safe:
> hsr_del_port() unregisters each RX handler with synchronize_net()
> before hsr_del_nodes() runs, which removes remaining entries with
> list_del_rcu() and defers their release with call_rcu() for readers
> already under RCU.
> 
> Fixes: 81ba6afd6e64 ("net/hsr: Switch from dev_add_pack() to netdev_rx_handler_register()")

Should we use this fix tag? The proxy_node_db is added in
5055cccfc2d1 ("net: hsr: Provide RedBox support (HSR-SAN)").

Thanks
Hangbin

Re: [PATCH net] net: hsr: free learned nodes on device setup failure

[Xin Xie]

On 10/08/2026 03:31, Hangbin Liu wrote:
> Should we use this fix tag? The proxy_node_db is added in
> 5055cccfc2d1 ("net: hsr: Provide RedBox support (HSR-SAN)").
> 
> Thanks
> Hangbin

Thanks for checking. I believe 81ba6afd6e64 is the right tag.

The demonstrated leak is node_db, and the window that creates it was
born in 81ba6afd6e64: that commit moved per-device RX handler
registration into hsr_dev_finalize(), ahead of steps that could still
fail (the second handler registration, self-node allocation,
register_netdevice()), while the failure unwind never released nodes
learned through the already-live handler. Before it, reception used
the module-global dev_add_pack() handler, which could not reach an
instance until register_hsr_master() at the successful end of
finalize, so there was nothing to leak.

proxy_node_db cannot hold entries on any current finalize error
exit: it is fed only by interlink-port RX, and the interlink add is
the last failable step in finalize. On this path, the second
hsr_del_nodes() call is a harmless no-op on an empty list, keeping
the unwind symmetric with hsr_dellink().

Using 5055cccfc2d1 would instead keep the fix away from older stable
trees, where the node_db leak does exist.

-- 
Xin