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 <sharpd@cumulusnetworks.com>

diff --git a/bgpd/bgpd.c b/bgpd/bgpd.c
index 60345769351c54b7cb742a7c14975fa3be788aee..fde72da4c8e33b93909b7d3442e41577bedea0a1 100644 (file)
--- 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 c7d64bf1edecb24efb780e66cf30785675ecf856..348f1557e2c244a649ee4e4990889b29e8e9b41d 100644 (file)
--- 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 2a8a465b70bfdb47c9c5994a5b880c7d025cf525..c71f59563fa830d8a1911921f18d9784f60b57e5 100644 (file)
--- 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 d4ff3ee6ce63a05afce2708d53779788a8dd3d8f..1af51c06c130ab06c0989a21ce088c8999c0e9a2 100644 (file)
--- 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 de49cb87fb925d23ebf0a6662e696d073b286d1a..c9785de09a5bc1d5d0539e84c6a68c027e143b9a 100644 (file)
--- 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 6a08247f11d4dda53df8e37e467c1285f6605392..00d853ea26d755e17e54652b9d5d3ee107cfede0 100644 (file)
--- 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();