netdev - Re: [Patch net] ipv6: initialize route null entry in addrconf

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Thu, 4 May 2017 14:28:37 +0200
From:   Andrey Konovalov <andreyknvl@...gle.com>
To:     Cong Wang <xiyou.wangcong@...il.com>
Cc:     netdev <netdev@...r.kernel.org>, David Ahern <dsahern@...il.com>
Subject: Re: [Patch net] ipv6: initialize route null entry in addrconf_init()

On Thu, May 4, 2017 at 7:07 AM, Cong Wang <xiyou.wangcong@...il.com> wrote:
> Andrey reported a crash on init_net.ipv6.ip6_null_entry->rt6i_idev
> since it is always NULL.
>
> This is clearly wrong, we have code to initialize it to loopback_dev,
> unfortunately the order is still not correct.
>
> loopback_dev is registered very early during boot, we lose a chance
> to re-initialize it in notifier. addrconf_init() is called after
> ip6_route_init(), which means we have no chance to correct it.
>
> Fix it by moving this initialization explicitly after
> ipv6_add_dev(init_net.loopback_dev) in addrconf_init().
>
> Reported-by: Andrey Konovalov <andreyknvl@...gle.com>
> Signed-off-by: Cong Wang <xiyou.wangcong@...il.com>

Hi Cong,

This fixes the bug triggered by my reproducer.

Thanks!

Tested-by: Andrey Konovalov <andreyknvl@...gle.com>

> ---
>  include/net/ip6_route.h |  1 +
>  net/ipv6/addrconf.c     |  2 ++
>  net/ipv6/route.c        | 26 +++++++++++++++-----------
>  3 files changed, 18 insertions(+), 11 deletions(-)
>
> diff --git a/include/net/ip6_route.h b/include/net/ip6_route.h
> index 9dc2c18..f5e625f 100644
> --- a/include/net/ip6_route.h
> +++ b/include/net/ip6_route.h
> @@ -84,6 +84,7 @@ struct dst_entry *ip6_route_lookup(struct net *net, struct flowi6 *fl6,
>  struct rt6_info *ip6_pol_route(struct net *net, struct fib6_table *table,
>                                int ifindex, struct flowi6 *fl6, int flags);
>
> +void ip6_route_init_special_entries(void);
>  int ip6_route_init(void);
>  void ip6_route_cleanup(void);
>
> diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
> index a2a370b..77a4bd5 100644
> --- a/net/ipv6/addrconf.c
> +++ b/net/ipv6/addrconf.c
> @@ -6573,6 +6573,8 @@ int __init addrconf_init(void)
>                 goto errlo;
>         }
>
> +       ip6_route_init_special_entries();
> +
>         for (i = 0; i < IN6_ADDR_HSIZE; i++)
>                 INIT_HLIST_HEAD(&inet6_addr_lst[i]);
>
> diff --git a/net/ipv6/route.c b/net/ipv6/route.c
> index a1bf426..2f11366 100644
> --- a/net/ipv6/route.c
> +++ b/net/ipv6/route.c
> @@ -4027,6 +4027,21 @@ static struct notifier_block ip6_route_dev_notifier = {
>         .priority = 0,
>  };
>
> +void __init ip6_route_init_special_entries(void)
> +{
> +       /* Registering of the loopback is done before this portion of code,
> +        * the loopback reference in rt6_info will not be taken, do it
> +        * manually for init_net */
> +       init_net.ipv6.ip6_null_entry->dst.dev = init_net.loopback_dev;
> +       init_net.ipv6.ip6_null_entry->rt6i_idev = in6_dev_get(init_net.loopback_dev);
> +  #ifdef CONFIG_IPV6_MULTIPLE_TABLES
> +       init_net.ipv6.ip6_prohibit_entry->dst.dev = init_net.loopback_dev;
> +       init_net.ipv6.ip6_prohibit_entry->rt6i_idev = in6_dev_get(init_net.loopback_dev);
> +       init_net.ipv6.ip6_blk_hole_entry->dst.dev = init_net.loopback_dev;
> +       init_net.ipv6.ip6_blk_hole_entry->rt6i_idev = in6_dev_get(init_net.loopback_dev);
> +  #endif
> +}
> +
>  int __init ip6_route_init(void)
>  {
>         int ret;
> @@ -4053,17 +4068,6 @@ int __init ip6_route_init(void)
>
>         ip6_dst_blackhole_ops.kmem_cachep = ip6_dst_ops_template.kmem_cachep;
>
> -       /* Registering of the loopback is done before this portion of code,
> -        * the loopback reference in rt6_info will not be taken, do it
> -        * manually for init_net */
> -       init_net.ipv6.ip6_null_entry->dst.dev = init_net.loopback_dev;
> -       init_net.ipv6.ip6_null_entry->rt6i_idev = in6_dev_get(init_net.loopback_dev);
> -  #ifdef CONFIG_IPV6_MULTIPLE_TABLES
> -       init_net.ipv6.ip6_prohibit_entry->dst.dev = init_net.loopback_dev;
> -       init_net.ipv6.ip6_prohibit_entry->rt6i_idev = in6_dev_get(init_net.loopback_dev);
> -       init_net.ipv6.ip6_blk_hole_entry->dst.dev = init_net.loopback_dev;
> -       init_net.ipv6.ip6_blk_hole_entry->rt6i_idev = in6_dev_get(init_net.loopback_dev);
> -  #endif
>         ret = fib6_init();
>         if (ret)
>                 goto out_register_subsys;
> --
> 2.5.5
>