• Ido Schimmel's avatar
    mlxsw: spectrum_router: Fix use-after-free in route replace · 7387dbbc
    Ido Schimmel authored
    While working on IPv6 route replace I realized we can have a
    use-after-free in IPv4 in case the replaced route is offloaded and the
    only one using its FIB info.
    
    The problem is that fib_table_insert() drops the reference on the FIB
    info of the replaced routes which is eventually freed via call_rcu().
    Since the driver doesn't hold a reference on this FIB info it can cause
    a use-after-free when it tries to clear the RTNH_F_OFFLOAD flag stored
    in fi->fib_flags.
    
    After running the following commands in a loop for enough time with a
    KASAN enabled kernel I finally got the below trace.
    
    $ ip route add 192.168.50.0/24 via 192.168.200.1 dev enp3s0np3
    $ ip route replace 192.168.50.0/24 dev enp3s0np5
    $ ip route del 192.168.50.0/24 dev enp3s0np5
    
    BUG: KASAN: use-after-free in mlxsw_sp_fib_entry_offload_unset+0xa7/0x120 [mlxsw_spectrum]
    Read of size 4 at addr ffff8803717d9820 by task kworker/u4:2/55
    [...]
    ? mlxsw_sp_fib_entry_offload_unset+0xa7/0x120 [mlxsw_spectrum]
    ? mlxsw_sp_fib_entry_offload_unset+0xa7/0x120 [mlxsw_spectrum]
    ? mlxsw_sp_router_neighs_update_work+0x1cd0/0x1ce0 [mlxsw_spectrum]
    ? mlxsw_sp_fib_entry_offload_unset+0xa7/0x120 [mlxsw_spectrum]
    __asan_load4+0x61/0x80
    mlxsw_sp_fib_entry_offload_unset+0xa7/0x120 [mlxsw_spectrum]
    mlxsw_sp_fib_entry_offload_refresh+0xb6/0x370 [mlxsw_spectrum]
    mlxsw_sp_router_fib_event_work+0xd1c/0x2780 [mlxsw_spectrum]
    [...]
    Freed by task 5131:
     save_stack_trace+0x16/0x20
     save_stack+0x46/0xd0
     kasan_slab_free+0x70/0xc0
     kfree+0x144/0x570
     free_fib_info_rcu+0x2e7/0x410
     rcu_process_callbacks+0x4f8/0xe30
     __do_softirq+0x1d3/0x9e2
    
    Fix this by taking a reference on the FIB info when creating the nexthop
    group it represents and drop it when the group is destroyed.
    
    Fixes: 599cf8f9 ("mlxsw: spectrum_router: Add support for route replace")
    Signed-off-by: default avatarIdo Schimmel <idosch@mellanox.com>
    Signed-off-by: default avatarJiri Pirko <jiri@mellanox.com>
    Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
    7387dbbc
spectrum_router.c 99.7 KB