proxmox-mirrors/mirror_frr
1
0
Fork 0
mirror of https://git.proxmox.com/git/mirror_frr synced 2025-08-12 12:53:26 +00:00
Code Issues Actions Packages Projects Releases Wiki Activity

bgpd: fix LU label callback crash

Browse Source 
under some conditions, the callback to get a label for
a LU bgp path could be called after the path had already
been freed. In this case we would be reading garbage
and potentially crash. Lock the path info before
queueing the callback, and unlock as the first step
of the callback, exiting gracefully if the path info
is now NULL.

Signed-off-by: Emanuele Di Pascale <emanuele@voltanet.io>
This commit is contained in:
Emanuele Di Pascale 2019-06-18 11:14:28 +02:00
parent 0c3bbed4e6
commit ea63ff6bbd
2 changed files with 58 additions and 7 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

15
bgpd/bgp_label.c
View File

@ -130,10 +130,21 @@ mpls_label_t bgp_adv_label(struct bgp_node *rn, struct bgp_path_info *pi,
int bgp_reg_for_label_callback(mpls_label_t new_label, void *labelid,
bool allocated)
{
struct bgp_path_info *pi = (struct bgp_path_info *)labelid;
struct bgp_node *rn = (struct bgp_node *)pi->net;
struct bgp_path_info *pi;
struct bgp_node *rn;
char addr[PREFIX_STRLEN];
pi = labelid;
/* Is this path still valid? */
if (!bgp_path_info_unlock(pi)) {
if (BGP_DEBUG(labelpool, LABELPOOL))
zlog_debug(
"%s: bgp_path_info is no longer valid, ignoring",
__func__);
return -1;
}
rn = pi->net;
prefix2str(&rn->p, addr, PREFIX_STRLEN);
if (BGP_DEBUG(labelpool, LABELPOOL))

50
bgpd/bgp_labelpool.c
View File

@ -180,9 +180,24 @@ void bgp_lp_init(struct thread_master *master, struct labelpool *pool)
lp->callback_q->spec.max_retries = 0;
}
/* check if a label callback was for a BGP LU path, and if so, unlock it */
static void check_bgp_lu_cb_unlock(struct lp_lcb *lcb)
{
if (lcb->type == LP_TYPE_BGP_LU)
bgp_path_info_unlock(lcb->labelid);
}
/* check if a label callback was for a BGP LU path, and if so, lock it */
static void check_bgp_lu_cb_lock(struct lp_lcb *lcb)
{
if (lcb->type == LP_TYPE_BGP_LU)
bgp_path_info_lock(lcb->labelid);
}
void bgp_lp_finish(void)
{
struct lp_fifo *lf;
struct work_queue_item *item, *titem;
if (!lp)
return;
@ -195,10 +210,21 @@ void bgp_lp_finish(void)
list_delete(&lp->chunks);
while ((lf = lp_fifo_pop(&lp->requests)))
while ((lf = lp_fifo_pop(&lp->requests))) {
check_bgp_lu_cb_unlock(&lf->lcb);
XFREE(MTYPE_BGP_LABEL_FIFO, lf);
}
lp_fifo_fini(&lp->requests);
/* we must unlock path infos for LU callbacks; but we cannot do that
* in the deletion callback of the workqueue, as that is also called
* to remove an element from the queue after it has been run, resulting
* in a double unlock. Hence we need to iterate over our queues and
* lists and manually perform the unlocking (ugh)
*/
STAILQ_FOREACH_SAFE (item, &lp->callback_q->items, wq, titem)
check_bgp_lu_cb_unlock(item->data);
work_queue_free_and_null(&lp->callback_q);
lp = NULL;
@ -328,6 +354,9 @@ void bgp_lp_get(
q->labelid = lcb->labelid;
q->allocated = true;
/* if this is a LU request, lock path info before queueing */
check_bgp_lu_cb_lock(lcb);
work_queue_add(lp->callback_q, q);
return;
@ -353,13 +382,16 @@ void bgp_lp_get(
sizeof(struct lp_fifo));
lf->lcb = *lcb;
/* if this is a LU request, lock path info before queueing */
check_bgp_lu_cb_lock(lcb);
lp_fifo_add_tail(&lp->requests, lf);
if (lp_fifo_count(&lp->requests) > lp->pending_count) {
if (!zclient_send_get_label_chunk(zclient, 0, LP_CHUNK_SIZE)) {
lp->pending_count += LP_CHUNK_SIZE;
if (!zclient || zclient->sock < 0)
return;
}
if (!zclient_send_get_label_chunk(zclient, 0, LP_CHUNK_SIZE))
lp->pending_count += LP_CHUNK_SIZE;
}
}
@ -436,6 +468,10 @@ void bgp_lp_event_chunk(uint8_t keep, uint32_t first, uint32_t last)
__func__, labelid,
lcb->label, lcb->label, lcb);
}
/* if this was a BGP_LU request, unlock path info node
*/
check_bgp_lu_cb_unlock(lcb);
goto finishedrequest;
}
@ -510,8 +546,10 @@ void bgp_lp_event_zebra_up(void)
lm_init_ok = lm_label_manager_connect(zclient, 1) == 0;
if (!lm_init_ok)
if (!lm_init_ok) {
zlog_err("%s: label manager connection error", __func__);
return;
}
zclient_send_get_label_chunk(zclient, 0, labels_needed);
lp->pending_count = labels_needed;
@ -544,6 +582,7 @@ void bgp_lp_event_zebra_up(void)
q->label = lcb->label;
q->labelid = lcb->labelid;
q->allocated = false;
check_bgp_lu_cb_lock(lcb);
work_queue_add(lp->callback_q, q);
lcb->label = MPLS_LABEL_NONE;
@ -556,6 +595,7 @@ void bgp_lp_event_zebra_up(void)
sizeof(struct lp_fifo));
lf->lcb = *lcb;
check_bgp_lu_cb_lock(lcb);
lp_fifo_add_tail(&lp->requests, lf);
}