Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

zebra: fix table heap-after-free crash #16614

Open
wants to merge 9 commits into
base: master
Choose a base branch
from

Commits on Oct 16, 2024

  1. zebra: fix table heap-after-free crash

    Fix a heap-after-free that causes zebra to crash even without
    address-sanitizer. To reproduce:
    
    > echo "100 my_table" | tee -a /etc/iproute2/rt_tables
    > ip route add blackhole default table 100
    > ip route show table 100
    > ip l add red type vrf table 100
    > ip l del red
    > ip route del blackhole default table 100
    
    Zebra manages routing tables for all existing Linux RT tables,
    regardless of whether they are assigned to a VRF interface. When a table
    is not assigned to any VRF, zebra arbitrarily assigns it to the default
    VRF, even though this is not strictly accurate (the code expects this
    behavior).
    
    When an RT table is created after a VRF, zebra correctly assigns the
    table to the VRF. However, if a VRF interface is assigned to an existing
    RT table, zebra does not update the table owner, which remains as the
    default VRF. As a result, existing routing entries remain under the
    default VRF, while new entries are correctly assigned to the VRF. The
    VRF mismatch is unexpected in the code and creates crashes and memory
    related issues.
    
    Furthermore, Linux does not automatically delete RT tables when they are
    unassigned from a VRF. It is incorrect to delete these tables from zebra.
    
    Instead, at VRF disabling, do not release the table but reassign it to
    the default VRF. At VRF enabling, change the table owner back to the
    appropriate VRF.
    
    > ==2866266==ERROR: AddressSanitizer: heap-use-after-free on address 0x606000154f54 at pc 0x7fa32474b83f bp 0x7ffe94f67d90 sp 0x7ffe94f67d88
    > READ of size 1 at 0x606000154f54 thread T0
    >     #0 0x7fa32474b83e in rn_hash_node_const_find lib/table.c:28
    >     #1 0x7fa32474bab1 in rn_hash_node_find lib/table.c:28
    >     #2 0x7fa32474d783 in route_node_get lib/table.c:283
    >     #3 0x7fa3247328dd in srcdest_rnode_get lib/srcdest_table.c:231
    >     FRRouting#4 0x55b0e4fa8da4 in rib_find_rn_from_ctx zebra/zebra_rib.c:1957
    >     FRRouting#5 0x55b0e4fa8e31 in rib_process_result zebra/zebra_rib.c:1988
    >     FRRouting#6 0x55b0e4fb9d64 in rib_process_dplane_results zebra/zebra_rib.c:4894
    >     FRRouting#7 0x7fa32476689c in event_call lib/event.c:1996
    >     FRRouting#8 0x7fa32463b7b2 in frr_run lib/libfrr.c:1232
    >     FRRouting#9 0x55b0e4e6c32a in main zebra/main.c:526
    >     FRRouting#10 0x7fa32424fd09 in __libc_start_main ../csu/libc-start.c:308
    >     FRRouting#11 0x55b0e4e2d649 in _start (/usr/lib/frr/zebra+0x1a1649)
    >
    > 0x606000154f54 is located 20 bytes inside of 56-byte region [0x606000154f40,0x606000154f78)
    > freed by thread T0 here:
    >     #0 0x7fa324ca9b6f in __interceptor_free ../../../../src/libsanitizer/asan/asan_malloc_linux.cpp:123
    >     #1 0x7fa324668d8f in qfree lib/memory.c:130
    >     #2 0x7fa32474c421 in route_table_free lib/table.c:126
    >     #3 0x7fa32474bf96 in route_table_finish lib/table.c:46
    >     FRRouting#4 0x55b0e4fbca3a in zebra_router_free_table zebra/zebra_router.c:191
    >     FRRouting#5 0x55b0e4fbccea in zebra_router_release_table zebra/zebra_router.c:214
    >     FRRouting#6 0x55b0e4fd428e in zebra_vrf_disable zebra/zebra_vrf.c:219
    >     FRRouting#7 0x7fa32476fabf in vrf_disable lib/vrf.c:326
    >     FRRouting#8 0x7fa32476f5d4 in vrf_delete lib/vrf.c:231
    >     FRRouting#9 0x55b0e4e4ad36 in interface_vrf_change zebra/interface.c:1478
    >     FRRouting#10 0x55b0e4e4d5d2 in zebra_if_dplane_ifp_handling zebra/interface.c:1949
    >     FRRouting#11 0x55b0e4e4fb89 in zebra_if_dplane_result zebra/interface.c:2268
    >     FRRouting#12 0x55b0e4fb9f26 in rib_process_dplane_results zebra/zebra_rib.c:4954
    >     FRRouting#13 0x7fa32476689c in event_call lib/event.c:1996
    >     FRRouting#14 0x7fa32463b7b2 in frr_run lib/libfrr.c:1232
    >     FRRouting#15 0x55b0e4e6c32a in main zebra/main.c:526
    >     FRRouting#16 0x7fa32424fd09 in __libc_start_main ../csu/libc-start.c:308
    >
    > previously allocated by thread T0 here:
    >     #0 0x7fa324caa037 in __interceptor_calloc ../../../../src/libsanitizer/asan/asan_malloc_linux.cpp:154
    >     #1 0x7fa324668c4d in qcalloc lib/memory.c:105
    >     #2 0x7fa32474bf33 in route_table_init_with_delegate lib/table.c:38
    >     #3 0x7fa32474e73c in route_table_init lib/table.c:512
    >     FRRouting#4 0x55b0e4fbc353 in zebra_router_get_table zebra/zebra_router.c:137
    >     FRRouting#5 0x55b0e4fd4da0 in zebra_vrf_table_create zebra/zebra_vrf.c:358
    >     FRRouting#6 0x55b0e4fd3d30 in zebra_vrf_enable zebra/zebra_vrf.c:140
    >     FRRouting#7 0x7fa32476f9b2 in vrf_enable lib/vrf.c:286
    >     FRRouting#8 0x55b0e4e4af76 in interface_vrf_change zebra/interface.c:1533
    >     FRRouting#9 0x55b0e4e4d612 in zebra_if_dplane_ifp_handling zebra/interface.c:1968
    >     FRRouting#10 0x55b0e4e4fb89 in zebra_if_dplane_result zebra/interface.c:2268
    >     FRRouting#11 0x55b0e4fb9f26 in rib_process_dplane_results zebra/zebra_rib.c:4954
    >     FRRouting#12 0x7fa32476689c in event_call lib/event.c:1996
    >     FRRouting#13 0x7fa32463b7b2 in frr_run lib/libfrr.c:1232
    >     FRRouting#14 0x55b0e4e6c32a in main zebra/main.c:526
    >     FRRouting#15 0x7fa32424fd09 in __libc_start_main ../csu/libc-start.c:308
    
    Fixes: d8612e6 ("zebra: Track tables allocated by vrf and cleanup")
    Signed-off-by: Louis Scalbert <[email protected]>
    louis-6wind committed Oct 16, 2024
    Configuration menu
    Copy the full SHA
    abd5dc7 View commit details
    Browse the repository at this point in the history
  2. zebra: remove vrf route entries at vrf disabling

    When a VRF is deleted, the kernel retains only its own routing entries
    in the former VRF table and removes all others.q
    
    This change ensures that routing entries created by FRR daemons are also
    removed from the former zebra VRF table when the VRF is disabled.
    
    To test:
    
    > echo "100 my_table" | tee -a /etc/iproute2/rt_tables
    > ip l add du0 type dummy
    > ifconfig du0 192.168.0.1/24 up
    > ip route add blackhole default table 100
    > ip route show table 100
    > ip l add red type vrf table 100
    > ip l set du0 master red
    > vtysh -c 'configure' -c 'vrf red' -c 'ip route 10.0.0.0/24 192.168.0.254'
    > vtysh -c 'show ip route table 100'
    > sleep 0.1
    > ip l del red
    > sleep 0.1
    > vtysh -c 'show ip route table 100'
    > ip l add red type vrf table 100
    > ip l set du0 master red
    > vtysh -c 'configure' -c 'vrf red' -c 'ip route 10.0.0.0/24 192.168.0.254'
    > vtysh -c 'show ip route table 100'
    > sleep 0.1
    > ip l del red
    > sleep 0.1
    > vtysh -c 'show ip route table 100'
    
    Fixes: d8612e6 ("zebra: Track tables allocated by vrf and cleanup")
    Signed-off-by: Louis Scalbert <[email protected]>
    louis-6wind committed Oct 16, 2024
    Configuration menu
    Copy the full SHA
    9292c83 View commit details
    Browse the repository at this point in the history
  3. zebra: fix removed default route at vrf enabling

    When a routing table (RT) already has a default route before being
    assigned to a VRF, the default route vanishes in zebra after the VRF
    assignment.
    
    > root@router:~# ip route add blackhole default table 100
    > root@router:~# ip route show table 100
    > blackhole default
    > root@router:~# vtysh -c 'show ip route table 100'
    > [...]
    > VRF default table 100:
    > K>* 0.0.0.0/0 [0/0] unreachable (blackhole), 00:00:05
    > root@router:~# ip l add red type vrf table 100
    > root@router:~# vtysh -c 'show ip route table 100'
    > root@router:~#
    
    Do not override the default route if it exists.
    
    Signed-off-by: Louis Scalbert <[email protected]>
    louis-6wind committed Oct 16, 2024
    Configuration menu
    Copy the full SHA
    440b521 View commit details
    Browse the repository at this point in the history
  4. zebra: fix vanished blackhole route

    Fix vanished blackhole route when kernel routes are updated.
    
    > root@router# echo "100 my_table" | tee -a /etc/iproute2/rt_tables
    > root@router# ip l add du0 type dummy
    > root@router# ifconfig du0 192.168.0.1/24 up
    > root@router# ip route add blackhole default table 100
    > root@router# ip route show table 100
    > blackhole default
    > root@router# vtysh -c 'show ip route table 100'
    > [...]
    > Table 100:
    > K>* 0.0.0.0/0 [0/0] unreachable (blackhole), weight 1, 00:00:05
    > root@router# ip l add red type vrf table 100
    > root@router# vtysh -c 'show ip route table 100'
    > [...]
    > Table 100:
    > K>* 0.0.0.0/0 [0/0] unreachable (blackhole), weight 1, 00:00:16
    > root@router# ip l set du0 master red
    > root@router# vtysh -c 'show ip route table 100'
    > [...]
    > Table 100:
    > C>* 192.168.0.0/24 is directly connected, du0, weight 1, 00:00:02
    > L>* 192.168.0.1/32 is directly connected, du0, weight 1, 00:00:02
    > root@router# ip route show table 100
    > blackhole default
    > 192.168.0.0/24 dev du0 proto kernel scope link src 192.168.0.1
    > local 192.168.0.1 dev du0 proto kernel scope host src 192.168.0.1
    > broadcast 192.168.0.255 dev du0 proto kernel scope link src 192.168.0.1
    
    Fixes: d528c02 ("zebra: Handle kernel routes appropriately")
    Signed-off-by: Louis Scalbert <[email protected]>
    louis-6wind committed Oct 16, 2024
    Configuration menu
    Copy the full SHA
    91d9714 View commit details
    Browse the repository at this point in the history
  5. zebra: fix show ip route table vrf display

    A table ID != 254 is not assigned to VRF default but "show ip route
    table XX" shows that is owned by VRF default.
    
    > ip route add blackhole default table 111
    > ip link add RED type vrf table 111
    
    > ubu-24-arm# do sho ip route vrf RED
    > [...]
    > VRF RED:
    > K>* 0.0.0.0/0 [0/0] unreachable (blackhole) (vrf default), weight 1, 00:00:53
    >
    > ubu-24-arm# do sho ip route table 111
    > [...]
    > VRF default table 111:
    > K>* 0.0.0.0/0 [0/0] unreachable (blackhole) (vrf default), weight 1, 00:00:39
    
    Fix the output:
    
    > ubu-24-arm# do sho ip route table all
    > [...]
    > Table 111:
    > K>* 0.0.0.0/0 [0/0] unreachable (blackhole) (vrf default), weight 1, 00:00:39
    >
    > Table 254:
    > [...]
    
    Signed-off-by: Louis Scalbert <[email protected]>
    louis-6wind committed Oct 16, 2024
    Configuration menu
    Copy the full SHA
    fc7fd90 View commit details
    Browse the repository at this point in the history
  6. tests: zebra_rib, test vrf change

    Test table ID move to a VRF and the removal of the VRF.
    
    Signed-off-by: Louis Scalbert <[email protected]>
    louis-6wind committed Oct 16, 2024
    Configuration menu
    Copy the full SHA
    031787a View commit details
    Browse the repository at this point in the history
  7. lib: add table_id route entry flag

    Add a flag to mean that a route entry is owned by a table ID and not by
    a VRF. If the VRF associated to the table ID is deleted, the route
    entry must not be deleted.
    
    Signed-off-by: Louis Scalbert <[email protected]>
    louis-6wind committed Oct 16, 2024
    Configuration menu
    Copy the full SHA
    56999da View commit details
    Browse the repository at this point in the history
  8. zebra: keep table route entries at vrf disabling

    At VRF disabling, keep the route entries that was associated to its
    table ID but not to the VRF itself. Kernel flushes these entries so we
    need to reinstall them.
    
    Signed-off-by: Louis Scalbert <[email protected]>
    louis-6wind committed Oct 16, 2024
    Configuration menu
    Copy the full SHA
    aeb482d View commit details
    Browse the repository at this point in the history
  9. tests: zebra_rib, test vrf change

    Test table ID move to a VRF and the removal of the VRF.
    
    Signed-off-by: Louis Scalbert <[email protected]>
    louis-6wind committed Oct 16, 2024
    Configuration menu
    Copy the full SHA
    4b620e6 View commit details
    Browse the repository at this point in the history