net: hsr: fixes for PRP duplication and VLAN unwind
Subsystems (3)
v4 cover net: hsr: fixes for PRP duplication and VLAN unwind
From: Luka Gejak <[email protected]>
This series addresses two logic bugs in the HSR/PRP implementation identified during a protocol audit. These are targeted for the 'net' tree as they fix potential memory corruption and state inconsistency.
The primary change resolves a race condition in the node merging path by implementing address-based lock ordering. This ensures that concurrent mutations of sequence blocks do not lead to state corruption or deadlocks.
An additional fix corrects asymmetric VLAN error unwinding by implementing a centralized unwind path on slave errors.
Changes in v4:
- Split from a 4-patch series into 'net' and 'net-next' as requested.
- Added Fixes: tags to both patches to facilitate stable backporting.
- No logic changes to these patches since v3.
History of pre-separation series (v1-v3):
Changes in v3:
- addressed Felix review feedback in the VLAN add unwind fix
- removed the superfluous empty line
Changes in v2:
- picked up Reviewed-by tags on patches 1, 3 and 4
- changes in patch 2 per advice of Felix Maurer
Luka Gejak (2):
net: hsr: serialize seq_blocks merge across nodes
net: hsr: fix VLAN add unwind on slave errors
net/hsr/hsr_device.c | 32 +++++++++++++++++--------------- net/hsr/hsr_framereg.c | 38 ++++++++++++++++++++++++++++++++++++-- 2 files changed, 53 insertions(+), 17 deletions(-)
Signature · 1 line
2.53.0
PATCH v4 1/2 net: hsr: serialize seq_blocks merge across nodes
From: Luka Gejak <[email protected]>
During node merging, hsr_handle_sup_frame() walks node_curr->seq_blocks to update node_real without holding node_curr->seq_out_lock. This allows concurrent mutations from duplicate registration paths, risking inconsistent state or XArray/bitmap corruption.
Fix this by locking both nodes' seq_out_lock during the merge. To prevent ABBA deadlocks, locks are acquired in order of memory address.
--- net/hsr/hsr_framereg.c | 38 ++++++++++++++++++++++++++++++++++++-- 1 file changed, 36 insertions(+), 2 deletions(-)
net/hsr/hsr_framereg.c +36 -2
@@ -123,6 +123,40 @@ static void hsr_free_node_rcu(struct rcu_head *rn)hsr_free_node (node); } +static void hsr_lock_seq_out_pair (struct hsr_node *node_a, +struct hsr_node *node_b) +{ +if (node_a == node_b) { +spin_lock_bh (&node_a->seq_out_lock); +return ; + } + +if (node_a < node_b) { +spin_lock_bh (&node_a->seq_out_lock); +spin_lock_nested (&node_b->seq_out_lock,SINGLE_DEPTH_NESTING ); + }else { +spin_lock_bh (&node_b->seq_out_lock); +spin_lock_nested (&node_a->seq_out_lock,SINGLE_DEPTH_NESTING ); + } +} + +static void hsr_unlock_seq_out_pair (struct hsr_node *node_a, +struct hsr_node *node_b) +{ +if (node_a == node_b) { +spin_unlock_bh (&node_a->seq_out_lock); +return ; + } + +if (node_a < node_b) { +spin_unlock (&node_b->seq_out_lock); +spin_unlock_bh (&node_a->seq_out_lock); + }else { +spin_unlock (&node_a->seq_out_lock); +spin_unlock_bh (&node_b->seq_out_lock); + } +} +void hsr_del_nodes (struct list_head *node_db) {struct hsr_node *node;
@@ -432,7 +466,7 @@ void hsr_handle_sup_frame(struct hsr_frame_info *frame) }ether_addr_copy (node_real->macaddress_B, ethhdr->h_source); -spin_lock_bh (&node_real->seq_out_lock); +hsr_lock_seq_out_pair (node_real, node_curr);for (i =0 ; i <HSR_PT_PORTS ; i++) {if (!node_curr->time_in_stale[i] &&time_after (node_curr->time_in[i], node_real->time_in[i])) {
@@ -455,7 +489,7 @@ void hsr_handle_sup_frame(struct hsr_frame_info *frame) src_blk->seq_nrs[i], HSR_SEQ_BLOCK_SIZE); } } -spin_unlock_bh (&node_real->seq_out_lock); +hsr_unlock_seq_out_pair (node_real, node_curr); node_real->addr_B_port = port_rcv->type;spin_lock_bh (&hsr->list_lock);
Signature · 1 line
2.53.0
PATCH v4 2/2 net: hsr: fix VLAN add unwind on slave errors
In reply to [email protected]From: Luka Gejak <[email protected]>
When vlan_vid_add() fails for a secondary slave, the error path calls vlan_vid_del() on the failing port instead of the peer slave that had already succeeded. This results in asymmetric VLAN state across the HSR pair.
Fix this by switching to a centralized unwind path that removes the VID from any slave device that was already programmed.
--- net/hsr/hsr_device.c | 32 +++++++++++++++++--------------- 1 file changed, 17 insertions(+), 15 deletions(-)
net/hsr/hsr_device.c +17 -15
@@ -532,8 +532,8 @@ static void hsr_change_rx_flags(struct net_device *dev, int change)static int hsr_ndo_vlan_rx_add_vid (struct net_device *dev,__be16 proto,u16 vid) { -bool is_slave_a_added = false; -bool is_slave_b_added = false; +struct net_device *slave_a_dev =NULL ; +struct net_device *slave_b_dev =NULL ;struct hsr_port *port;struct hsr_priv *hsr;int ret =0 ;
@@ -549,33 +549,35 @@ static int hsr_ndo_vlan_rx_add_vid(struct net_device *dev,switch (port->type) {case HSR_PT_SLAVE_A :if (ret) { -/* clean up Slave-B */ netdev_err (dev,"add vid failed for Slave-A\n" ); -if (is_slave_b_added) -vlan_vid_del (port->dev, proto, vid); -return ret; + goto unwind; } - - is_slave_a_added = true; + slave_a_dev = port->dev;break ; -case HSR_PT_SLAVE_B :if (ret) { -/* clean up Slave-A */ netdev_err (dev,"add vid failed for Slave-B\n" ); -if (is_slave_a_added) -vlan_vid_del (port->dev, proto, vid); -return ret; + goto unwind; } - - is_slave_b_added = true; + slave_b_dev = port->dev;break ;default : +if (ret) + goto unwind;break ; } }return 0 ; + +unwind: +if (slave_a_dev) +vlan_vid_del (slave_a_dev, proto, vid); + +if (slave_b_dev) +vlan_vid_del (slave_b_dev, proto, vid); + +return ret; } static int hsr_ndo_vlan_rx_kill_vid(struct net_device *dev,
Signature · 1 line
2.53.0
This series was applied to netdev/net.git (main) by Jakub Kicinski <[email protected]>:
Hello:
This series was applied to netdev/net.git (main)
by Jakub Kicinski <[email protected]>:
Quoted 6 lines · On Wed, 1 Apr 2026 11:22:41 +0200 you wrote:
From: Luka Gejak <[email protected]>
This series addresses two logic bugs in the HSR/PRP implementation identified during a protocol audit. These are targeted for the 'net' tree as they fix potential memory corruption and state inconsistency.
Quoted 6 lines
The primary change resolves a race condition in the node merging path by implementing address-based lock ordering. This ensures that concurrent mutations of sequence blocks do not lead to state corruption or deadlocks.
[...]
Here is the summary with links:
- [net,v4,1/2] net: hsr: serialize seq_blocks merge across nodes
https://git.kernel.org/netdev/net/c/f5df2990c364 - [net,v4,2/2] net: hsr: fix VLAN add unwind on slave errors https://git.kernel.org/netdev/net/c/2e3514e63bfb
You are awesome, thank you!
Signature · 2 lines
Deet-doot-dot, I am a bot. https://korg.docs.kernel.org/patchwork/pwbot.html