xfrm: policy: fix reinsertion on node merge

"newpos" has wrong scope.  It must be NULL on each iteration of the loop.
Otherwise, when policy is to be inserted at the start, we would instead
insert at point found by the previous loop-iteration instead.

Also, we need to unlink the policy before we reinsert it to the new node,
else we can get next-points-to-self loops.

Because policies are only ordered by priority it is irrelevant which policy
is "more recent" except when two policies have same priority.
(the more recent one is placed after the older one).

In these cases, we can use the ->pos id number to know which one is the
'older': the higher the id, the more recent the policy.

So we only need to unlink all policies from the node that is about to be
removed, and insert them to the replacement node.

Fixes: 9cf545ebd591da ("xfrm: policy: store inexact policies in a tree ordered by destination address")
Signed-off-by: Florian Westphal <fw@strlen.de>
Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>

diff --git a/net/xfrm/xfrm_policy.c b/net/xfrm/xfrm_policy.c
index 24dfd1e47cf085f00bc129f60e0d439e315df3f2..e691683223ee5e40d19c7428e9cdd2d6fe3923e9 100644 (file)
--- a/net/xfrm/xfrm_policy.c
+++ b/net/xfrm/xfrm_policy.c
@@ -823,13 +823,13 @@ static void xfrm_policy_inexact_list_reinsert(struct net *net,
                                              u16 family)
 {
        unsigned int matched_s, matched_d;
-       struct hlist_node *newpos = NULL;
        struct xfrm_policy *policy, *p;
 
        matched_s = 0;
        matched_d = 0;
 
        list_for_each_entry_reverse(policy, &net->xfrm.policy_all, walk.all) {
+               struct hlist_node *newpos = NULL;
                bool matches_s, matches_d;
 
                if (!policy->bydst_reinsert)
@@ -839,7 +839,10 @@ static void xfrm_policy_inexact_list_reinsert(struct net *net,
 
                policy->bydst_reinsert = false;
                hlist_for_each_entry(p, &n->hhead, bydst) {
-                       if (policy->priority >= p->priority)
+                       if (policy->priority > p->priority)
+                               newpos = &p->bydst;
+                       else if (policy->priority == p->priority &&
+                                policy->pos > p->pos)
                                newpos = &p->bydst;
                        else
                                break;
@@ -955,12 +958,11 @@ static void xfrm_policy_inexact_node_merge(struct net *net,
                                                  family);
        }
 
-       hlist_for_each_entry(tmp, &v->hhead, bydst)
-               tmp->bydst_reinsert = true;
-       hlist_for_each_entry(tmp, &n->hhead, bydst)
+       hlist_for_each_entry(tmp, &v->hhead, bydst) {
                tmp->bydst_reinsert = true;
+               hlist_del_rcu(&tmp->bydst);
+       }
 
-       INIT_HLIST_HEAD(&n->hhead);
        xfrm_policy_inexact_list_reinsert(net, n, family);
 }