Skip to content

Commit

Permalink
bgpd: fix crash when unconfiguring a network with per-nexthop allocat…
Browse files Browse the repository at this point in the history
…ion mode

The following configuration commands trigger a crash of the BGP daemon:

> ipv6 route ff12::/64 fec0::6
> router bgp 65500
> address-family ipv6 unicast
> label vpn export auto
> rd vpn export 22:2
> rt vpn import 22:2
> rt vpn export 22:22
> network ff12::/64
> export vpn
> label vpn export allocation-mode per-nexthop
> no network ff12::/64
>

The following backtrace extract can be seen.

>        Signal: 11 (SEGV)
> [..]
>#4  0x00007fef4d51b711 in core_handler (signo=11, siginfo=0x7ffe9b70c170, context=0x7ffe9b70c040)
>    at /build/make-pkg/output/_packages/cp-routing/src/lib/sigevent.c:262
>#5  <signal handler called>
>#6  bgp_label_per_nexthop_send_nexthop_label (blnc=0x55fb67ebfcf0, cmd=46)
>    at /build/make-pkg/output/_packages/cp-routing/src/bgpd/bgp_labelpool.c:1158
>#7  0x000055fb66bcba61 in bgp_mplsvpn_get_label_per_nexthop_cb (label=81, context=0x55fb67ebfcf0,
>    allocated=true) at /build/make-pkg/output/_packages/cp-routing/src/bgpd/bgp_mplsvpn.c:765
>#8  0x000055fb66bbde4a in lp_cbq_docallback (wq=0x55fb678e64e0, data=0x55fb678c8580)
>    at /build/make-pkg/output/_packages/cp-routing/src/bgpd/bgp_labelpool.c:112

We are trying to access the 'blnc' memory zone that has been freed.
The 'blnc' memory zone is the label next-hop cache entry context that
stores the list of paths that use the same label value.

When configuring a network in an L3VPN instance, when the 'per-nexthop'
allocation mode is used, any declared network that has an identified
next-hop triggers a label request. The label request is always
asynchronous, and when a label is obtained, a callback is called, with
the label and the 'blnc' pointer passed as parameters.
If, in the meantime, the declared network is un-configured, then the
'blnc' context may be freed and becomes invalid.

Do free the blnc pointer only when the callback is requested:
- A flag indicates that there is a label allocation in progress.
When a free is detected, unset this flag, and detach the blnc
from the label next-hop cache hash list.
- When the requester is called back, if the flag is unset, the
'blnc' context is freed.

Fixes: ("577be36a41be") bgpd: add support for l3vpn per-nexthop label
Signed-off-by: Philippe Guibert <philippe.guibert@6wind.com>
  • Loading branch information
pguibert6WIND committed Feb 14, 2025
1 parent 196b7f1 commit 5336a20
Show file tree
Hide file tree
Showing 3 changed files with 14 additions and 0 deletions.
6 changes: 6 additions & 0 deletions bgpd/bgp_labelpool.c
Original file line number Diff line number Diff line change
Expand Up @@ -1734,6 +1734,12 @@ bgp_label_per_nexthop_find(struct bgp_label_per_nexthop_cache_head *tree,

void bgp_label_per_nexthop_free(struct bgp_label_per_nexthop_cache *blnc)
{
if (blnc->allocation_in_progress) {
blnc->allocation_in_progress = false;
bgp_label_per_nexthop_cache_del(blnc->tree, blnc);
return;
}

if (blnc->label != MPLS_INVALID_LABEL) {
bgp_zebra_send_nexthop_label(ZEBRA_MPLS_LABELS_DELETE,
blnc->label, blnc->nh->ifindex,
Expand Down
2 changes: 2 additions & 0 deletions bgpd/bgp_labelpool.h
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,8 @@ struct bgp_label_per_nexthop_cache {

/* Back pointer to the cache tree this entry belongs to. */
struct bgp_label_per_nexthop_cache_head *tree;

bool allocation_in_progress;
};

DECLARE_RBTREE_UNIQ(bgp_label_per_nexthop_cache,
Expand Down
6 changes: 6 additions & 0 deletions bgpd/bgp_mplsvpn.c
Original file line number Diff line number Diff line change
Expand Up @@ -1405,6 +1405,11 @@ static int bgp_mplsvpn_get_label_per_nexthop_cb(mpls_label_t label,
*/
blnc->label = MPLS_INVALID_LABEL;

if (!blnc->allocation_in_progress) {
bgp_label_per_nexthop_free(blnc);
return 0;
}

if (old_label == blnc->label)
return 0; /* no change */

Expand Down Expand Up @@ -1478,6 +1483,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
*/
blnc->allocation_in_progress = true;
bgp_lp_get(LP_TYPE_NEXTHOP, blnc, from_bgp->vrf_id,
bgp_mplsvpn_get_label_per_nexthop_cb);
}
Expand Down

0 comments on commit 5336a20

Please sign in to comment.