| 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
| ||
|
Message-ID: <54105185.7010206@gmail.com>
Date: Wed, 10 Sep 2014 09:26:29 -0400
From: Vlad Yasevich <vyasevich@...il.com>
To: Hannes Frederic Sowa <hannes@...essinduktion.org>,
netdev@...r.kernel.org
CC: Eric Dumazet <eric.dumazet@...il.com>,
Nicolas Dichtel <nicolas.dichtel@...nd.com>
Subject: Re: [PATCH net-next] ipv6: implement rt_genid_bump_ipv6 with fn_sernum
and remove rt6i_genid
On 09/10/2014 05:31 AM, Hannes Frederic Sowa wrote:
> Some special routes will never be cloned in IPv6. Those are mainly
> DST_HOST routes without RTF_NONEXTHOP or RTF_GATEWAY flags, thus mostly
> routes handling the input path.
>
> rt_genid depends on rt6_info clones being removed from the trie and
> recreated with current rt6i_genid, thus bumping the genid would invalidate
> all routes cached in the sockets and relookup will recreate them. But in
> case a non-cloned route ends up in the per-socket cache, it will never
> be recreated, thus will never get a current rt_genid.
>
> After the rt_genid is incremented for the first time all those routes
> will always get invalidated by ip6_dst_check on the next check, thus
> making early socket demultiplexing absolutely pointless for IPv6. It is
> not possible to solve this with rt6i_genid, thus remove it.
>
> In case we need to force the sockets to relookup the routes we now
> increase the fn_sernum on all fibnodes in the routing tree. This is a
> costly operation but should only happen if we have major routing/policy
> changes in the kernel (e.g. manual route adding/removal, xfrm policy
> changes). Also this patch optimized the walk over the trie a bit, we
> don't touch every rt6_info but only touch the fib6_nodes.
>
> Thanks to Eric Dumazet who noticed this problem.
Hi Hannes
I also noticed the ipv6_ifa_notify() is called a lot with even being
0. This will only trigger rtnl notification, but would not trigger any
routing table changes, but would trigger a call to bump the gen_id
which now would perform a rather expensive clean-table operation.
In particular the loop in manage_tempaddrs() is very scary as it can
bump the rev multiples times.
I think it the genid bump in __ipv6_ifa_notify() should only happen
if there was an actual address change.
-vlad
>
> Fixes: 6f3118b571b8 ("ipv6: use net->rt_genid to check dst validity")
> Cc: Eric Dumazet <eric.dumazet@...il.com>
> Cc: Nicolas Dichtel <nicolas.dichtel@...nd.com>
> Signed-off-by: Hannes Frederic Sowa <hannes@...essinduktion.org>
> ---
>
> Notes:
> I based this patch on net-next, because it "only" fixes a performance
> problem so far and want it to be a bit more exposed to testing before
> a possible submission to stable.
>
> include/net/ip6_fib.h | 5 ++--
> include/net/net_namespace.h | 4 +++-
> net/core/dst.c | 8 +++++++
> net/ipv6/ip6_fib.c | 57 ++++++++++++++++++++++++++++++++++++---------
> net/ipv6/route.c | 4 ----
> 5 files changed, 59 insertions(+), 19 deletions(-)
>
> diff --git a/include/net/ip6_fib.h b/include/net/ip6_fib.h
> index 9bcb220..a5e09b8 100644
> --- a/include/net/ip6_fib.h
> +++ b/include/net/ip6_fib.h
> @@ -119,8 +119,6 @@ struct rt6_info {
> struct inet6_dev *rt6i_idev;
> unsigned long _rt6i_peer;
>
> - u32 rt6i_genid;
> -
> /* more non-fragment space at head required */
> unsigned short rt6i_nfheader_len;
>
> @@ -281,7 +279,8 @@ struct fib6_node *fib6_locate(struct fib6_node *root,
> const struct in6_addr *daddr, int dst_len,
> const struct in6_addr *saddr, int src_len);
>
> -void fib6_clean_all(struct net *net, int (*func)(struct rt6_info *, void *arg),
> +void fib6_clean_all(struct net *net,
> + int (*rt_visit)(struct rt6_info *, void *arg),
> void *arg);
>
> int fib6_add(struct fib6_node *root, struct rt6_info *rt, struct nl_info *info,
> diff --git a/include/net/net_namespace.h b/include/net/net_namespace.h
> index 361d260..e0b0476 100644
> --- a/include/net/net_namespace.h
> +++ b/include/net/net_namespace.h
> @@ -358,9 +358,11 @@ static inline int rt_genid_ipv6(struct net *net)
> return atomic_read(&net->ipv6.rt_genid);
> }
>
> +extern void (*__rt_genid_bump_ipv6)(struct net *net);
> static inline void rt_genid_bump_ipv6(struct net *net)
> {
> - atomic_inc(&net->ipv6.rt_genid);
> + if (__rt_genid_bump_ipv6)
> + __rt_genid_bump_ipv6(net);
> }
> #else
> static inline int rt_genid_ipv6(struct net *net)
> diff --git a/net/core/dst.c b/net/core/dst.c
> index a028409..995183b 100644
> --- a/net/core/dst.c
> +++ b/net/core/dst.c
> @@ -23,6 +23,14 @@
>
> #include <net/dst.h>
>
> +/* as soon as the ipv6 module registers, this function is used to
> + * force all cached routing lookups to relookup
> + */
> +#if IS_ENABLED(CONFIG_IPV6)
> +void (*__rt_genid_bump_ipv6)(struct net *net);
> +EXPORT_SYMBOL(__rt_genid_bump_ipv6);
> +#endif
> +
> /*
> * Theory of operations:
> * 1) We use a list, protected by a spinlock, to add
> diff --git a/net/ipv6/ip6_fib.c b/net/ipv6/ip6_fib.c
> index 76b7f5e..d23b76f 100644
> --- a/net/ipv6/ip6_fib.c
> +++ b/net/ipv6/ip6_fib.c
> @@ -106,10 +106,15 @@ static inline void fib6_walker_unlink(struct fib6_walker_t *w)
> }
> static __inline__ u32 fib6_new_sernum(void)
> {
> - u32 n = ++rt_sernum;
> - if ((__s32)n <= 0)
> - rt_sernum = n = 1;
> - return n;
> + u32 new, old;
> +
> + do {
> + old = ACCESS_ONCE(rt_sernum);
> + new = old + 1;
> + if ((s32)new <= 0)
> + new = 1;
> + } while (cmpxchg(&rt_sernum, old, new) != old);
> + return new;
> }
>
> /*
> @@ -1553,25 +1558,28 @@ static int fib6_clean_node(struct fib6_walker_t *w)
> */
>
> static void fib6_clean_tree(struct net *net, struct fib6_node *root,
> - int (*func)(struct rt6_info *, void *arg),
> + int (*node_visit)(struct fib6_walker_t *),
> + int (*rt_visit)(struct rt6_info *, void *arg),
> int prune, void *arg)
> {
> struct fib6_cleaner_t c;
>
> c.w.root = root;
> - c.w.func = fib6_clean_node;
> + c.w.func = node_visit;
> c.w.prune = prune;
> c.w.count = 0;
> c.w.skip = 0;
> - c.func = func;
> + c.func = rt_visit;
> c.arg = arg;
> c.net = net;
>
> fib6_walk(&c.w);
> }
>
> -void fib6_clean_all(struct net *net, int (*func)(struct rt6_info *, void *arg),
> - void *arg)
> +static void __fib6_clean_all(struct net *net,
> + int (*node_visit)(struct fib6_walker_t *),
> + int (*rt_visit)(struct rt6_info *, void *arg),
> + void *arg)
> {
> struct fib6_table *table;
> struct hlist_head *head;
> @@ -1583,13 +1591,20 @@ void fib6_clean_all(struct net *net, int (*func)(struct rt6_info *, void *arg),
> hlist_for_each_entry_rcu(table, head, tb6_hlist) {
> write_lock_bh(&table->tb6_lock);
> fib6_clean_tree(net, &table->tb6_root,
> - func, 0, arg);
> + node_visit, rt_visit, 0, arg);
> write_unlock_bh(&table->tb6_lock);
> }
> }
> rcu_read_unlock();
> }
>
> +void fib6_clean_all(struct net *net,
> + int (*rt_visit)(struct rt6_info *, void *arg),
> + void *arg)
> +{
> + __fib6_clean_all(net, fib6_clean_node, rt_visit, arg);
> +}
> +
> static int fib6_prune_clone(struct rt6_info *rt, void *arg)
> {
> if (rt->rt6i_flags & RTF_CACHE) {
> @@ -1602,7 +1617,25 @@ static int fib6_prune_clone(struct rt6_info *rt, void *arg)
>
> static void fib6_prune_clones(struct net *net, struct fib6_node *fn)
> {
> - fib6_clean_tree(net, fn, fib6_prune_clone, 1, NULL);
> + fib6_clean_tree(net, fn, fib6_clean_node, fib6_prune_clone, 1, NULL);
> +}
> +
> +static int fib6_upd_sernum(struct fib6_walker_t *w)
> +{
> + struct fib6_cleaner_t *c = container_of(w, struct fib6_cleaner_t, w);
> + int *sernum = c->arg;
> +
> + w->node->fn_sernum = *sernum;
> + w->leaf = NULL;
> + return 0;
> +}
> +
> +static void fib6_genid_bump(struct net *net)
> +{
> + int sernum;
> +
> + sernum = fib6_new_sernum();
> + __fib6_clean_all(net, fib6_upd_sernum, NULL, &sernum);
> }
>
> /*
> @@ -1788,6 +1821,8 @@ int __init fib6_init(void)
> NULL);
> if (ret)
> goto out_unregister_subsys;
> +
> + __rt_genid_bump_ipv6 = fib6_genid_bump;
> out:
> return ret;
>
> diff --git a/net/ipv6/route.c b/net/ipv6/route.c
> index f74b041..a318dd89 100644
> --- a/net/ipv6/route.c
> +++ b/net/ipv6/route.c
> @@ -314,7 +314,6 @@ static inline struct rt6_info *ip6_dst_alloc(struct net *net,
>
> memset(dst + 1, 0, sizeof(*rt) - sizeof(*dst));
> rt6_init_peer(rt, table ? &table->tb6_peers : net->ipv6.peers);
> - rt->rt6i_genid = rt_genid_ipv6(net);
> INIT_LIST_HEAD(&rt->rt6i_siblings);
> }
> return rt;
> @@ -1096,9 +1095,6 @@ static struct dst_entry *ip6_dst_check(struct dst_entry *dst, u32 cookie)
> * DST_OBSOLETE_FORCE_CHK which forces validation calls down
> * into this function always.
> */
> - if (rt->rt6i_genid != rt_genid_ipv6(dev_net(rt->dst.dev)))
> - return NULL;
> -
> if (!rt->rt6i_node || (rt->rt6i_node->fn_sernum != cookie))
> return NULL;
>
>
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Powered by blists - more mailing lists