Bug 270910 - net/frr8: fix bgpd crash in some cases (8.x)
Summary: net/frr8: fix bgpd crash in some cases (8.x)
Status: Closed FIXED
Alias: None
Product: Ports & Packages
Classification: Unclassified
Component: Individual Port(s) (show other bugs)
Version: Latest
Hardware: Any Any
: --- Affects Only Me
Assignee: Olivier Cochard
URL: https://lists.frrouting.org/pipermail...
Keywords:
Depends on:
Blocks:
 
Reported: 2023-04-18 10:42 UTC by Kurt Jaeger
Modified: 2023-10-05 13:47 UTC (History)
3 users (show)

See Also:
bugzilla: maintainer-feedback? (olivier)


Attachments
Put patch in files/, recompile (660 bytes, text/plain)
2023-04-18 10:42 UTC, Kurt Jaeger
no flags Details
Alternative silly patch (378 bytes, patch)
2023-04-20 14:49 UTC, Philip Paeps
no flags Details | Diff
official patch from upstream (3.57 KB, patch)
2023-04-21 18:06 UTC, Kurt Jaeger
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Kurt Jaeger freebsd_committer freebsd_triage 2023-04-18 10:42:17 UTC
Created attachment 241558 [details]
Put patch in files/, recompile

This type of crash can happen:

BGP: bgpd/bgp_lcommunity.c:236: set_lcommunity_string(): assertion ((unsigned int)len < str_buf_sz) failed

The patch should fix this problem.
Comment 1 mike 2023-04-20 13:50:17 UTC
Hi, I seem to run into this issue as well, but the patch does not seem to fix it.

Apr 20 09:42:22 cg-2023 bgpd[40721]: bgpd/bgp_lcommunity.c:239: set_lcommunity_string(): assertion ((unsigned int)len < str_buf_sz) failed

I have a pcap of the session, but so far havent found the trigger.
Comment 2 Philip Paeps freebsd_committer freebsd_triage 2023-04-20 14:49:14 UTC
Created attachment 241619 [details]
Alternative silly patch

Kurt's patch disables bypasses parsing large communities.  My patch doubles the buffer size and kicks the can down the road.
Comment 3 Philip Paeps freebsd_committer freebsd_triage 2023-04-20 14:51:23 UTC
(In reply to mike from comment #1)

I suspect it triggers when you learn a route with more large communities that, when parsed, do not fit in a buffer of a little over 1024 bytes.

Glancing at the code, it looks like BUFSIZ is a very arbitrary buffer size.  I simply doubled it to make the problem go away.
Comment 4 mike 2023-04-20 14:58:03 UTC
(In reply to Philip Paeps from comment #3)

The danger is that you need to update ALL your bgp speakers, otherwise you will propagate that internally via ibgp and.... fun times with everything crashing :(

Is there a way to drop inbound communities from ebgp peers ? I dont make routing decisions so would just like to drop them at this point
Comment 5 Philip Paeps freebsd_committer freebsd_triage 2023-04-20 15:07:04 UTC
No idea ... sorry.  I don't have that particular problem.  I only give my ibgp speakers a default route and my own routes.  I don't propagate the full routing table.

You should be able to use a route-map and bgp set large-community without "additive".  That should drop the large communities you learned.  You might want to define a large community that means "dropped all other large communities to work around an frr bug". (untested)
Comment 6 mike 2023-04-20 21:02:31 UTC
They have an issue and suggested fix in their repo now

https://github.com/FRRouting/frr/pull/13341
Comment 7 mike 2023-04-21 17:41:41 UTC
A note about the bug. Unfortunately, there does not seem to be a way to defend against this bug being exploited without patching. Using a route-map to avoid the large community works after the problematic code path is hit.  e.g given the peer 

 neighbor 172.16.20.1 remote-as 65679
 neighbor 172.16.20.1 description Upstream
 neighbor 172.16.20.1 update-source 172.16.20.2
  neighbor 172.16.20.1 next-hop-self
  neighbor 172.16.20.1 soft-reconfiguration inbound
  neighbor 172.16.20.1 prefix-list UPSTREAM-IN in
  neighbor 172.16.20.1 prefix-list UPSTREAM-OUT out
  neighbor 172.16.20.1 route-map DELETE-COM-IN in

route-map DELETE-COM-IN permit 10
 set large-community none
exit

By the time the routemap is hit, the damage is done.  However, if you just patch your ebgp speakers and pass along all routes to ibgp speakers, make sure you either remove the big large communities, or patch first your ibgp speakers as they too will crash from this bug.
Comment 8 Kurt Jaeger freebsd_committer freebsd_triage 2023-04-21 18:06:38 UTC
Created attachment 241636 [details]
official patch from upstream

Using this now in some systems.
Comment 9 commit-hook freebsd_committer freebsd_triage 2023-04-21 22:08:28 UTC
A commit in branch main references this bug:

URL: https://cgit.FreeBSD.org/ports/commit/?id=dc2c49ff321068aacbb763d951f301b90094958b

commit dc2c49ff321068aacbb763d951f301b90094958b
Author:     Olivier Cochard <olivier@FreeBSD.org>
AuthorDate: 2023-04-21 21:25:25 +0000
Commit:     Olivier Cochard <olivier@FreeBSD.org>
CommitDate: 2023-04-21 21:34:01 +0000

    net/frr8: Fix bgpd crash with large communities and enable backtrace

    PR:             270910
    Reported by:    pi

 net/frr8/Makefile                                 | 14 ++++----
 net/frr8/files/patch-bgpd_bgp__lcommunity.c (new) | 42 +++++++++++++++++++++++
 2 files changed, 50 insertions(+), 6 deletions(-)