Commit 5a1dd8a4 authored by Simon Wunderlich's avatar Simon Wunderlich Committed by Antonio Quartulli

batman-adv: lock crc access in bridge loop avoidance

We have found some networks in which nodes were constantly requesting
other nodes BLA claim tables to synchronize, just to ask for that again
once completed. The reason was that the crc checksum of the asked nodes
were out of sync due to missing locking and multiple writes to the same
crc checksum when adding/removing entries. Therefore the asked nodes
constantly reported the wrong crc, which caused repeating requests.

To avoid multiple functions changing a backbone gateways crc entry at
the same time, lock it using a spinlock.
Signed-off-by: default avatarSimon Wunderlich <sw@simonwunderlich.de>
Tested-by: default avatarAlfons Name <AlfonsName@web.de>
Signed-off-by: default avatarMarek Lindner <mareklindner@neomailbox.ch>
Signed-off-by: default avatarAntonio Quartulli <antonio@meshcoding.com>
parent c05a57f6
...@@ -260,7 +260,9 @@ batadv_bla_del_backbone_claims(struct batadv_bla_backbone_gw *backbone_gw) ...@@ -260,7 +260,9 @@ batadv_bla_del_backbone_claims(struct batadv_bla_backbone_gw *backbone_gw)
} }
/* all claims gone, initialize CRC */ /* all claims gone, initialize CRC */
spin_lock_bh(&backbone_gw->crc_lock);
backbone_gw->crc = BATADV_BLA_CRC_INIT; backbone_gw->crc = BATADV_BLA_CRC_INIT;
spin_unlock_bh(&backbone_gw->crc_lock);
} }
/** /**
...@@ -408,6 +410,7 @@ batadv_bla_get_backbone_gw(struct batadv_priv *bat_priv, u8 *orig, ...@@ -408,6 +410,7 @@ batadv_bla_get_backbone_gw(struct batadv_priv *bat_priv, u8 *orig,
entry->lasttime = jiffies; entry->lasttime = jiffies;
entry->crc = BATADV_BLA_CRC_INIT; entry->crc = BATADV_BLA_CRC_INIT;
entry->bat_priv = bat_priv; entry->bat_priv = bat_priv;
spin_lock_init(&entry->crc_lock);
atomic_set(&entry->request_sent, 0); atomic_set(&entry->request_sent, 0);
atomic_set(&entry->wait_periods, 0); atomic_set(&entry->wait_periods, 0);
ether_addr_copy(entry->orig, orig); ether_addr_copy(entry->orig, orig);
...@@ -557,7 +560,9 @@ static void batadv_bla_send_announce(struct batadv_priv *bat_priv, ...@@ -557,7 +560,9 @@ static void batadv_bla_send_announce(struct batadv_priv *bat_priv,
__be16 crc; __be16 crc;
memcpy(mac, batadv_announce_mac, 4); memcpy(mac, batadv_announce_mac, 4);
spin_lock_bh(&backbone_gw->crc_lock);
crc = htons(backbone_gw->crc); crc = htons(backbone_gw->crc);
spin_unlock_bh(&backbone_gw->crc_lock);
memcpy(&mac[4], &crc, 2); memcpy(&mac[4], &crc, 2);
batadv_bla_send_claim(bat_priv, mac, backbone_gw->vid, batadv_bla_send_claim(bat_priv, mac, backbone_gw->vid,
...@@ -618,14 +623,18 @@ static void batadv_bla_add_claim(struct batadv_priv *bat_priv, ...@@ -618,14 +623,18 @@ static void batadv_bla_add_claim(struct batadv_priv *bat_priv,
"bla_add_claim(): changing ownership for %pM, vid %d\n", "bla_add_claim(): changing ownership for %pM, vid %d\n",
mac, BATADV_PRINT_VID(vid)); mac, BATADV_PRINT_VID(vid));
spin_lock_bh(&claim->backbone_gw->crc_lock);
claim->backbone_gw->crc ^= crc16(0, claim->addr, ETH_ALEN); claim->backbone_gw->crc ^= crc16(0, claim->addr, ETH_ALEN);
spin_unlock_bh(&claim->backbone_gw->crc_lock);
batadv_backbone_gw_free_ref(claim->backbone_gw); batadv_backbone_gw_free_ref(claim->backbone_gw);
} }
/* set (new) backbone gw */ /* set (new) backbone gw */
atomic_inc(&backbone_gw->refcount); atomic_inc(&backbone_gw->refcount);
claim->backbone_gw = backbone_gw; claim->backbone_gw = backbone_gw;
spin_lock_bh(&backbone_gw->crc_lock);
backbone_gw->crc ^= crc16(0, claim->addr, ETH_ALEN); backbone_gw->crc ^= crc16(0, claim->addr, ETH_ALEN);
spin_unlock_bh(&backbone_gw->crc_lock);
backbone_gw->lasttime = jiffies; backbone_gw->lasttime = jiffies;
claim_free_ref: claim_free_ref:
...@@ -653,7 +662,9 @@ static void batadv_bla_del_claim(struct batadv_priv *bat_priv, ...@@ -653,7 +662,9 @@ static void batadv_bla_del_claim(struct batadv_priv *bat_priv,
batadv_choose_claim, claim); batadv_choose_claim, claim);
batadv_claim_free_ref(claim); /* reference from the hash is gone */ batadv_claim_free_ref(claim); /* reference from the hash is gone */
spin_lock_bh(&claim->backbone_gw->crc_lock);
claim->backbone_gw->crc ^= crc16(0, claim->addr, ETH_ALEN); claim->backbone_gw->crc ^= crc16(0, claim->addr, ETH_ALEN);
spin_unlock_bh(&claim->backbone_gw->crc_lock);
/* don't need the reference from hash_find() anymore */ /* don't need the reference from hash_find() anymore */
batadv_claim_free_ref(claim); batadv_claim_free_ref(claim);
...@@ -664,7 +675,7 @@ static int batadv_handle_announce(struct batadv_priv *bat_priv, u8 *an_addr, ...@@ -664,7 +675,7 @@ static int batadv_handle_announce(struct batadv_priv *bat_priv, u8 *an_addr,
u8 *backbone_addr, unsigned short vid) u8 *backbone_addr, unsigned short vid)
{ {
struct batadv_bla_backbone_gw *backbone_gw; struct batadv_bla_backbone_gw *backbone_gw;
u16 crc; u16 backbone_crc, crc;
if (memcmp(an_addr, batadv_announce_mac, 4) != 0) if (memcmp(an_addr, batadv_announce_mac, 4) != 0)
return 0; return 0;
...@@ -683,12 +694,16 @@ static int batadv_handle_announce(struct batadv_priv *bat_priv, u8 *an_addr, ...@@ -683,12 +694,16 @@ static int batadv_handle_announce(struct batadv_priv *bat_priv, u8 *an_addr,
"handle_announce(): ANNOUNCE vid %d (sent by %pM)... CRC = %#.4x\n", "handle_announce(): ANNOUNCE vid %d (sent by %pM)... CRC = %#.4x\n",
BATADV_PRINT_VID(vid), backbone_gw->orig, crc); BATADV_PRINT_VID(vid), backbone_gw->orig, crc);
if (backbone_gw->crc != crc) { spin_lock_bh(&backbone_gw->crc_lock);
backbone_crc = backbone_gw->crc;
spin_unlock_bh(&backbone_gw->crc_lock);
if (backbone_crc != crc) {
batadv_dbg(BATADV_DBG_BLA, backbone_gw->bat_priv, batadv_dbg(BATADV_DBG_BLA, backbone_gw->bat_priv,
"handle_announce(): CRC FAILED for %pM/%d (my = %#.4x, sent = %#.4x)\n", "handle_announce(): CRC FAILED for %pM/%d (my = %#.4x, sent = %#.4x)\n",
backbone_gw->orig, backbone_gw->orig,
BATADV_PRINT_VID(backbone_gw->vid), BATADV_PRINT_VID(backbone_gw->vid),
backbone_gw->crc, crc); backbone_crc, crc);
batadv_bla_send_request(backbone_gw); batadv_bla_send_request(backbone_gw);
} else { } else {
...@@ -1647,6 +1662,7 @@ int batadv_bla_claim_table_seq_print_text(struct seq_file *seq, void *offset) ...@@ -1647,6 +1662,7 @@ int batadv_bla_claim_table_seq_print_text(struct seq_file *seq, void *offset)
struct batadv_bla_claim *claim; struct batadv_bla_claim *claim;
struct batadv_hard_iface *primary_if; struct batadv_hard_iface *primary_if;
struct hlist_head *head; struct hlist_head *head;
u16 backbone_crc;
u32 i; u32 i;
bool is_own; bool is_own;
u8 *primary_addr; u8 *primary_addr;
...@@ -1669,11 +1685,15 @@ int batadv_bla_claim_table_seq_print_text(struct seq_file *seq, void *offset) ...@@ -1669,11 +1685,15 @@ int batadv_bla_claim_table_seq_print_text(struct seq_file *seq, void *offset)
hlist_for_each_entry_rcu(claim, head, hash_entry) { hlist_for_each_entry_rcu(claim, head, hash_entry) {
is_own = batadv_compare_eth(claim->backbone_gw->orig, is_own = batadv_compare_eth(claim->backbone_gw->orig,
primary_addr); primary_addr);
spin_lock_bh(&claim->backbone_gw->crc_lock);
backbone_crc = claim->backbone_gw->crc;
spin_unlock_bh(&claim->backbone_gw->crc_lock);
seq_printf(seq, " * %pM on %5d by %pM [%c] (%#.4x)\n", seq_printf(seq, " * %pM on %5d by %pM [%c] (%#.4x)\n",
claim->addr, BATADV_PRINT_VID(claim->vid), claim->addr, BATADV_PRINT_VID(claim->vid),
claim->backbone_gw->orig, claim->backbone_gw->orig,
(is_own ? 'x' : ' '), (is_own ? 'x' : ' '),
claim->backbone_gw->crc); backbone_crc);
} }
rcu_read_unlock(); rcu_read_unlock();
} }
...@@ -1692,6 +1712,7 @@ int batadv_bla_backbone_table_seq_print_text(struct seq_file *seq, void *offset) ...@@ -1692,6 +1712,7 @@ int batadv_bla_backbone_table_seq_print_text(struct seq_file *seq, void *offset)
struct batadv_hard_iface *primary_if; struct batadv_hard_iface *primary_if;
struct hlist_head *head; struct hlist_head *head;
int secs, msecs; int secs, msecs;
u16 backbone_crc;
u32 i; u32 i;
bool is_own; bool is_own;
u8 *primary_addr; u8 *primary_addr;
...@@ -1722,10 +1743,14 @@ int batadv_bla_backbone_table_seq_print_text(struct seq_file *seq, void *offset) ...@@ -1722,10 +1743,14 @@ int batadv_bla_backbone_table_seq_print_text(struct seq_file *seq, void *offset)
if (is_own) if (is_own)
continue; continue;
spin_lock_bh(&backbone_gw->crc_lock);
backbone_crc = backbone_gw->crc;
spin_unlock_bh(&backbone_gw->crc_lock);
seq_printf(seq, " * %pM on %5d %4i.%03is (%#.4x)\n", seq_printf(seq, " * %pM on %5d %4i.%03is (%#.4x)\n",
backbone_gw->orig, backbone_gw->orig,
BATADV_PRINT_VID(backbone_gw->vid), secs, BATADV_PRINT_VID(backbone_gw->vid), secs,
msecs, backbone_gw->crc); msecs, backbone_crc);
} }
rcu_read_unlock(); rcu_read_unlock();
} }
......
...@@ -906,6 +906,7 @@ struct batadv_socket_packet { ...@@ -906,6 +906,7 @@ struct batadv_socket_packet {
* backbone gateway - no bcast traffic is formwared until the situation was * backbone gateway - no bcast traffic is formwared until the situation was
* resolved * resolved
* @crc: crc16 checksum over all claims * @crc: crc16 checksum over all claims
* @crc_lock: lock protecting crc
* @refcount: number of contexts the object is used * @refcount: number of contexts the object is used
* @rcu: struct used for freeing in an RCU-safe manner * @rcu: struct used for freeing in an RCU-safe manner
*/ */
...@@ -919,6 +920,7 @@ struct batadv_bla_backbone_gw { ...@@ -919,6 +920,7 @@ struct batadv_bla_backbone_gw {
atomic_t wait_periods; atomic_t wait_periods;
atomic_t request_sent; atomic_t request_sent;
u16 crc; u16 crc;
spinlock_t crc_lock; /* protects crc */
atomic_t refcount; atomic_t refcount;
struct rcu_head rcu; struct rcu_head rcu;
}; };
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment