Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

bgpd: fix crash when unconfiguring a network with per-nexthop allocation mode #18150

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

pguibert6WIND
Copy link
Member

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

Copy link

This pull request has conflicts, please resolve those before we can evaluate the pull request.

…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)
> [..]
>FRRouting#4  0x00007fef4d51b711 in core_handler (signo=11, siginfo=0x7ffe9b70c170, context=0x7ffe9b70c040)
>    at /build/make-pkg/output/_packages/cp-routing/src/lib/sigevent.c:262
>FRRouting#5  <signal handler called>
>FRRouting#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
>FRRouting#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
>FRRouting#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>
Copy link
Member

@riw777 riw777 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks good

@riw777
Copy link
Member

riw777 commented Feb 18, 2025

E AssertionError: r1, MPLS labels check fail: r1, show mpls table, duplicated or blacklisted nexthop address assert False

:-(

@pguibert6WIND pguibert6WIND marked this pull request as draft February 21, 2025 15:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants