]> git.puffer.fish Git - mirror/frr.git/commit
bgpd: Prevent use after free 14032/head
authorDonald Sharp <sharpd@nvidia.com>
Fri, 14 Jul 2023 16:14:20 +0000 (12:14 -0400)
committerMergify <37929162+mergify[bot]@users.noreply.github.com>
Mon, 17 Jul 2023 12:13:33 +0000 (12:13 +0000)
commita2db52813832e82ad01f99e03dd275041bf371e7
treecd4218c7a26832bb02a5d48a18d1850f772a30a8
parente1ca52e80afe62216a1b3edf61bc53e6ac32ceb3
bgpd: Prevent use after free

When running bgp_always_compare_med, I am frequently seeing a crash
After running with valgrind I am seeing this and a invalid write
immediately after this as well.

==311743== Invalid read of size 2
==311743==    at 0x4992421: route_map_counter_decrement (routemap.c:3308)
==311743==    by 0x35664D: peer_route_map_unset (bgpd.c:7259)
==311743==    by 0x306546: peer_route_map_unset_vty (bgp_vty.c:8037)
==311743==    by 0x3066AC: no_neighbor_route_map (bgp_vty.c:8081)
==311743==    by 0x49078DE: cmd_execute_command_real (command.c:990)
==311743==    by 0x4907A63: cmd_execute_command (command.c:1050)
==311743==    by 0x490801F: cmd_execute (command.c:1217)
==311743==    by 0x49C5535: vty_command (vty.c:551)
==311743==    by 0x49C7459: vty_execute (vty.c:1314)
==311743==    by 0x49C97D1: vtysh_read (vty.c:2223)
==311743==    by 0x49BE5E2: event_call (event.c:1995)
==311743==    by 0x494786C: frr_run (libfrr.c:1204)
==311743==    by 0x1F7655: main (bgp_main.c:505)
==311743==  Address 0x9ec2180 is 64 bytes inside a block of size 120 free'd
==311743==    at 0x484B27F: free (in /usr/libexec/valgrind/vgpreload_memcheck-amd64-linux.so)
==311743==    by 0x495A1BA: qfree (memory.c:130)
==311743==    by 0x498D412: route_map_free_map (routemap.c:748)
==311743==    by 0x498D176: route_map_add (routemap.c:672)
==311743==    by 0x498D79B: route_map_get (routemap.c:857)
==311743==    by 0x499C256: lib_route_map_create (routemap_northbound.c:102)
==311743==    by 0x49702D8: nb_callback_create (northbound.c:1234)
==311743==    by 0x497107F: nb_callback_configuration (northbound.c:1578)
==311743==    by 0x4971693: nb_transaction_process (northbound.c:1709)
==311743==    by 0x496FCF4: nb_candidate_commit_apply (northbound.c:1103)
==311743==    by 0x496FE4E: nb_candidate_commit (northbound.c:1136)
==311743==    by 0x497798F: nb_cli_classic_commit (northbound_cli.c:49)
==311743==    by 0x4977B4F: nb_cli_pending_commit_check (northbound_cli.c:88)
==311743==    by 0x49078C1: cmd_execute_command_real (command.c:987)
==311743==    by 0x4907B44: cmd_execute_command (command.c:1068)
==311743==    by 0x490801F: cmd_execute (command.c:1217)
==311743==    by 0x49C5535: vty_command (vty.c:551)
==311743==    by 0x49C7459: vty_execute (vty.c:1314)
==311743==    by 0x49C97D1: vtysh_read (vty.c:2223)
==311743==    by 0x49BE5E2: event_call (event.c:1995)
==311743==    by 0x494786C: frr_run (libfrr.c:1204)
==311743==    by 0x1F7655: main (bgp_main.c:505)
==311743==  Block was alloc'd at
==311743==    at 0x484DA83: calloc (in /usr/libexec/valgrind/vgpreload_memcheck-amd64-linux.so)
==311743==    by 0x495A068: qcalloc (memory.c:105)
==311743==    by 0x498D0C8: route_map_new (routemap.c:646)
==311743==    by 0x498D128: route_map_add (routemap.c:658)
==311743==    by 0x498D79B: route_map_get (routemap.c:857)
==311743==    by 0x499C256: lib_route_map_create (routemap_northbound.c:102)
==311743==    by 0x49702D8: nb_callback_create (northbound.c:1234)
==311743==    by 0x497107F: nb_callback_configuration (northbound.c:1578)
==311743==    by 0x4971693: nb_transaction_process (northbound.c:1709)
==311743==    by 0x496FCF4: nb_candidate_commit_apply (northbound.c:1103)
==311743==    by 0x496FE4E: nb_candidate_commit (northbound.c:1136)
==311743==    by 0x497798F: nb_cli_classic_commit (northbound_cli.c:49)
==311743==    by 0x4977B4F: nb_cli_pending_commit_check (northbound_cli.c:88)
==311743==    by 0x49078C1: cmd_execute_command_real (command.c:987)
==311743==    by 0x4907B44: cmd_execute_command (command.c:1068)
==311743==    by 0x490801F: cmd_execute (command.c:1217)
==311743==    by 0x49C5535: vty_command (vty.c:551)
==311743==    by 0x49C7459: vty_execute (vty.c:1314)
==311743==    by 0x49C97D1: vtysh_read (vty.c:2223)
==311743==    by 0x49BE5E2: event_call (event.c:1995)
==311743==    by 0x494786C: frr_run (libfrr.c:1204)

Effectively the route_map that is being stored has been freed already
but we have not cleaned up properly yet.  Go through and clean the
code up by ensuring that the pointer actually exists instead of trusting
it does when doing the decrement operation.

Signed-off-by: Donald Sharp <sharpd@nvidia.com>
(cherry picked from commit 2ba2c284bab39f220eea3beabd5feeea216e3bfb)
bgpd/bgpd.c