From: Donatas Abraitis Date: Thu, 9 Feb 2023 20:29:25 +0000 (+0200) Subject: bgpd: Intern default-originate attributes to avoid use-after-free X-Git-Tag: frr-8.4.3~14^2 X-Git-Url: https://git.puffer.fish/?a=commitdiff_plain;h=c6be72a69c2294bf93e1c7aa640ca44faeb04b6f;p=matthieu%2Ffrr.git bgpd: Intern default-originate attributes to avoid use-after-free When we receive a default route from a peer and we originate default route using `neighbor default-originate`, we do not track of struct attr we use, and when we do `no neighbor default-originate` we withdraw our generated default route, but we announce default-route from the peer. After we do this, we unintern aspath (which was used for default-originate), BUT it was used also for peer's default route we received. And here we have a use-after-free crash, because bgp_process_main_one() reaps old paths that are marked as BGP_PATH_REMOVED with aspath->refcnt > 0, but here it's 0. ``` 0 0x55c24bbcd022 in aspath_key_make bgpd/bgp_aspath.c:2070 1 0x55c24b8f1140 in attrhash_key_make bgpd/bgp_attr.c:777 2 0x7f52322e66c9 in hash_release lib/hash.c:220 3 0x55c24b8f6017 in bgp_attr_unintern bgpd/bgp_attr.c:1271 4 0x55c24ba0acaa in bgp_path_info_free_with_caller bgpd/bgp_route.c:283 5 0x55c24ba0a7de in bgp_path_info_unlock bgpd/bgp_route.c:309 6 0x55c24ba0af6d in bgp_path_info_reap bgpd/bgp_route.c:426 7 0x55c24ba17b9a in bgp_process_main_one bgpd/bgp_route.c:3333 8 0x55c24ba18a1d in bgp_process_wq bgpd/bgp_route.c:3425 9 0x7f52323c2cd5 in work_queue_run lib/workqueue.c:282 10 0x7f52323aab92 in thread_call lib/thread.c:2006 11 0x7f5232300dc7 in frr_run lib/libfrr.c:1198 12 0x55c24b8ea792 in main bgpd/bgp_main.c:520 13 0x7f5231c3a082 in __libc_start_main ../csu/libc-start.c:308 14 0x55c24b8ef0bd in _start (/usr/lib/frr/bgpd+0x2c90bd) ``` Signed-off-by: Donatas Abraitis --- diff --git a/bgpd/bgp_updgrp_adv.c b/bgpd/bgp_updgrp_adv.c index 27e3677702..46ace65c56 100644 --- a/bgpd/bgp_updgrp_adv.c +++ b/bgpd/bgp_updgrp_adv.c @@ -918,10 +918,14 @@ void subgroup_default_originate(struct update_subgroup *subgrp, int withdraw) if (subgroup_announce_check( dest, pi, subgrp, bgp_dest_get_prefix(dest), - &attr, NULL)) + &attr, NULL)) { + struct attr *default_attr = + bgp_attr_intern(&attr); + bgp_adj_out_set_subgroup( - dest, subgrp, &attr, - pi); + dest, subgrp, + default_attr, pi); + } } bgp_dest_unlock_node(dest); }