diff options
author | Arthur Kepner <akepner@sgi.com> | 2006-03-21 00:26:56 -0500 |
---|---|---|
committer | David S. Miller <davem@davemloft.net> | 2006-03-21 00:26:56 -0500 |
commit | 95ed63f791656fc19e36ae68bc328e367958c76b (patch) | |
tree | dbee5227c99677c35874e0b522f6b84df262cf63 /net/core | |
parent | 4bf07ef3fd5db2df7d1899fcf9c67d2263ead2e2 (diff) |
[NET] pktgen: Fix races between control/worker threads.
There's a race in pktgen which can lead to a double
free of a pktgen_dev's skb. If a worker thread is in
the midst of doing fill_packet(), and the controlling
thread gets a "stop" message, the already freed skb
can be freed once again in pktgen_stop_device(). This
patch gives all responsibility for cleaning up a
pktgen_dev's skb to the associated worker thread.
Signed-off-by: Arthur Kepner <akepner@sgi.com>
Acked-by: Robert Olsson <Robert.Olsson@data.slu.se>
Signed-off-by: David S. Miller <davem@davemloft.net>
Diffstat (limited to 'net/core')
-rw-r--r-- | net/core/pktgen.c | 135 |
1 files changed, 110 insertions, 25 deletions
diff --git a/net/core/pktgen.c b/net/core/pktgen.c index da16f8fd1494..6586321b0187 100644 --- a/net/core/pktgen.c +++ b/net/core/pktgen.c | |||
@@ -153,7 +153,7 @@ | |||
153 | #include <asm/timex.h> | 153 | #include <asm/timex.h> |
154 | 154 | ||
155 | 155 | ||
156 | #define VERSION "pktgen v2.63: Packet Generator for packet performance testing.\n" | 156 | #define VERSION "pktgen v2.64: Packet Generator for packet performance testing.\n" |
157 | 157 | ||
158 | /* #define PG_DEBUG(a) a */ | 158 | /* #define PG_DEBUG(a) a */ |
159 | #define PG_DEBUG(a) | 159 | #define PG_DEBUG(a) |
@@ -176,7 +176,8 @@ | |||
176 | #define T_TERMINATE (1<<0) | 176 | #define T_TERMINATE (1<<0) |
177 | #define T_STOP (1<<1) /* Stop run */ | 177 | #define T_STOP (1<<1) /* Stop run */ |
178 | #define T_RUN (1<<2) /* Start run */ | 178 | #define T_RUN (1<<2) /* Start run */ |
179 | #define T_REMDEV (1<<3) /* Remove all devs */ | 179 | #define T_REMDEVALL (1<<3) /* Remove all devs */ |
180 | #define T_REMDEV (1<<4) /* Remove one dev */ | ||
180 | 181 | ||
181 | /* Locks */ | 182 | /* Locks */ |
182 | #define thread_lock() down(&pktgen_sem) | 183 | #define thread_lock() down(&pktgen_sem) |
@@ -218,6 +219,8 @@ struct pktgen_dev { | |||
218 | * we will do a random selection from within the range. | 219 | * we will do a random selection from within the range. |
219 | */ | 220 | */ |
220 | __u32 flags; | 221 | __u32 flags; |
222 | int removal_mark; /* non-zero => the device is marked for | ||
223 | * removal by worker thread */ | ||
221 | 224 | ||
222 | int min_pkt_size; /* = ETH_ZLEN; */ | 225 | int min_pkt_size; /* = ETH_ZLEN; */ |
223 | int max_pkt_size; /* = ETH_ZLEN; */ | 226 | int max_pkt_size; /* = ETH_ZLEN; */ |
@@ -481,7 +484,7 @@ static void pktgen_stop_all_threads_ifs(void); | |||
481 | static int pktgen_stop_device(struct pktgen_dev *pkt_dev); | 484 | static int pktgen_stop_device(struct pktgen_dev *pkt_dev); |
482 | static void pktgen_stop(struct pktgen_thread* t); | 485 | static void pktgen_stop(struct pktgen_thread* t); |
483 | static void pktgen_clear_counters(struct pktgen_dev *pkt_dev); | 486 | static void pktgen_clear_counters(struct pktgen_dev *pkt_dev); |
484 | static struct pktgen_dev *pktgen_NN_threads(const char* dev_name, int remove); | 487 | static int pktgen_mark_device(const char* ifname); |
485 | static unsigned int scan_ip6(const char *s,char ip[16]); | 488 | static unsigned int scan_ip6(const char *s,char ip[16]); |
486 | static unsigned int fmt_ip6(char *s,const char ip[16]); | 489 | static unsigned int fmt_ip6(char *s,const char ip[16]); |
487 | 490 | ||
@@ -1406,7 +1409,7 @@ static ssize_t pktgen_thread_write(struct file *file, | |||
1406 | 1409 | ||
1407 | if (!strcmp(name, "rem_device_all")) { | 1410 | if (!strcmp(name, "rem_device_all")) { |
1408 | thread_lock(); | 1411 | thread_lock(); |
1409 | t->control |= T_REMDEV; | 1412 | t->control |= T_REMDEVALL; |
1410 | thread_unlock(); | 1413 | thread_unlock(); |
1411 | schedule_timeout_interruptible(msecs_to_jiffies(125)); /* Propagate thread->control */ | 1414 | schedule_timeout_interruptible(msecs_to_jiffies(125)); /* Propagate thread->control */ |
1412 | ret = count; | 1415 | ret = count; |
@@ -1457,7 +1460,8 @@ static struct pktgen_dev *__pktgen_NN_threads(const char* ifname, int remove) | |||
1457 | if (pkt_dev) { | 1460 | if (pkt_dev) { |
1458 | if(remove) { | 1461 | if(remove) { |
1459 | if_lock(t); | 1462 | if_lock(t); |
1460 | pktgen_remove_device(t, pkt_dev); | 1463 | pkt_dev->removal_mark = 1; |
1464 | t->control |= T_REMDEV; | ||
1461 | if_unlock(t); | 1465 | if_unlock(t); |
1462 | } | 1466 | } |
1463 | break; | 1467 | break; |
@@ -1467,13 +1471,44 @@ static struct pktgen_dev *__pktgen_NN_threads(const char* ifname, int remove) | |||
1467 | return pkt_dev; | 1471 | return pkt_dev; |
1468 | } | 1472 | } |
1469 | 1473 | ||
1470 | static struct pktgen_dev *pktgen_NN_threads(const char* ifname, int remove) | 1474 | /* |
1475 | * mark a device for removal | ||
1476 | */ | ||
1477 | static int pktgen_mark_device(const char* ifname) | ||
1471 | { | 1478 | { |
1472 | struct pktgen_dev *pkt_dev = NULL; | 1479 | struct pktgen_dev *pkt_dev = NULL; |
1480 | const int max_tries = 10, msec_per_try = 125; | ||
1481 | int i = 0; | ||
1482 | int ret = 0; | ||
1483 | |||
1473 | thread_lock(); | 1484 | thread_lock(); |
1474 | pkt_dev = __pktgen_NN_threads(ifname, remove); | 1485 | PG_DEBUG(printk("pktgen: pktgen_mark_device marking %s for removal\n", |
1475 | thread_unlock(); | 1486 | ifname)); |
1476 | return pkt_dev; | 1487 | |
1488 | while(1) { | ||
1489 | |||
1490 | pkt_dev = __pktgen_NN_threads(ifname, REMOVE); | ||
1491 | if (pkt_dev == NULL) break; /* success */ | ||
1492 | |||
1493 | thread_unlock(); | ||
1494 | PG_DEBUG(printk("pktgen: pktgen_mark_device waiting for %s " | ||
1495 | "to disappear....\n", ifname)); | ||
1496 | schedule_timeout_interruptible(msecs_to_jiffies(msec_per_try)); | ||
1497 | thread_lock(); | ||
1498 | |||
1499 | if (++i >= max_tries) { | ||
1500 | printk("pktgen_mark_device: timed out after waiting " | ||
1501 | "%d msec for device %s to be removed\n", | ||
1502 | msec_per_try*i, ifname); | ||
1503 | ret = 1; | ||
1504 | break; | ||
1505 | } | ||
1506 | |||
1507 | } | ||
1508 | |||
1509 | thread_unlock(); | ||
1510 | |||
1511 | return ret; | ||
1477 | } | 1512 | } |
1478 | 1513 | ||
1479 | static int pktgen_device_event(struct notifier_block *unused, unsigned long event, void *ptr) | 1514 | static int pktgen_device_event(struct notifier_block *unused, unsigned long event, void *ptr) |
@@ -1493,7 +1528,7 @@ static int pktgen_device_event(struct notifier_block *unused, unsigned long even | |||
1493 | break; | 1528 | break; |
1494 | 1529 | ||
1495 | case NETDEV_UNREGISTER: | 1530 | case NETDEV_UNREGISTER: |
1496 | pktgen_NN_threads(dev->name, REMOVE); | 1531 | pktgen_mark_device(dev->name); |
1497 | break; | 1532 | break; |
1498 | }; | 1533 | }; |
1499 | 1534 | ||
@@ -2303,11 +2338,11 @@ static void pktgen_stop_all_threads_ifs(void) | |||
2303 | { | 2338 | { |
2304 | struct pktgen_thread *t = pktgen_threads; | 2339 | struct pktgen_thread *t = pktgen_threads; |
2305 | 2340 | ||
2306 | PG_DEBUG(printk("pktgen: entering pktgen_stop_all_threads.\n")); | 2341 | PG_DEBUG(printk("pktgen: entering pktgen_stop_all_threads_ifs.\n")); |
2307 | 2342 | ||
2308 | thread_lock(); | 2343 | thread_lock(); |
2309 | while(t) { | 2344 | while(t) { |
2310 | pktgen_stop(t); | 2345 | t->control |= T_STOP; |
2311 | t = t->next; | 2346 | t = t->next; |
2312 | } | 2347 | } |
2313 | thread_unlock(); | 2348 | thread_unlock(); |
@@ -2431,7 +2466,9 @@ static void show_results(struct pktgen_dev *pkt_dev, int nr_frags) | |||
2431 | 2466 | ||
2432 | static int pktgen_stop_device(struct pktgen_dev *pkt_dev) | 2467 | static int pktgen_stop_device(struct pktgen_dev *pkt_dev) |
2433 | { | 2468 | { |
2434 | 2469 | int nr_frags = pkt_dev->skb ? | |
2470 | skb_shinfo(pkt_dev->skb)->nr_frags: -1; | ||
2471 | |||
2435 | if (!pkt_dev->running) { | 2472 | if (!pkt_dev->running) { |
2436 | printk("pktgen: interface: %s is already stopped\n", pkt_dev->ifname); | 2473 | printk("pktgen: interface: %s is already stopped\n", pkt_dev->ifname); |
2437 | return -EINVAL; | 2474 | return -EINVAL; |
@@ -2440,13 +2477,8 @@ static int pktgen_stop_device(struct pktgen_dev *pkt_dev) | |||
2440 | pkt_dev->stopped_at = getCurUs(); | 2477 | pkt_dev->stopped_at = getCurUs(); |
2441 | pkt_dev->running = 0; | 2478 | pkt_dev->running = 0; |
2442 | 2479 | ||
2443 | show_results(pkt_dev, skb_shinfo(pkt_dev->skb)->nr_frags); | 2480 | show_results(pkt_dev, nr_frags); |
2444 | |||
2445 | if (pkt_dev->skb) | ||
2446 | kfree_skb(pkt_dev->skb); | ||
2447 | 2481 | ||
2448 | pkt_dev->skb = NULL; | ||
2449 | |||
2450 | return 0; | 2482 | return 0; |
2451 | } | 2483 | } |
2452 | 2484 | ||
@@ -2469,26 +2501,66 @@ static struct pktgen_dev *next_to_run(struct pktgen_thread *t ) | |||
2469 | static void pktgen_stop(struct pktgen_thread *t) { | 2501 | static void pktgen_stop(struct pktgen_thread *t) { |
2470 | struct pktgen_dev *next = NULL; | 2502 | struct pktgen_dev *next = NULL; |
2471 | 2503 | ||
2472 | PG_DEBUG(printk("pktgen: entering pktgen_stop.\n")); | 2504 | PG_DEBUG(printk("pktgen: entering pktgen_stop\n")); |
2473 | 2505 | ||
2474 | if_lock(t); | 2506 | if_lock(t); |
2475 | 2507 | ||
2476 | for(next=t->if_list; next; next=next->next) | 2508 | for(next=t->if_list; next; next=next->next) { |
2477 | pktgen_stop_device(next); | 2509 | pktgen_stop_device(next); |
2510 | if (next->skb) | ||
2511 | kfree_skb(next->skb); | ||
2512 | |||
2513 | next->skb = NULL; | ||
2514 | } | ||
2478 | 2515 | ||
2479 | if_unlock(t); | 2516 | if_unlock(t); |
2480 | } | 2517 | } |
2481 | 2518 | ||
2519 | /* | ||
2520 | * one of our devices needs to be removed - find it | ||
2521 | * and remove it | ||
2522 | */ | ||
2523 | static void pktgen_rem_one_if(struct pktgen_thread *t) | ||
2524 | { | ||
2525 | struct pktgen_dev *cur, *next = NULL; | ||
2526 | |||
2527 | PG_DEBUG(printk("pktgen: entering pktgen_rem_one_if\n")); | ||
2528 | |||
2529 | if_lock(t); | ||
2530 | |||
2531 | for(cur=t->if_list; cur; cur=next) { | ||
2532 | next = cur->next; | ||
2533 | |||
2534 | if (!cur->removal_mark) continue; | ||
2535 | |||
2536 | if (cur->skb) | ||
2537 | kfree_skb(cur->skb); | ||
2538 | cur->skb = NULL; | ||
2539 | |||
2540 | pktgen_remove_device(t, cur); | ||
2541 | |||
2542 | break; | ||
2543 | } | ||
2544 | |||
2545 | if_unlock(t); | ||
2546 | } | ||
2547 | |||
2482 | static void pktgen_rem_all_ifs(struct pktgen_thread *t) | 2548 | static void pktgen_rem_all_ifs(struct pktgen_thread *t) |
2483 | { | 2549 | { |
2484 | struct pktgen_dev *cur, *next = NULL; | 2550 | struct pktgen_dev *cur, *next = NULL; |
2485 | |||
2486 | /* Remove all devices, free mem */ | ||
2487 | 2551 | ||
2552 | /* Remove all devices, free mem */ | ||
2553 | |||
2554 | PG_DEBUG(printk("pktgen: entering pktgen_rem_all_ifs\n")); | ||
2488 | if_lock(t); | 2555 | if_lock(t); |
2489 | 2556 | ||
2490 | for(cur=t->if_list; cur; cur=next) { | 2557 | for(cur=t->if_list; cur; cur=next) { |
2491 | next = cur->next; | 2558 | next = cur->next; |
2559 | |||
2560 | if (cur->skb) | ||
2561 | kfree_skb(cur->skb); | ||
2562 | cur->skb = NULL; | ||
2563 | |||
2492 | pktgen_remove_device(t, cur); | 2564 | pktgen_remove_device(t, cur); |
2493 | } | 2565 | } |
2494 | 2566 | ||
@@ -2550,6 +2622,9 @@ static __inline__ void pktgen_xmit(struct pktgen_dev *pkt_dev) | |||
2550 | 2622 | ||
2551 | if (!netif_running(odev)) { | 2623 | if (!netif_running(odev)) { |
2552 | pktgen_stop_device(pkt_dev); | 2624 | pktgen_stop_device(pkt_dev); |
2625 | if (pkt_dev->skb) | ||
2626 | kfree_skb(pkt_dev->skb); | ||
2627 | pkt_dev->skb = NULL; | ||
2553 | goto out; | 2628 | goto out; |
2554 | } | 2629 | } |
2555 | if (need_resched()) | 2630 | if (need_resched()) |
@@ -2581,7 +2656,7 @@ static __inline__ void pktgen_xmit(struct pktgen_dev *pkt_dev) | |||
2581 | pkt_dev->clone_count = 0; /* reset counter */ | 2656 | pkt_dev->clone_count = 0; /* reset counter */ |
2582 | } | 2657 | } |
2583 | } | 2658 | } |
2584 | 2659 | ||
2585 | spin_lock_bh(&odev->xmit_lock); | 2660 | spin_lock_bh(&odev->xmit_lock); |
2586 | if (!netif_queue_stopped(odev)) { | 2661 | if (!netif_queue_stopped(odev)) { |
2587 | 2662 | ||
@@ -2644,6 +2719,9 @@ retry_now: | |||
2644 | 2719 | ||
2645 | /* Done with this */ | 2720 | /* Done with this */ |
2646 | pktgen_stop_device(pkt_dev); | 2721 | pktgen_stop_device(pkt_dev); |
2722 | if (pkt_dev->skb) | ||
2723 | kfree_skb(pkt_dev->skb); | ||
2724 | pkt_dev->skb = NULL; | ||
2647 | } | 2725 | } |
2648 | out:; | 2726 | out:; |
2649 | } | 2727 | } |
@@ -2685,6 +2763,7 @@ static void pktgen_thread_worker(struct pktgen_thread *t) | |||
2685 | t->control &= ~(T_TERMINATE); | 2763 | t->control &= ~(T_TERMINATE); |
2686 | t->control &= ~(T_RUN); | 2764 | t->control &= ~(T_RUN); |
2687 | t->control &= ~(T_STOP); | 2765 | t->control &= ~(T_STOP); |
2766 | t->control &= ~(T_REMDEVALL); | ||
2688 | t->control &= ~(T_REMDEV); | 2767 | t->control &= ~(T_REMDEV); |
2689 | 2768 | ||
2690 | t->pid = current->pid; | 2769 | t->pid = current->pid; |
@@ -2748,8 +2827,13 @@ static void pktgen_thread_worker(struct pktgen_thread *t) | |||
2748 | t->control &= ~(T_RUN); | 2827 | t->control &= ~(T_RUN); |
2749 | } | 2828 | } |
2750 | 2829 | ||
2751 | if(t->control & T_REMDEV) { | 2830 | if(t->control & T_REMDEVALL) { |
2752 | pktgen_rem_all_ifs(t); | 2831 | pktgen_rem_all_ifs(t); |
2832 | t->control &= ~(T_REMDEVALL); | ||
2833 | } | ||
2834 | |||
2835 | if(t->control & T_REMDEV) { | ||
2836 | pktgen_rem_one_if(t); | ||
2753 | t->control &= ~(T_REMDEV); | 2837 | t->control &= ~(T_REMDEV); |
2754 | } | 2838 | } |
2755 | 2839 | ||
@@ -2833,6 +2917,7 @@ static int pktgen_add_device(struct pktgen_thread *t, const char* ifname) | |||
2833 | } | 2917 | } |
2834 | memset(pkt_dev->flows, 0, MAX_CFLOWS*sizeof(struct flow_state)); | 2918 | memset(pkt_dev->flows, 0, MAX_CFLOWS*sizeof(struct flow_state)); |
2835 | 2919 | ||
2920 | pkt_dev->removal_mark = 0; | ||
2836 | pkt_dev->min_pkt_size = ETH_ZLEN; | 2921 | pkt_dev->min_pkt_size = ETH_ZLEN; |
2837 | pkt_dev->max_pkt_size = ETH_ZLEN; | 2922 | pkt_dev->max_pkt_size = ETH_ZLEN; |
2838 | pkt_dev->nfrags = 0; | 2923 | pkt_dev->nfrags = 0; |