batman-adv: Add reference counting + nullptr fixes
[feed/routing.git] / batman-adv / patches / 0005-batman-adv-Fix-non-atomic-bla_claim-backbone_gw-acce.patch
1 From: Sven Eckelmann <sven@narfation.org>
2 Date: Fri, 1 Jul 2016 15:49:43 +0200
3 Subject: [PATCH] batman-adv: Fix non-atomic bla_claim::backbone_gw access
4
5 The pointer batadv_bla_claim::backbone_gw can be changed at any time.
6 Therefore, access to it must be protected to ensure that two function
7 accessing the same backbone_gw are actually accessing the same. This is
8 especially important when the crc_lock is used or when the backbone_gw of a
9 claim is exchanged.
10
11 Not doing so leads to invalid memory access and/or reference leaks.
12
13 Fixes: a9ce0dc43e2c ("batman-adv: add basic bridge loop avoidance code")
14 Fixes: b307e72d119f ("batman-adv: lock crc access in bridge loop avoidance")
15 Signed-off-by: Sven Eckelmann <sven@narfation.org>
16 Acked-by: Simon Wunderlich <sw@simonwunderlich.de>
17 Signed-off-by: Marek Lindner <mareklindner@neomailbox.ch>
18
19 Origin: upstream, https://git.open-mesh.org/batman-adv.git/commit/e401297e3a393896e9b07bef8d6e2df203b60d43
20 ---
21 net/batman-adv/bridge_loop_avoidance.c | 111 ++++++++++++++++++++++++++-------
22 net/batman-adv/types.h | 2 +
23 2 files changed, 90 insertions(+), 23 deletions(-)
24
25 diff --git a/net/batman-adv/bridge_loop_avoidance.c b/net/batman-adv/bridge_loop_avoidance.c
26 index 7129780..825a5cd 100644
27 --- a/net/batman-adv/bridge_loop_avoidance.c
28 +++ b/net/batman-adv/bridge_loop_avoidance.c
29 @@ -177,10 +177,21 @@ static void batadv_backbone_gw_put(struct batadv_bla_backbone_gw *backbone_gw)
30 static void batadv_claim_release(struct kref *ref)
31 {
32 struct batadv_bla_claim *claim;
33 + struct batadv_bla_backbone_gw *old_backbone_gw;
34
35 claim = container_of(ref, struct batadv_bla_claim, refcount);
36
37 - batadv_backbone_gw_put(claim->backbone_gw);
38 + spin_lock_bh(&claim->backbone_lock);
39 + old_backbone_gw = claim->backbone_gw;
40 + claim->backbone_gw = NULL;
41 + spin_unlock_bh(&claim->backbone_lock);
42 +
43 + spin_lock_bh(&old_backbone_gw->crc_lock);
44 + old_backbone_gw->crc ^= crc16(0, claim->addr, ETH_ALEN);
45 + spin_unlock_bh(&old_backbone_gw->crc_lock);
46 +
47 + batadv_backbone_gw_put(old_backbone_gw);
48 +
49 kfree_rcu(claim, rcu);
50 }
51
52 @@ -677,8 +688,10 @@ static void batadv_bla_add_claim(struct batadv_priv *bat_priv,
53 const u8 *mac, const unsigned short vid,
54 struct batadv_bla_backbone_gw *backbone_gw)
55 {
56 + struct batadv_bla_backbone_gw *old_backbone_gw;
57 struct batadv_bla_claim *claim;
58 struct batadv_bla_claim search_claim;
59 + bool remove_crc = false;
60 int hash_added;
61
62 ether_addr_copy(search_claim.addr, mac);
63 @@ -692,8 +705,10 @@ static void batadv_bla_add_claim(struct batadv_priv *bat_priv,
64 return;
65
66 ether_addr_copy(claim->addr, mac);
67 + spin_lock_init(&claim->backbone_lock);
68 claim->vid = vid;
69 claim->lasttime = jiffies;
70 + kref_get(&backbone_gw->refcount);
71 claim->backbone_gw = backbone_gw;
72
73 kref_init(&claim->refcount);
74 @@ -721,15 +736,26 @@ static void batadv_bla_add_claim(struct batadv_priv *bat_priv,
75 "bla_add_claim(): changing ownership for %pM, vid %d\n",
76 mac, BATADV_PRINT_VID(vid));
77
78 - spin_lock_bh(&claim->backbone_gw->crc_lock);
79 - claim->backbone_gw->crc ^= crc16(0, claim->addr, ETH_ALEN);
80 - spin_unlock_bh(&claim->backbone_gw->crc_lock);
81 - batadv_backbone_gw_put(claim->backbone_gw);
82 + remove_crc = true;
83 }
84 - /* set (new) backbone gw */
85 +
86 + /* replace backbone_gw atomically and adjust reference counters */
87 + spin_lock_bh(&claim->backbone_lock);
88 + old_backbone_gw = claim->backbone_gw;
89 kref_get(&backbone_gw->refcount);
90 claim->backbone_gw = backbone_gw;
91 + spin_unlock_bh(&claim->backbone_lock);
92
93 + if (remove_crc) {
94 + /* remove claim address from old backbone_gw */
95 + spin_lock_bh(&old_backbone_gw->crc_lock);
96 + old_backbone_gw->crc ^= crc16(0, claim->addr, ETH_ALEN);
97 + spin_unlock_bh(&old_backbone_gw->crc_lock);
98 + }
99 +
100 + batadv_backbone_gw_put(old_backbone_gw);
101 +
102 + /* add claim address to new backbone_gw */
103 spin_lock_bh(&backbone_gw->crc_lock);
104 backbone_gw->crc ^= crc16(0, claim->addr, ETH_ALEN);
105 spin_unlock_bh(&backbone_gw->crc_lock);
106 @@ -740,6 +766,26 @@ claim_free_ref:
107 }
108
109 /**
110 + * batadv_bla_claim_get_backbone_gw - Get valid reference for backbone_gw of
111 + * claim
112 + * @claim: claim whose backbone_gw should be returned
113 + *
114 + * Return: valid reference to claim::backbone_gw
115 + */
116 +static struct batadv_bla_backbone_gw *
117 +batadv_bla_claim_get_backbone_gw(struct batadv_bla_claim *claim)
118 +{
119 + struct batadv_bla_backbone_gw *backbone_gw;
120 +
121 + spin_lock_bh(&claim->backbone_lock);
122 + backbone_gw = claim->backbone_gw;
123 + kref_get(&backbone_gw->refcount);
124 + spin_unlock_bh(&claim->backbone_lock);
125 +
126 + return backbone_gw;
127 +}
128 +
129 +/**
130 * batadv_bla_del_claim - delete a claim from the claim hash
131 * @bat_priv: the bat priv with all the soft interface information
132 * @mac: mac address of the claim to be removed
133 @@ -763,10 +809,6 @@ static void batadv_bla_del_claim(struct batadv_priv *bat_priv,
134 batadv_choose_claim, claim);
135 batadv_claim_put(claim); /* reference from the hash is gone */
136
137 - spin_lock_bh(&claim->backbone_gw->crc_lock);
138 - claim->backbone_gw->crc ^= crc16(0, claim->addr, ETH_ALEN);
139 - spin_unlock_bh(&claim->backbone_gw->crc_lock);
140 -
141 /* don't need the reference from hash_find() anymore */
142 batadv_claim_put(claim);
143 }
144 @@ -1219,6 +1261,7 @@ static void batadv_bla_purge_claims(struct batadv_priv *bat_priv,
145 struct batadv_hard_iface *primary_if,
146 int now)
147 {
148 + struct batadv_bla_backbone_gw *backbone_gw;
149 struct batadv_bla_claim *claim;
150 struct hlist_head *head;
151 struct batadv_hashtable *hash;
152 @@ -1233,14 +1276,17 @@ static void batadv_bla_purge_claims(struct batadv_priv *bat_priv,
153
154 rcu_read_lock();
155 hlist_for_each_entry_rcu(claim, head, hash_entry) {
156 + backbone_gw = batadv_bla_claim_get_backbone_gw(claim);
157 if (now)
158 goto purge_now;
159 - if (!batadv_compare_eth(claim->backbone_gw->orig,
160 +
161 + if (!batadv_compare_eth(backbone_gw->orig,
162 primary_if->net_dev->dev_addr))
163 - continue;
164 + goto skip;
165 +
166 if (!batadv_has_timed_out(claim->lasttime,
167 BATADV_BLA_CLAIM_TIMEOUT))
168 - continue;
169 + goto skip;
170
171 batadv_dbg(BATADV_DBG_BLA, bat_priv,
172 "bla_purge_claims(): %pM, vid %d, time out\n",
173 @@ -1248,8 +1294,10 @@ static void batadv_bla_purge_claims(struct batadv_priv *bat_priv,
174
175 purge_now:
176 batadv_handle_unclaim(bat_priv, primary_if,
177 - claim->backbone_gw->orig,
178 + backbone_gw->orig,
179 claim->addr, claim->vid);
180 +skip:
181 + batadv_backbone_gw_put(backbone_gw);
182 }
183 rcu_read_unlock();
184 }
185 @@ -1760,9 +1808,11 @@ batadv_bla_loopdetect_check(struct batadv_priv *bat_priv, struct sk_buff *skb,
186 bool batadv_bla_rx(struct batadv_priv *bat_priv, struct sk_buff *skb,
187 unsigned short vid, bool is_bcast)
188 {
189 + struct batadv_bla_backbone_gw *backbone_gw;
190 struct ethhdr *ethhdr;
191 struct batadv_bla_claim search_claim, *claim = NULL;
192 struct batadv_hard_iface *primary_if;
193 + bool own_claim;
194 bool ret;
195
196 ethhdr = eth_hdr(skb);
197 @@ -1797,8 +1847,12 @@ bool batadv_bla_rx(struct batadv_priv *bat_priv, struct sk_buff *skb,
198 }
199
200 /* if it is our own claim ... */
201 - if (batadv_compare_eth(claim->backbone_gw->orig,
202 - primary_if->net_dev->dev_addr)) {
203 + backbone_gw = batadv_bla_claim_get_backbone_gw(claim);
204 + own_claim = batadv_compare_eth(backbone_gw->orig,
205 + primary_if->net_dev->dev_addr);
206 + batadv_backbone_gw_put(backbone_gw);
207 +
208 + if (own_claim) {
209 /* ... allow it in any case */
210 claim->lasttime = jiffies;
211 goto allow;
212 @@ -1862,7 +1916,9 @@ bool batadv_bla_tx(struct batadv_priv *bat_priv, struct sk_buff *skb,
213 {
214 struct ethhdr *ethhdr;
215 struct batadv_bla_claim search_claim, *claim = NULL;
216 + struct batadv_bla_backbone_gw *backbone_gw;
217 struct batadv_hard_iface *primary_if;
218 + bool client_roamed;
219 bool ret = false;
220
221 primary_if = batadv_primary_if_get_selected(bat_priv);
222 @@ -1892,8 +1948,12 @@ bool batadv_bla_tx(struct batadv_priv *bat_priv, struct sk_buff *skb,
223 goto allow;
224
225 /* check if we are responsible. */
226 - if (batadv_compare_eth(claim->backbone_gw->orig,
227 - primary_if->net_dev->dev_addr)) {
228 + backbone_gw = batadv_bla_claim_get_backbone_gw(claim);
229 + client_roamed = batadv_compare_eth(backbone_gw->orig,
230 + primary_if->net_dev->dev_addr);
231 + batadv_backbone_gw_put(backbone_gw);
232 +
233 + if (client_roamed) {
234 /* if yes, the client has roamed and we have
235 * to unclaim it.
236 */
237 @@ -1941,6 +2001,7 @@ int batadv_bla_claim_table_seq_print_text(struct seq_file *seq, void *offset)
238 struct net_device *net_dev = (struct net_device *)seq->private;
239 struct batadv_priv *bat_priv = netdev_priv(net_dev);
240 struct batadv_hashtable *hash = bat_priv->bla.claim_hash;
241 + struct batadv_bla_backbone_gw *backbone_gw;
242 struct batadv_bla_claim *claim;
243 struct batadv_hard_iface *primary_if;
244 struct hlist_head *head;
245 @@ -1965,17 +2026,21 @@ int batadv_bla_claim_table_seq_print_text(struct seq_file *seq, void *offset)
246
247 rcu_read_lock();
248 hlist_for_each_entry_rcu(claim, head, hash_entry) {
249 - is_own = batadv_compare_eth(claim->backbone_gw->orig,
250 + backbone_gw = batadv_bla_claim_get_backbone_gw(claim);
251 +
252 + is_own = batadv_compare_eth(backbone_gw->orig,
253 primary_addr);
254
255 - spin_lock_bh(&claim->backbone_gw->crc_lock);
256 - backbone_crc = claim->backbone_gw->crc;
257 - spin_unlock_bh(&claim->backbone_gw->crc_lock);
258 + spin_lock_bh(&backbone_gw->crc_lock);
259 + backbone_crc = backbone_gw->crc;
260 + spin_unlock_bh(&backbone_gw->crc_lock);
261 seq_printf(seq, " * %pM on %5d by %pM [%c] (%#.4x)\n",
262 claim->addr, BATADV_PRINT_VID(claim->vid),
263 - claim->backbone_gw->orig,
264 + backbone_gw->orig,
265 (is_own ? 'x' : ' '),
266 backbone_crc);
267 +
268 + batadv_backbone_gw_put(backbone_gw);
269 }
270 rcu_read_unlock();
271 }
272 diff --git a/net/batman-adv/types.h b/net/batman-adv/types.h
273 index ba846b0..0051222 100644
274 --- a/net/batman-adv/types.h
275 +++ b/net/batman-adv/types.h
276 @@ -1042,6 +1042,7 @@ struct batadv_bla_backbone_gw {
277 * @addr: mac address of claimed non-mesh client
278 * @vid: vlan id this client was detected on
279 * @backbone_gw: pointer to backbone gw claiming this client
280 + * @backbone_lock: lock protecting backbone_gw pointer
281 * @lasttime: last time we heard of claim (locals only)
282 * @hash_entry: hlist node for batadv_priv_bla::claim_hash
283 * @refcount: number of contexts the object is used
284 @@ -1051,6 +1052,7 @@ struct batadv_bla_claim {
285 u8 addr[ETH_ALEN];
286 unsigned short vid;
287 struct batadv_bla_backbone_gw *backbone_gw;
288 + spinlock_t backbone_lock; /* protects backbone_gw */
289 unsigned long lasttime;
290 struct hlist_node hash_entry;
291 struct rcu_head rcu;