From e208c8f94392286aaf77c6d8f2a8b4d22fa3f1d7 Mon Sep 17 00:00:00 2001 From: Donald Sharp Date: Mon, 5 Feb 2018 10:40:09 -0500 Subject: [PATCH] bgpd, lib, zebra: Switch to work_queue_free_and_null The work_queue_free function free'd up the wq pointer but did not set it too NULL. This of course causes situations where we may use the work_queue after it is freed. Let's modify the work_queue to set the pointer for you. Signed-off-by: Donald Sharp --- bgpd/bgpd.c | 12 ++++-------- bgpd/rfapi/rfapi_import.c | 2 +- bgpd/rfapi/rfapi_rib.c | 6 ++---- lib/workqueue.c | 8 +++++++- lib/workqueue.h | 18 ++++++++++++++++-- zebra/main.c | 4 ++-- 6 files changed, 32 insertions(+), 18 deletions(-) diff --git a/bgpd/bgpd.c b/bgpd/bgpd.c index 6034576935..fde72da4c8 100644 --- a/bgpd/bgpd.c +++ b/bgpd/bgpd.c @@ -1079,10 +1079,8 @@ static void peer_free(struct peer *peer) XFREE(MTYPE_TMP, peer->notify.data); memset(&peer->notify, 0, sizeof(struct bgp_notify)); - if (peer->clear_node_queue) { - work_queue_free(peer->clear_node_queue); - peer->clear_node_queue = NULL; - } + if (peer->clear_node_queue) + work_queue_free_and_null(&peer->clear_node_queue); bgp_sync_delete(peer); @@ -7639,10 +7637,8 @@ void bgp_terminate(void) bgp_notify_send(peer, BGP_NOTIFY_CEASE, BGP_NOTIFY_CEASE_PEER_UNCONFIG); - if (bm->process_main_queue) { - work_queue_free(bm->process_main_queue); - bm->process_main_queue = NULL; - } + if (bm->process_main_queue) + work_queue_free_and_null(&bm->process_main_queue); if (bm->t_rmap_update) BGP_TIMER_OFF(bm->t_rmap_update); diff --git a/bgpd/rfapi/rfapi_import.c b/bgpd/rfapi/rfapi_import.c index c7d64bf1ed..348f1557e2 100644 --- a/bgpd/rfapi/rfapi_import.c +++ b/bgpd/rfapi/rfapi_import.c @@ -4336,7 +4336,7 @@ void bgp_rfapi_destroy(struct bgp *bgp, struct rfapi *h) h->import_mac = NULL; } - work_queue_free(h->deferred_close_q); + work_queue_free_and_null(&h->deferred_close_q); if (h->rfp != NULL) rfp_stop(h->rfp); diff --git a/bgpd/rfapi/rfapi_rib.c b/bgpd/rfapi/rfapi_rib.c index 2a8a465b70..c71f59563f 100644 --- a/bgpd/rfapi/rfapi_rib.c +++ b/bgpd/rfapi/rfapi_rib.c @@ -571,10 +571,8 @@ void rfapiRibClear(struct rfapi_descriptor *rfd) } } } - if (rfd->updated_responses_queue) { - work_queue_free(rfd->updated_responses_queue); - rfd->updated_responses_queue = NULL; - } + if (rfd->updated_responses_queue) + work_queue_free_and_null(&rfd->updated_responses_queue); } /* diff --git a/lib/workqueue.c b/lib/workqueue.c index d4ff3ee6ce..1af51c06c1 100644 --- a/lib/workqueue.c +++ b/lib/workqueue.c @@ -101,7 +101,7 @@ struct work_queue *work_queue_new(struct thread_master *m, return new; } -void work_queue_free(struct work_queue *wq) +void work_queue_free_original(struct work_queue *wq) { if (wq->thread != NULL) thread_cancel(wq->thread); @@ -119,6 +119,12 @@ void work_queue_free(struct work_queue *wq) return; } +void work_queue_free_and_null(struct work_queue **wq) +{ + work_queue_free_original(*wq); + *wq = NULL; +} + bool work_queue_is_scheduled(struct work_queue *wq) { return (wq->thread != NULL); diff --git a/lib/workqueue.h b/lib/workqueue.h index de49cb87fb..c9785de09a 100644 --- a/lib/workqueue.h +++ b/lib/workqueue.h @@ -28,7 +28,7 @@ DECLARE_MTYPE(WORK_QUEUE) /* Hold time for the initial schedule of a queue run, in millisec */ -#define WORK_QUEUE_DEFAULT_HOLD 50 +#define WORK_QUEUE_DEFAULT_HOLD 50 /* action value, for use by item processor and item error handlers */ typedef enum { @@ -148,8 +148,22 @@ static inline void work_queue_item_dequeue(struct work_queue *wq, * anything to it */ extern struct work_queue *work_queue_new(struct thread_master *, const char *); + /* destroy work queue */ -extern void work_queue_free(struct work_queue *); +/* + * The usage of work_queue_free is being transitioned to pass + * in the double pointer to remove use after free's. + */ +#if CONFDATE > 20190205 +CPP_NOTICE("work_queue_free without double pointer is deprecated, please fixup") +#endif +extern void work_queue_free_and_null(struct work_queue **); +extern void work_queue_free_original(struct work_queue *); +#define work_queue_free(X) \ + do { \ + work_queue_free_original((X)); \ + CPP_WARN("Please use work_queue_free_and_null"); \ + } while (0) /* Add the supplied data as an item onto the workqueue */ extern void work_queue_add(struct work_queue *, void *); diff --git a/zebra/main.c b/zebra/main.c index 6a08247f11..00d853ea26 100644 --- a/zebra/main.c +++ b/zebra/main.c @@ -140,7 +140,7 @@ static void sigint(void) SET_FLAG(zvrf->flags, ZEBRA_VRF_RETAIN); } if (zebrad.lsp_process_q) - work_queue_free(zebrad.lsp_process_q); + work_queue_free_and_null(&zebrad.lsp_process_q); vrf_terminate(); ns_walk_func(zebra_ns_disabled); @@ -151,7 +151,7 @@ static void sigint(void) route_map_finish(); list_delete_and_null(&zebrad.client_list); - work_queue_free(zebrad.ribq); + work_queue_free_and_null(&zebrad.ribq); meta_queue_free(zebrad.mq); frr_fini(); -- 2.39.5