hsr: Avoid double remove of a node.

[ Upstream commit 0c74d9f79ec4299365bbe803baa736ae0068179e ]

Due to the hashed-MAC optimisation one problem become visible:
hsr_handle_sup_frame() walks over the list of available nodes and merges
two node entries into one if based on the information in the supervision
both MAC addresses belong to one node. The list-walk happens on a RCU
protected list and delete operation happens under a lock.

If the supervision arrives on both slave interfaces at the same time
then this delete operation can occur simultaneously on two CPUs. The
result is the first-CPU deletes the from the list and the second CPUs
BUGs while attempting to dereference a poisoned list-entry. This happens
more likely with the optimisation because a new node for the mac_B entry
is created once a packet has been received and removed (merged) once the
supervision frame has been received.

Avoid removing/ cleaning up a hsr_node twice by adding a `removed' field
which is set to true after the removal and checked before the removal.

Fixes: 177416896e22e ("net/hsr: Better frame dispatch")
Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>

diff --git a/net/hsr/hsr_framereg.c b/net/hsr/hsr_framereg.c
index 4a9200729a326f1ba6cbca4382f332398aec273b..783e741491ec3c20a8479e60093d06e5f59dc8ed 100644 (file)
--- a/net/hsr/hsr_framereg.c
+++ b/net/hsr/hsr_framereg.c
@@ -269,9 +269,12 @@ void hsr_handle_sup_frame(struct sk_buff *skb, struct hsr_node *node_curr,
        node_real->addr_B_port = port_rcv->type;
 
        spin_lock_bh(&hsr->list_lock);
-       list_del_rcu(&node_curr->mac_list);
+       if (!node_curr->removed) {
+               list_del_rcu(&node_curr->mac_list);
+               node_curr->removed = true;
+               kfree_rcu(node_curr, rcu_head);
+       }
        spin_unlock_bh(&hsr->list_lock);
-       kfree_rcu(node_curr, rcu_head);
 
 done:
        skb_push(skb, sizeof(struct hsrv1_ethhdr_sp));
@@ -436,9 +439,12 @@ void hsr_prune_nodes(struct timer_list *t)
                if (time_is_before_jiffies(timestamp +
                                msecs_to_jiffies(HSR_NODE_FORGET_TIME))) {
                        hsr_nl_nodedown(hsr, node->macaddress_A);
-                       list_del_rcu(&node->mac_list);
-                       /* Note that we need to free this entry later: */
-                       kfree_rcu(node, rcu_head);
+                       if (!node->removed) {
+                               list_del_rcu(&node->mac_list);
+                               node->removed = true;
+                               /* Note that we need to free this entry later: */
+                               kfree_rcu(node, rcu_head);
+                       }
                }
        }
        spin_unlock_bh(&hsr->list_lock);

diff --git a/net/hsr/hsr_framereg.h b/net/hsr/hsr_framereg.h
index 0f0fa12b432937fdae50dc2f57953fffe238c20b..01f4ef4ae494bfe05c1c4fee1fc26565a089913c 100644 (file)
--- a/net/hsr/hsr_framereg.h
+++ b/net/hsr/hsr_framereg.h
@@ -56,6 +56,7 @@ struct hsr_node {
        unsigned long           time_in[HSR_PT_PORTS];
        bool                    time_in_stale[HSR_PT_PORTS];
        u16                     seq_out[HSR_PT_PORTS];
+       bool                    removed;
        struct rcu_head         rcu_head;
 };