Message ID | 20240303144801.702646-1-edumazet@google.com (mailing list archive) |
---|---|
State | Accepted |
Commit | 685f7d531264599b3f167f1e94bbd22f120e5fab |
Delegated to: | Netdev Maintainers |
Headers | show |
Series | [net] net/ipv6: avoid possible UAF in ip6_route_mpath_notify() | expand |
On 3/3/24 7:48 AM, Eric Dumazet wrote: > syzbot found another use-after-free in ip6_route_mpath_notify() [1] > > Commit f7225172f25a ("net/ipv6: prevent use after free in > ip6_route_mpath_notify") was not able to fix the root cause. > > We need to defer the fib6_info_release() calls after > ip6_route_mpath_notify(), in the cleanup phase. > ... > > Fixes: 3b1137fe7482 ("net: ipv6: Change notifications for multipath add to RTA_MULTIPATH") > Reported-by: syzbot <syzkaller@googlegroups.com> > Signed-off-by: Eric Dumazet <edumazet@google.com> > Cc: David Ahern <dsahern@kernel.org> > --- > net/ipv6/route.c | 21 +++++++-------------- > 1 file changed, 7 insertions(+), 14 deletions(-) > Reviewed-by: David Ahern <dsahern@kernel.org>
Hello: This patch was applied to netdev/net.git (main) by Jakub Kicinski <kuba@kernel.org>: On Sun, 3 Mar 2024 14:48:00 +0000 you wrote: > syzbot found another use-after-free in ip6_route_mpath_notify() [1] > > Commit f7225172f25a ("net/ipv6: prevent use after free in > ip6_route_mpath_notify") was not able to fix the root cause. > > We need to defer the fib6_info_release() calls after > ip6_route_mpath_notify(), in the cleanup phase. > > [...] Here is the summary with links: - [net] net/ipv6: avoid possible UAF in ip6_route_mpath_notify() https://git.kernel.org/netdev/net/c/685f7d531264 You are awesome, thank you!
diff --git a/net/ipv6/route.c b/net/ipv6/route.c index ea1dec8448fce8ccf29be650301e937cfce6bd7a..ef815ba583a8f4ed0ca523a13c515f108132a939 100644 --- a/net/ipv6/route.c +++ b/net/ipv6/route.c @@ -5332,19 +5332,7 @@ static int ip6_route_multipath_add(struct fib6_config *cfg, err_nh = NULL; list_for_each_entry(nh, &rt6_nh_list, next) { err = __ip6_ins_rt(nh->fib6_info, info, extack); - fib6_info_release(nh->fib6_info); - - if (!err) { - /* save reference to last route successfully inserted */ - rt_last = nh->fib6_info; - - /* save reference to first route for notification */ - if (!rt_notif) - rt_notif = nh->fib6_info; - } - /* nh->fib6_info is used or freed at this point, reset to NULL*/ - nh->fib6_info = NULL; if (err) { if (replace && nhn) NL_SET_ERR_MSG_MOD(extack, @@ -5352,6 +5340,12 @@ static int ip6_route_multipath_add(struct fib6_config *cfg, err_nh = nh; goto add_errout; } + /* save reference to last route successfully inserted */ + rt_last = nh->fib6_info; + + /* save reference to first route for notification */ + if (!rt_notif) + rt_notif = nh->fib6_info; /* Because each route is added like a single route we remove * these flags after the first nexthop: if there is a collision, @@ -5412,8 +5406,7 @@ static int ip6_route_multipath_add(struct fib6_config *cfg, cleanup: list_for_each_entry_safe(nh, nh_safe, &rt6_nh_list, next) { - if (nh->fib6_info) - fib6_info_release(nh->fib6_info); + fib6_info_release(nh->fib6_info); list_del(&nh->next); kfree(nh); }