Commit 7387dbbc authored by Ido Schimmel's avatar Ido Schimmel Committed by David S. Miller

mlxsw: spectrum_router: Fix use-after-free in route replace

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>
parent a4e75b76
...@@ -1867,6 +1867,7 @@ mlxsw_sp_nexthop_group_create(struct mlxsw_sp *mlxsw_sp, struct fib_info *fi) ...@@ -1867,6 +1867,7 @@ mlxsw_sp_nexthop_group_create(struct mlxsw_sp *mlxsw_sp, struct fib_info *fi)
nh_grp->gateway = fi->fib_nh->nh_scope == RT_SCOPE_LINK; nh_grp->gateway = fi->fib_nh->nh_scope == RT_SCOPE_LINK;
nh_grp->count = fi->fib_nhs; nh_grp->count = fi->fib_nhs;
nh_grp->key.fi = fi; nh_grp->key.fi = fi;
fib_info_hold(fi);
for (i = 0; i < nh_grp->count; i++) { for (i = 0; i < nh_grp->count; i++) {
nh = &nh_grp->nexthops[i]; nh = &nh_grp->nexthops[i];
fib_nh = &fi->fib_nh[i]; fib_nh = &fi->fib_nh[i];
...@@ -1886,6 +1887,7 @@ mlxsw_sp_nexthop_group_create(struct mlxsw_sp *mlxsw_sp, struct fib_info *fi) ...@@ -1886,6 +1887,7 @@ mlxsw_sp_nexthop_group_create(struct mlxsw_sp *mlxsw_sp, struct fib_info *fi)
nh = &nh_grp->nexthops[i]; nh = &nh_grp->nexthops[i];
mlxsw_sp_nexthop_fini(mlxsw_sp, nh); mlxsw_sp_nexthop_fini(mlxsw_sp, nh);
} }
fib_info_put(nh_grp->key.fi);
kfree(nh_grp); kfree(nh_grp);
return ERR_PTR(err); return ERR_PTR(err);
} }
...@@ -1904,6 +1906,7 @@ mlxsw_sp_nexthop_group_destroy(struct mlxsw_sp *mlxsw_sp, ...@@ -1904,6 +1906,7 @@ mlxsw_sp_nexthop_group_destroy(struct mlxsw_sp *mlxsw_sp,
} }
mlxsw_sp_nexthop_group_refresh(mlxsw_sp, nh_grp); mlxsw_sp_nexthop_group_refresh(mlxsw_sp, nh_grp);
WARN_ON_ONCE(nh_grp->adj_index_valid); WARN_ON_ONCE(nh_grp->adj_index_valid);
fib_info_put(nh_grp->key.fi);
kfree(nh_grp); kfree(nh_grp);
} }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment