bgpd: Fix crash in bgp_labelpool

The bgp labelpool code is grabbing the vpn policy data structure.
This vpn_policy has a pointer to the bgp data structure.  If
a item placed on the bgp label pool workqueue happens to sit
there for the microsecond or so and the operator issues a
`no router bgp...` command that corresponds to the vpn_policy
bgp pointer, when the workqueue is run it will crash because
the bgp pointer is now freed and something else owns it.

Modify the labelpool code to store the vrf id associated
with the request on the workqueue.  When you wake up
if the vrf id still has a bgp pointer allow the request
to continue, else drop it.

Signed-off-by: Donald Sharp <sharpd@nvidia.com>
(cherry picked from commit 14eac319e8ae9314f5270f871106a70c4986c60c)

diff --git a/bgpd/bgp_label.c b/bgpd/bgp_label.c
index 5db36217387887502b8ba971a27921dc0fcc2286..8ed9584b0affac59fa0be0686881b783a7ebff53 100644 (file)
--- a/bgpd/bgp_label.c
+++ b/bgpd/bgp_label.c
@@ -387,6 +387,8 @@ void bgp_reg_dereg_for_label(struct bgp_dest *dest, struct bgp_path_info *pi,
                         */
                        if (!have_label_to_reg) {
                                SET_FLAG(dest->flags, BGP_NODE_LABEL_REQUESTED);
+                               struct bgp_table *table;
+
                                if (BGP_DEBUG(labelpool, LABELPOOL))
                                        zlog_debug(
                                                "%s: Requesting label from LP for %pFX",
@@ -396,7 +398,9 @@ void bgp_reg_dereg_for_label(struct bgp_dest *dest, struct bgp_path_info *pi,
                                 * the pool. This means we'll never register
                                 * FECs withoutvalid labels.
                                 */
-                               bgp_lp_get(LP_TYPE_BGP_LU, dest,
+                               table = bgp_dest_table(dest);
+
+                               bgp_lp_get(LP_TYPE_BGP_LU, dest, table->bgp->vrf_id,
                                           bgp_reg_for_label_callback);
                                return;
                        }

diff --git a/bgpd/bgp_labelpool.c b/bgpd/bgp_labelpool.c
index 54a966e191fa62fc741a48c80a203f185c6f2bf8..a75cbcc5d9712987a1ca328a899bf4f349b1dd6e 100644 (file)
--- a/bgpd/bgp_labelpool.c
+++ b/bgpd/bgp_labelpool.c
@@ -77,6 +77,7 @@ struct lp_lcb {
        mpls_label_t    label;          /* MPLS_LABEL_NONE = not allocated */
        int             type;
        void            *labelid;       /* unique ID */
+       vrf_id_t        vrf_id;
        /*
         * callback for label allocation and loss
         *
@@ -97,6 +98,7 @@ struct lp_cbq_item {
        int             type;
        mpls_label_t    label;
        void            *labelid;
+       vrf_id_t        vrf_id;
        bool            allocated;      /* false = lost */
 };
 
@@ -105,6 +107,7 @@ static wq_item_status lp_cbq_docallback(struct work_queue *wq, void *data)
        struct lp_cbq_item *lcbq = data;
        int rc;
        int debug = BGP_DEBUG(labelpool, LABELPOOL);
+       struct bgp *bgp = bgp_lookup_by_vrf_id(lcbq->vrf_id);
 
        if (debug)
                zlog_debug("%s: calling callback with labelid=%p label=%u allocated=%d",
@@ -117,6 +120,9 @@ static wq_item_status lp_cbq_docallback(struct work_queue *wq, void *data)
                return WQ_SUCCESS;
        }
 
+       if (!bgp)
+               return WQ_SUCCESS;
+
        rc = (*(lcbq->cbfunc))(lcbq->label, lcbq->labelid, lcbq->allocated);
 
        if (lcbq->allocated && rc) {
@@ -320,10 +326,8 @@ static mpls_label_t get_label_from_pool(void *labelid)
 /*
  * Success indicated by value of "label" field in returned LCB
  */
-static struct lp_lcb *lcb_alloc(
-       int     type,
-       void    *labelid,
-       int     (*cbfunc)(mpls_label_t label, void *labelid, bool allocated))
+static struct lp_lcb *lcb_alloc(int type, void *labelid, vrf_id_t vrf_id,
+                               int (*cbfunc)(mpls_label_t label, void *labelid, bool allocated))
 {
        /*
         * Set up label control block
@@ -334,6 +338,7 @@ static struct lp_lcb *lcb_alloc(
        new->label = get_label_from_pool(labelid);
        new->type = type;
        new->labelid = labelid;
+       new->vrf_id = vrf_id;
        new->cbfunc = cbfunc;
 
        return new;
@@ -365,10 +370,8 @@ static struct lp_lcb *lcb_alloc(
  * Prior requests for a given labelid are detected so that requests and
  * assignments are not duplicated.
  */
-void bgp_lp_get(
-       int     type,
-       void    *labelid,
-       int     (*cbfunc)(mpls_label_t label, void *labelid, bool allocated))
+void bgp_lp_get(int type, void *labelid, vrf_id_t vrf_id,
+               int (*cbfunc)(mpls_label_t label, void *labelid, bool allocated))
 {
        struct lp_lcb *lcb;
        int requested = 0;
@@ -383,7 +386,7 @@ void bgp_lp_get(
        if (!skiplist_search(lp->ledger, labelid, (void **)&lcb)) {
                requested = 1;
        } else {
-               lcb = lcb_alloc(type, labelid, cbfunc);
+               lcb = lcb_alloc(type, labelid, vrf_id, cbfunc);
                if (debug)
                        zlog_debug("%s: inserting lcb=%p label=%u",
                                __func__, lcb, lcb->label);
@@ -413,6 +416,7 @@ void bgp_lp_get(
                q->type = lcb->type;
                q->label = lcb->label;
                q->labelid = lcb->labelid;
+               q->vrf_id = lcb->vrf_id;
                q->allocated = true;
 
                /* if this is a LU request, lock node before queueing */
@@ -580,6 +584,7 @@ static void bgp_sync_label_manager(struct event *e)
                q->type = lcb->type;
                q->label = lcb->label;
                q->labelid = lcb->labelid;
+               q->vrf_id = lcb->vrf_id;
                q->allocated = true;
 
                if (debug)
@@ -693,6 +698,7 @@ void bgp_lp_event_zebra_up(void)
                                q->type = lcb->type;
                                q->label = lcb->label;
                                q->labelid = lcb->labelid;
+                               q->vrf_id = lcb->vrf_id;
                                q->allocated = false;
                                check_bgp_lu_cb_lock(lcb);
                                work_queue_add(lp->callback_q, q);

diff --git a/bgpd/bgp_labelpool.h b/bgpd/bgp_labelpool.h
index a17482d112e8a2631d8a52aa00087154641349d4..509f9e625d936c4648c6a03cf2d0079845f316a1 100644 (file)
--- a/bgpd/bgp_labelpool.h
+++ b/bgpd/bgp_labelpool.h
@@ -35,8 +35,8 @@ struct labelpool {
 
 extern void bgp_lp_init(struct event_loop *master, struct labelpool *pool);
 extern void bgp_lp_finish(void);
-extern void bgp_lp_get(int type, void *labelid,
-       int (*cbfunc)(mpls_label_t label, void *labelid, bool allocated));
+extern void bgp_lp_get(int type, void *labelid, vrf_id_t vrf_id,
+                      int (*cbfunc)(mpls_label_t label, void *labelid, bool allocated));
 extern void bgp_lp_release(int type, void *labelid, mpls_label_t label);
 extern void bgp_lp_event_chunk(uint32_t first, uint32_t last);
 extern void bgp_lp_event_zebra_down(void);

diff --git a/bgpd/bgp_mplsvpn.c b/bgpd/bgp_mplsvpn.c
index 46e529f03dfc0efc9bef0be1753d2f24c713050c..580561aa9e6bb23d92c15e91e41a0664419d88e9 100644 (file)
--- a/bgpd/bgp_mplsvpn.c
+++ b/bgpd/bgp_mplsvpn.c
@@ -1450,7 +1450,7 @@ _vpn_leak_from_vrf_get_per_nexthop_label(struct bgp_path_info *pi,
                /* request a label to zebra for this nexthop
                 * the response from zebra will trigger the callback
                 */
-               bgp_lp_get(LP_TYPE_NEXTHOP, blnc,
+               bgp_lp_get(LP_TYPE_NEXTHOP, blnc, from_bgp->vrf_id,
                           bgp_mplsvpn_get_label_per_nexthop_cb);
        }
 
@@ -1490,7 +1490,8 @@ static mpls_label_t bgp_mplsvpn_get_vpn_label(struct vpn_policy *bgp_policy)
 {
        if (bgp_policy->tovpn_label == MPLS_LABEL_NONE &&
            CHECK_FLAG(bgp_policy->flags, BGP_VPN_POLICY_TOVPN_LABEL_AUTO)) {
-               bgp_lp_get(LP_TYPE_VRF, bgp_policy, vpn_leak_label_callback);
+               bgp_lp_get(LP_TYPE_VRF, bgp_policy, bgp_policy->bgp->vrf_id,
+                          vpn_leak_label_callback);
                return MPLS_INVALID_LABEL;
        }
        return bgp_policy->tovpn_label;
@@ -4387,7 +4388,7 @@ void bgp_mplsvpn_nh_label_bind_register_local_label(struct bgp *bgp,
                                                     label);
                bmnc->bgp_vpn = bgp;
                bmnc->allocation_in_progress = true;
-               bgp_lp_get(LP_TYPE_BGP_L3VPN_BIND, bmnc,
+               bgp_lp_get(LP_TYPE_BGP_L3VPN_BIND, bmnc, bgp->vrf_id,
                           bgp_mplsvpn_nh_label_bind_get_local_label_cb);
        }