]> git.puffer.fish Git - matthieu/frr.git/commit
bgpd: Prevent use after free
authorDonald Sharp <sharpd@nvidia.com>
Fri, 14 Jul 2023 16:14:20 +0000 (12:14 -0400)
committerDonald Sharp <sharpd@nvidia.com>
Fri, 14 Jul 2023 16:16:38 +0000 (12:16 -0400)
commit2ba2c284bab39f220eea3beabd5feeea216e3bfb
tree9c040641955e3dd4398a0f7806ba2cdbb3236bd0
parenteb6a8a02f38391b505be0313684b258b585a82fa
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>
bgpd/bgpd.c