bgpd: fix crash when unconfiguring a network with per-nexthop allocat…

…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>

bgpd/bgp_labelpool.c

@@ -1728,6 +1728,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,

bgpd/bgp_labelpool.h

@@ -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,

bgpd/bgp_mplsvpn.c

@@ -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 */
 
@@ -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,
 			   bgp_mplsvpn_get_label_per_nexthop_cb);
 	}