Linux kernel mirror (for testing) git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel os linux

bonding: fix multiple 3ad mode sysfs race conditions

When bond_3ad_get_active_agg_info() is used in all show_ad_ functions
it is not protected against slave manipulation and since it walks over
the slaves and uses them, this can easily result in NULL pointer
dereference or use of freed memory. Both the new wrapper and the
internal function are exported to the bonding as they're needed in
different places.

Signed-off-by: Nikolay Aleksandrov <nikolay@redhat.com>
Signed-off-by: David S. Miller <davem@davemloft.net>

authored by

nikolay@redhat.com and committed by
David S. Miller
318debd8 5a5c5fd4

+24 -10
+17 -4
drivers/net/bonding/bond_3ad.c
··· 2360 2360 } 2361 2361 2362 2362 /** 2363 - * bond_3ad_get_active_agg_info - get information of the active aggregator 2363 + * __bond_3ad_get_active_agg_info - get information of the active aggregator 2364 2364 * @bond: bonding struct to work on 2365 2365 * @ad_info: ad_info struct to fill with the bond's info 2366 2366 * 2367 2367 * Returns: 0 on success 2368 2368 * < 0 on error 2369 2369 */ 2370 - int bond_3ad_get_active_agg_info(struct bonding *bond, struct ad_info *ad_info) 2370 + int __bond_3ad_get_active_agg_info(struct bonding *bond, 2371 + struct ad_info *ad_info) 2371 2372 { 2372 2373 struct aggregator *aggregator = NULL; 2373 2374 struct port *port; ··· 2392 2391 return -1; 2393 2392 } 2394 2393 2394 + /* Wrapper used to hold bond->lock so no slave manipulation can occur */ 2395 + int bond_3ad_get_active_agg_info(struct bonding *bond, struct ad_info *ad_info) 2396 + { 2397 + int ret; 2398 + 2399 + read_lock(&bond->lock); 2400 + ret = __bond_3ad_get_active_agg_info(bond, ad_info); 2401 + read_unlock(&bond->lock); 2402 + 2403 + return ret; 2404 + } 2405 + 2395 2406 int bond_3ad_xmit_xor(struct sk_buff *skb, struct net_device *dev) 2396 2407 { 2397 2408 struct slave *slave, *start_at; ··· 2415 2402 struct ad_info ad_info; 2416 2403 int res = 1; 2417 2404 2418 - if (bond_3ad_get_active_agg_info(bond, &ad_info)) { 2419 - pr_debug("%s: Error: bond_3ad_get_active_agg_info failed\n", 2405 + if (__bond_3ad_get_active_agg_info(bond, &ad_info)) { 2406 + pr_debug("%s: Error: __bond_3ad_get_active_agg_info failed\n", 2420 2407 dev->name); 2421 2408 goto out; 2422 2409 }
+2
drivers/net/bonding/bond_3ad.h
··· 273 273 void bond_3ad_adapter_duplex_changed(struct slave *slave); 274 274 void bond_3ad_handle_link_change(struct slave *slave, char link); 275 275 int bond_3ad_get_active_agg_info(struct bonding *bond, struct ad_info *ad_info); 276 + int __bond_3ad_get_active_agg_info(struct bonding *bond, 277 + struct ad_info *ad_info); 276 278 int bond_3ad_xmit_xor(struct sk_buff *skb, struct net_device *dev); 277 279 int bond_3ad_lacpdu_recv(const struct sk_buff *skb, struct bonding *bond, 278 280 struct slave *slave);
+1 -1
drivers/net/bonding/bond_procfs.c
··· 130 130 seq_printf(seq, "Aggregator selection policy (ad_select): %s\n", 131 131 ad_select_tbl[bond->params.ad_select].modename); 132 132 133 - if (bond_3ad_get_active_agg_info(bond, &ad_info)) { 133 + if (__bond_3ad_get_active_agg_info(bond, &ad_info)) { 134 134 seq_printf(seq, "bond %s has no active aggregator\n", 135 135 bond->dev->name); 136 136 } else {
+4 -5
drivers/net/bonding/bond_sysfs.c
··· 1319 1319 } 1320 1320 static DEVICE_ATTR(mii_status, S_IRUGO, bonding_show_mii_status, NULL); 1321 1321 1322 - 1323 1322 /* 1324 1323 * Show current 802.3ad aggregator ID. 1325 1324 */ ··· 1332 1333 if (bond->params.mode == BOND_MODE_8023AD) { 1333 1334 struct ad_info ad_info; 1334 1335 count = sprintf(buf, "%d\n", 1335 - (bond_3ad_get_active_agg_info(bond, &ad_info)) 1336 + bond_3ad_get_active_agg_info(bond, &ad_info) 1336 1337 ? 0 : ad_info.aggregator_id); 1337 1338 } 1338 1339 ··· 1354 1355 if (bond->params.mode == BOND_MODE_8023AD) { 1355 1356 struct ad_info ad_info; 1356 1357 count = sprintf(buf, "%d\n", 1357 - (bond_3ad_get_active_agg_info(bond, &ad_info)) 1358 + bond_3ad_get_active_agg_info(bond, &ad_info) 1358 1359 ? 0 : ad_info.ports); 1359 1360 } 1360 1361 ··· 1376 1377 if (bond->params.mode == BOND_MODE_8023AD) { 1377 1378 struct ad_info ad_info; 1378 1379 count = sprintf(buf, "%d\n", 1379 - (bond_3ad_get_active_agg_info(bond, &ad_info)) 1380 + bond_3ad_get_active_agg_info(bond, &ad_info) 1380 1381 ? 0 : ad_info.actor_key); 1381 1382 } 1382 1383 ··· 1398 1399 if (bond->params.mode == BOND_MODE_8023AD) { 1399 1400 struct ad_info ad_info; 1400 1401 count = sprintf(buf, "%d\n", 1401 - (bond_3ad_get_active_agg_info(bond, &ad_info)) 1402 + bond_3ad_get_active_agg_info(bond, &ad_info) 1402 1403 ? 0 : ad_info.partner_key); 1403 1404 } 1404 1405