Commit 769a807d authored Aug 12, 2019 by Florian Westphal Committed by Steffen Klassert Aug 20, 2019

xfrm: policy: avoid warning splat when merging nodes



syzbot reported a splat:
 xfrm_policy_inexact_list_reinsert+0x625/0x6e0 net/xfrm/xfrm_policy.c:877
 CPU: 1 PID: 6756 Comm: syz-executor.1 Not tainted 5.3.0-rc2+ #57
 Call Trace:
  xfrm_policy_inexact_node_reinsert net/xfrm/xfrm_policy.c:922 [inline]
  xfrm_policy_inexact_node_merge net/xfrm/xfrm_policy.c:958 [inline]
  xfrm_policy_inexact_insert_node+0x537/0xb50 net/xfrm/xfrm_policy.c:1023
  xfrm_policy_inexact_alloc_chain+0x62b/0xbd0 net/xfrm/xfrm_policy.c:1139
  xfrm_policy_inexact_insert+0xe8/0x1540 net/xfrm/xfrm_policy.c:1182
  xfrm_policy_insert+0xdf/0xce0 net/xfrm/xfrm_policy.c:1574
  xfrm_add_policy+0x4cf/0x9b0 net/xfrm/xfrm_user.c:1670
  xfrm_user_rcv_msg+0x46b/0x720 net/xfrm/xfrm_user.c:2676
  netlink_rcv_skb+0x1f0/0x460 net/netlink/af_netlink.c:2477
  xfrm_netlink_rcv+0x74/0x90 net/xfrm/xfrm_user.c:2684
  netlink_unicast_kernel net/netlink/af_netlink.c:1302 [inline]
  netlink_unicast+0x809/0x9a0 net/netlink/af_netlink.c:1328
  netlink_sendmsg+0xa70/0xd30 net/netlink/af_netlink.c:1917
  sock_sendmsg_nosec net/socket.c:637 [inline]
  sock_sendmsg net/socket.c:657 [inline]

There is no reproducer, however, the warning can be reproduced
by adding rules with ever smaller prefixes.

The sanity check ("does the policy match the node") uses the prefix value
of the node before its updated to the smaller value.

To fix this, update the prefix earlier.  The bug has no impact on tree
correctness, this is only to prevent a false warning.

Reported-by:  <syzbot+8cc27ace5f6972910b31@syzkaller.appspotmail.com>
Signed-off-by: Florian Westphal <fw@strlen.de>
Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>

parent 22d6552f

net/xfrm/xfrm_policy.c

+4 −2

Original line number	Diff line number	Diff line
		@@ -912,6 +912,7 @@ restart:
		} else if (delta > 0) {
		p = &parent->rb_right;
		} else {
		bool same_prefixlen = node->prefixlen == n->prefixlen;
		struct xfrm_policy *tmp;

		hlist_for_each_entry(tmp, &n->hhead, bydst) {
		@@ -919,9 +920,11 @@ restart:
		hlist_del_rcu(&tmp->bydst);
		}

		node->prefixlen = prefixlen;

		xfrm_policy_inexact_list_reinsert(net, node, family);

		if (node->prefixlen == n->prefixlen) {
		if (same_prefixlen) {
		kfree_rcu(n, rcu);
		return;
		}
		@@ -929,7 +932,6 @@ restart:
		rb_erase(*p, new);
		kfree_rcu(n, rcu);
		n = node;
		n->prefixlen = prefixlen;
		goto restart;
		}
		}

tools/testing/selftests/net/xfrm_policy.sh

+7 −0

Original line number	Diff line number	Diff line
		@@ -106,6 +106,13 @@ do_overlap()
		#
		# 10.0.0.0/24 and 10.0.1.0/24 nodes have been merged as 10.0.0.0/23.
		ip -net $ns xfrm policy add src 10.1.0.0/24 dst 10.0.0.0/23 dir fwd priority 200 action block

		# similar to above: add policies (with partially random address), with shrinking prefixes.
		for p in 29 28 27;do
		for k in $(seq 1 32); do
		ip -net $ns xfrm policy add src 10.253.1.$((RANDOM%255))/$p dst 10.254.1.$((RANDOM%255))/$p dir fwd priority $((200+k)) action block 2>/dev/null
		done
		done
		}

		do_esp_policy_get_check() {

Admin message