[PATCH -stable] ipsec: Fix aborted xfrm policy dump crash

[PATCH -stable] ipsec: Fix aborted xfrm policy dump crash

[Steffen Klassert]

Please queue up the following patch for -stable.
It fixes a security bug and should be applied to
all maintained stable versions.

From: Herbert Xu <herbert@gondor.apana.org.au>
Date: Thu, 19 Oct 2017 20:51:10 +0800
Subject: [PATCH] ipsec: Fix aborted xfrm policy dump crash

[ Upstream commit 1137b5e2529a8f5ca8ee709288ecba3e68044df2 ]

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")
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>
---
 net/xfrm/xfrm_user.c | 25 +++++++++++++++----------
 1 file changed, 15 insertions(+), 10 deletions(-)

diff --git a/net/xfrm/xfrm_user.c b/net/xfrm/xfrm_user.c
index b997f13..e44a0fe 100644
--- a/net/xfrm/xfrm_user.c
+++ b/net/xfrm/xfrm_user.c
@@ -1693,32 +1693,34 @@ static int dump_one_policy(struct xfrm_policy *xp, int dir, int count, void *ptr
 
 static int xfrm_dump_policy_done(struct netlink_callback *cb)
 {
-	struct xfrm_policy_walk *walk = (struct xfrm_policy_walk *) &cb->args[1];
+	struct xfrm_policy_walk *walk = (struct xfrm_policy_walk *)cb->args;
 	struct net *net = sock_net(cb->skb->sk);
 
 	xfrm_policy_walk_done(walk, net);
 	return 0;
 }
 
+static int xfrm_dump_policy_start(struct netlink_callback *cb)
+{
+	struct xfrm_policy_walk *walk = (struct xfrm_policy_walk *)cb->args;
+
+	BUILD_BUG_ON(sizeof(*walk) > sizeof(cb->args));
+
+	xfrm_policy_walk_init(walk, XFRM_POLICY_TYPE_ANY);
+	return 0;
+}
+
 static int xfrm_dump_policy(struct sk_buff *skb, struct netlink_callback *cb)
 {
 	struct net *net = sock_net(skb->sk);
-	struct xfrm_policy_walk *walk = (struct xfrm_policy_walk *) &cb->args[1];
+	struct xfrm_policy_walk *walk = (struct xfrm_policy_walk *)cb->args;
 	struct xfrm_dump_info info;
 
-	BUILD_BUG_ON(sizeof(struct xfrm_policy_walk) >
-		     sizeof(cb->args) - sizeof(cb->args[0]));
-
 	info.in_skb = cb->skb;
 	info.out_skb = skb;
 	info.nlmsg_seq = cb->nlh->nlmsg_seq;
 	info.nlmsg_flags = NLM_F_MULTI;
 
-	if (!cb->args[0]) {
-		cb->args[0] = 1;
-		xfrm_policy_walk_init(walk, XFRM_POLICY_TYPE_ANY);
-	}
-
 	(void) xfrm_policy_walk(net, walk, dump_one_policy, &info);
 
 	return skb->len;
@@ -2474,6 +2476,7 @@ static const struct nla_policy xfrma_spd_policy[XFRMA_SPD_MAX+1] = {
 
 static const struct xfrm_link {
 	int (*doit)(struct sk_buff *, struct nlmsghdr *, struct nlattr **);
+	int (*start)(struct netlink_callback *);
 	int (*dump)(struct sk_buff *, struct netlink_callback *);
 	int (*done)(struct netlink_callback *);
 	const struct nla_policy *nla_pol;
@@ -2487,6 +2490,7 @@ static const struct xfrm_link {
 	[XFRM_MSG_NEWPOLICY   - XFRM_MSG_BASE] = { .doit = xfrm_add_policy    },
 	[XFRM_MSG_DELPOLICY   - XFRM_MSG_BASE] = { .doit = xfrm_get_policy    },
 	[XFRM_MSG_GETPOLICY   - XFRM_MSG_BASE] = { .doit = xfrm_get_policy,
+						   .start = xfrm_dump_policy_start,
 						   .dump = xfrm_dump_policy,
 						   .done = xfrm_dump_policy_done },
 	[XFRM_MSG_ALLOCSPI    - XFRM_MSG_BASE] = { .doit = xfrm_alloc_userspi },
@@ -2539,6 +2543,7 @@ static int xfrm_user_rcv_msg(struct sk_buff *skb, struct nlmsghdr *nlh,
 
 		{
 			struct netlink_dump_control c = {
+				.start = link->start,
 				.dump = link->dump,
 				.done = link->done,
 			};
-- 
2.7.4

Re: [PATCH -stable] ipsec: Fix aborted xfrm policy dump crash

[Greg KH]

On Tue, Oct 24, 2017 at 02:01:35PM +0200, Steffen Klassert wrote:
> Please queue up the following patch for -stable.
> It fixes a security bug and should be applied to
> all maintained stable versions.
> 
> From: Herbert Xu <herbert@gondor.apana.org.au>
> Date: Thu, 19 Oct 2017 20:51:10 +0800
> Subject: [PATCH] ipsec: Fix aborted xfrm policy dump crash
> 
> [ Upstream commit 1137b5e2529a8f5ca8ee709288ecba3e68044df2 ]

That commit id is not in Linus's tree, where did you get it from?

thanks,

greg k-h

Re: [PATCH -stable] ipsec: Fix aborted xfrm policy dump crash

[Steffen Klassert]

On Tue, Oct 24, 2017 at 02:12:03PM +0200, Greg KH wrote:
> On Tue, Oct 24, 2017 at 02:01:35PM +0200, Steffen Klassert wrote:
> > Please queue up the following patch for -stable.
> > It fixes a security bug and should be applied to
> > all maintained stable versions.
> > 
> > From: Herbert Xu <herbert@gondor.apana.org.au>
> > Date: Thu, 19 Oct 2017 20:51:10 +0800
> > Subject: [PATCH] ipsec: Fix aborted xfrm policy dump crash
> > 
> > [ Upstream commit 1137b5e2529a8f5ca8ee709288ecba3e68044df2 ]
> 
> That commit id is not in Linus's tree, where did you get it from?

It is in the 'net' tree from David Miller. It was merged to 'net'
today from the ipsec tree.

Re: [PATCH -stable] ipsec: Fix aborted xfrm policy dump crash

[David Miller]

From: Steffen Klassert <steffen.klassert@secunet.com>
Date: Tue, 24 Oct 2017 14:01:35 +0200

> Please queue up the following patch for -stable.
> It fixes a security bug and should be applied to
> all maintained stable versions.

Indeed, Greg, please queue this up.

Thanks!

Re: [PATCH -stable] ipsec: Fix aborted xfrm policy dump crash

[Greg KH]

On Tue, Oct 24, 2017 at 02:14:47PM +0200, Steffen Klassert wrote:
> On Tue, Oct 24, 2017 at 02:12:03PM +0200, Greg KH wrote:
> > On Tue, Oct 24, 2017 at 02:01:35PM +0200, Steffen Klassert wrote:
> > > Please queue up the following patch for -stable.
> > > It fixes a security bug and should be applied to
> > > all maintained stable versions.
> > > 
> > > From: Herbert Xu <herbert@gondor.apana.org.au>
> > > Date: Thu, 19 Oct 2017 20:51:10 +0800
> > > Subject: [PATCH] ipsec: Fix aborted xfrm policy dump crash
> > > 
> > > [ Upstream commit 1137b5e2529a8f5ca8ee709288ecba3e68044df2 ]
> > 
> > That commit id is not in Linus's tree, where did you get it from?
> 
> It is in the 'net' tree from David Miller. It was merged to 'net'
> today from the ipsec tree.

Ok, it's now in Linus's tree, so now queued up, thanks!

greg k-h

Re: [PATCH -stable] ipsec: Fix aborted xfrm policy dump crash

[Greg KH]

On Mon, Oct 30, 2017 at 10:45:10AM +0100, Greg KH wrote:
> On Tue, Oct 24, 2017 at 02:14:47PM +0200, Steffen Klassert wrote:
> > On Tue, Oct 24, 2017 at 02:12:03PM +0200, Greg KH wrote:
> > > On Tue, Oct 24, 2017 at 02:01:35PM +0200, Steffen Klassert wrote:
> > > > Please queue up the following patch for -stable.
> > > > It fixes a security bug and should be applied to
> > > > all maintained stable versions.
> > > > 
> > > > From: Herbert Xu <herbert@gondor.apana.org.au>
> > > > Date: Thu, 19 Oct 2017 20:51:10 +0800
> > > > Subject: [PATCH] ipsec: Fix aborted xfrm policy dump crash
> > > > 
> > > > [ Upstream commit 1137b5e2529a8f5ca8ee709288ecba3e68044df2 ]
> > > 
> > > That commit id is not in Linus's tree, where did you get it from?
> > 
> > It is in the 'net' tree from David Miller. It was merged to 'net'
> > today from the ipsec tree.
> 
> Ok, it's now in Linus's tree, so now queued up, thanks!

But it fails in 4.4 due to api changes.  If you think it is needed
there, and you are able to, could you provide a backported patch?

thanks,

greg k-h

Re: [PATCH -stable] ipsec: Fix aborted xfrm policy dump crash

[Steffen Klassert]

On Mon, Oct 30, 2017 at 10:53:59AM +0100, Greg KH wrote:
> On Mon, Oct 30, 2017 at 10:45:10AM +0100, Greg KH wrote:
> > On Tue, Oct 24, 2017 at 02:14:47PM +0200, Steffen Klassert wrote:
> > > On Tue, Oct 24, 2017 at 02:12:03PM +0200, Greg KH wrote:
> > > > On Tue, Oct 24, 2017 at 02:01:35PM +0200, Steffen Klassert wrote:
> > > > > Please queue up the following patch for -stable.
> > > > > It fixes a security bug and should be applied to
> > > > > all maintained stable versions.
> > > > > 
> > > > > From: Herbert Xu <herbert@gondor.apana.org.au>
> > > > > Date: Thu, 19 Oct 2017 20:51:10 +0800
> > > > > Subject: [PATCH] ipsec: Fix aborted xfrm policy dump crash
> > > > > 
> > > > > [ Upstream commit 1137b5e2529a8f5ca8ee709288ecba3e68044df2 ]
> > > > 
> > > > That commit id is not in Linus's tree, where did you get it from?
> > > 
> > > It is in the 'net' tree from David Miller. It was merged to 'net'
> > > today from the ipsec tree.
> > 
> > Ok, it's now in Linus's tree, so now queued up, thanks!
> 
> But it fails in 4.4 due to api changes.  If you think it is needed
> there, and you are able to, could you provide a backported patch?

I'll have a look at it, thanks for the info!