[Devel] [PATCH RHEL7 COMMIT] ms/ipsec: Fix aborted xfrm policy dump crash

[Konstantin Khorenko]

The commit is pushed to "branch-rh7-3.10.0-693.11.1.vz7.39.x-ovz" and will appear at https://src.openvz.org/scm/ovz/vzkernel.git
after rh7-3.10.0-693.11.1.vz7.39.1
------>
commit 076978f402f69d9ea84b91a1a5173777b5cc9363
Author: Evgenii Shatokhin <eshatokhin at virtuozzo.com>
Date:   Thu Dec 7 14:42:58 2017 +0300

    ms/ipsec: Fix aborted xfrm policy dump crash
    
    The patch fixes the same issue as mainline commit 1137b5e2529a ("ipsec: Fix
    aborted xfrm policy dump crash") but in a different way.
    
    The root of the problem is that xfrm_policy_walk_done() may be called when
    xfrm_policy_walk instance is not initialized yet, which can result in an
    instant kernel crash or, potentially, memory corruption.
    
    The mainlined patch uses cb->start callback to make sure
    xfrm_policy_walk instance is initialized first. However, such callbacks were
    only introduced by mainline commit fc9e50f5a5a4 ("netlink: add a start
    callback for starting a netlink dump"), which is not present in vzkernel
    yet. One option would be to backport that and use the original patch from
    the mainline but, in this particular case, it might be easier to just call
    xfrm_policy_walk_init() directly, like xfrm_dump_policy() does.
    
    Description of the original patch:
    
      An independent security researcher, Mohamed Ghannam, has reported
      this vulnerability to Beyond Security's SecuriTeam Secure Disclosure
      program.
    
      The xfrm_dump_policy_done function expects xfrm_dump_policy to
      have been called at least once or it will crash.  This can be
      triggered if a dump fails because the target socket's receive
      buffer is full.
    
      This patch fixes it by using the cb->start mechanism to ensure that
      the initialisation is always done regardless of the buffer situation.
    
      Fixes: 12a169e7d8f4 ("ipsec: Put dumpers on the dump list")
    
    CVE-2017-16939
    https://jira.sw.ru/browse/PSBM-78287
    
    Signed-off-by: Evgenii Shatokhin <eshatokhin at virtuozzo.com>
---
 net/xfrm/xfrm_user.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/net/xfrm/xfrm_user.c b/net/xfrm/xfrm_user.c
index 64329a11b9ec..7afdff20a733 100644
--- a/net/xfrm/xfrm_user.c
+++ b/net/xfrm/xfrm_user.c
@@ -1616,6 +1616,14 @@ static int xfrm_dump_policy_done(struct netlink_callback *cb)
 	struct xfrm_policy_walk *walk = (struct xfrm_policy_walk *) &cb->args[1];
 	struct net *net = sock_net(cb->skb->sk);
 
+	/*
+	 * .done callback runs only once for a given 'cb', so there is no
+	 * need to  set cb->args[0] to indicate that xfrm_policy_walk_init()
+	 * has already been called.
+	 */
+	if (!cb->args[0])
+		xfrm_policy_walk_init(walk, XFRM_POLICY_TYPE_ANY);
+
 	xfrm_policy_walk_done(walk, net);
 	return 0;
 }