lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Sun, 3 Jun 2018 08:46:44 -0600 From: David Ahern <dsa@...ulusnetworks.com> To: Eric Dumazet <eric.dumazet@...il.com>, Eric Dumazet <edumazet@...gle.com>, "David S . Miller" <davem@...emloft.net> Cc: netdev <netdev@...r.kernel.org> Subject: Re: [PATCH net] net: ipv6: prevent use after free in ip6_route_mpath_notify() On 6/3/18 8:31 AM, Eric Dumazet wrote: > > > On 06/03/2018 07:01 AM, David Ahern wrote: >> On 6/3/18 7:35 AM, Eric Dumazet wrote: >>> diff --git a/net/ipv6/route.c b/net/ipv6/route.c >>> index f4d61736c41abe8cd7f439c4a37100e90c1eacca..830eefdbdb6734eb81ea0322fb6077ee20be1889 100644 >>> --- a/net/ipv6/route.c >>> +++ b/net/ipv6/route.c >>> @@ -4263,7 +4263,9 @@ static int ip6_route_multipath_add(struct fib6_config *cfg, >>> >>> err_nh = NULL; >>> list_for_each_entry(nh, &rt6_nh_list, next) { >>> + dst_release(&rt_last->dst); >>> rt_last = nh->rt6_info; >>> + dst_hold(&rt_last->dst); >>> err = __ip6_ins_rt(nh->rt6_info, info, &nh->mxc, extack); >>> /* save reference to first route for notification */ >>> if (!rt_notif && !err) >>> @@ -4317,7 +4319,7 @@ static int ip6_route_multipath_add(struct fib6_config *cfg, >>> list_del(&nh->next); >>> kfree(nh); >>> } >>> - >>> + dst_release(&rt_last->dst); >>> return err; >>> } >> >> Since the rtnl lock is held, a successfully inserted route can not be >> removed until ip6_route_multipath_add finishes. This is a simpler change >> that works with net-next as well: > > Your patch changes the intent of your original commit. > > It seems you wanted rt_last to point to the last attempted insertion, > not the last successful one ? The note in ip6_route_mpath_notify explains it: /* if this is an APPEND route, then rt points to the first route * inserted and rt_last points to last route inserted. Userspace > > Or have I misunderstood, and not only we had a use-after-free, but also > a semantic error ? It was a mistake to set rt_last before checking err. So the use-after-free exposed the semantic error.
Powered by blists - more mailing lists