]> git.proxmox.com Git - mirror_frr.git/commitdiff
bgpd: Intern default-originate attributes to avoid use-after-free
authorDonatas Abraitis <donatas@opensourcerouting.org>
Thu, 9 Feb 2023 20:29:25 +0000 (22:29 +0200)
committerMergify <37929162+mergify[bot]@users.noreply.github.com>
Fri, 10 Feb 2023 00:26:28 +0000 (00:26 +0000)
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 <donatas@opensourcerouting.org>
(cherry picked from commit e9340ff429f5f1a255e89a50867a04a370cd56bb)

bgpd/bgp_updgrp_adv.c

index cbd5d05922f924feafb6085d5708e57ccebbd8da..9d75abe5de757a85a06695cbe38f563efa62419c 100644 (file)
@@ -934,10 +934,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);
                }