All of lore.kernel.org
 help / color / mirror / Atom feed
* pull request (net): ipsec 2017-08-21
@ 2017-08-21  6:07 Steffen Klassert
  2017-08-21  6:08 ` [PATCH 1/4] esp: Fix memleaks on error paths Steffen Klassert
                   ` (4 more replies)
  0 siblings, 5 replies; 6+ messages in thread
From: Steffen Klassert @ 2017-08-21  6:07 UTC (permalink / raw)
  To: David Miller; +Cc: Herbert Xu, Steffen Klassert, netdev

1) Fix memleaks when ESP takes an error path.

2) Fix null pointer dereference when creating a sub policy
   that matches the same outer flow as main policy does.
   From Koichiro Den.

3) Fix possible out-of-bound access in xfrm_migrate.
   This patch should go to the stable trees too.
   From Vladis Dronov.

4) ESP can return positive and negative error values,
   so treat both cases as an error.

Please pull or let me know if there are problems.

Thanks!

The following changes since commit edaf3825182958a1fd5e39708fcb6ea48eca2060:

  Merge git://git.kernel.org/pub/scm/linux/kernel/git/davem/net (2017-07-12 19:30:57 -0700)

are available in the git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/klassert/ipsec.git master

for you to fetch changes up to 4ff0308f06da5016aafb05330ed37809b54f81ae:

  esp: Fix error handling on layer 2 xmit. (2017-08-07 08:31:07 +0200)

----------------------------------------------------------------
Koichiro Den (1):
      xfrm: fix null pointer dereference on state and tmpl sort

Steffen Klassert (2):
      esp: Fix memleaks on error paths.
      esp: Fix error handling on layer 2 xmit.

Vladis Dronov (1):
      xfrm: policy: check policy direction value

 net/ipv4/esp4.c         | 13 ++++++++-----
 net/ipv4/esp4_offload.c |  2 +-
 net/ipv6/esp6.c         |  9 +++++----
 net/ipv6/esp6_offload.c |  2 +-
 net/xfrm/xfrm_policy.c  |  6 ++++++
 net/xfrm/xfrm_state.c   |  8 ++++++++
 6 files changed, 29 insertions(+), 11 deletions(-)

^ permalink raw reply	[flat|nested] 6+ messages in thread
* [PATCH 1/4] esp: Fix memleaks on error paths.
  2017-08-21  6:07 pull request (net): ipsec 2017-08-21 Steffen Klassert
@ 2017-08-21  6:08 ` Steffen Klassert
  2017-08-21  6:08 ` [PATCH 2/4] xfrm: fix null pointer dereference on state and tmpl sort Steffen Klassert
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 6+ messages in thread
From: Steffen Klassert @ 2017-08-21  6:08 UTC (permalink / raw)
  To: David Miller; +Cc: Herbert Xu, Steffen Klassert, netdev

We leak the temporary allocated resources in error paths,
fix this by freeing them.

Fixes: fca11ebde3f ("esp4: Reorganize esp_output")
Fixes: 383d0350f2c ("esp6: Reorganize esp_output")
Fixes: 3f29770723f ("ipsec: check return value of skb_to_sgvec always")
Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>
---
 net/ipv4/esp4.c | 13 ++++++++-----
 net/ipv6/esp6.c |  9 +++++----
 2 files changed, 13 insertions(+), 9 deletions(-)

diff --git a/net/ipv4/esp4.c b/net/ipv4/esp4.c
index 0cbee0a..dbb31a9 100644
--- a/net/ipv4/esp4.c
+++ b/net/ipv4/esp4.c
@@ -381,7 +381,7 @@ int esp_output_tail(struct xfrm_state *x, struct sk_buff *skb, struct esp_info *
 		           (unsigned char *)esph - skb->data,
 		           assoclen + ivlen + esp->clen + alen);
 	if (unlikely(err < 0))
-		goto error;
+		goto error_free;
 
 	if (!esp->inplace) {
 		int allocsize;
@@ -392,7 +392,7 @@ int esp_output_tail(struct xfrm_state *x, struct sk_buff *skb, struct esp_info *
 		spin_lock_bh(&x->lock);
 		if (unlikely(!skb_page_frag_refill(allocsize, pfrag, GFP_ATOMIC))) {
 			spin_unlock_bh(&x->lock);
-			goto error;
+			goto error_free;
 		}
 
 		skb_shinfo(skb)->nr_frags = 1;
@@ -409,7 +409,7 @@ int esp_output_tail(struct xfrm_state *x, struct sk_buff *skb, struct esp_info *
 			           (unsigned char *)esph - skb->data,
 			           assoclen + ivlen + esp->clen + alen);
 		if (unlikely(err < 0))
-			goto error;
+			goto error_free;
 	}
 
 	if ((x->props.flags & XFRM_STATE_ESN))
@@ -442,8 +442,9 @@ int esp_output_tail(struct xfrm_state *x, struct sk_buff *skb, struct esp_info *
 
 	if (sg != dsg)
 		esp_ssg_unref(x, tmp);
-	kfree(tmp);
 
+error_free:
+	kfree(tmp);
 error:
 	return err;
 }
@@ -695,8 +696,10 @@ static int esp_input(struct xfrm_state *x, struct sk_buff *skb)
 
 	sg_init_table(sg, nfrags);
 	err = skb_to_sgvec(skb, sg, 0, skb->len);
-	if (unlikely(err < 0))
+	if (unlikely(err < 0)) {
+		kfree(tmp);
 		goto out;
+	}
 
 	skb->ip_summed = CHECKSUM_NONE;
 
diff --git a/net/ipv6/esp6.c b/net/ipv6/esp6.c
index 9ed3547..392def1 100644
--- a/net/ipv6/esp6.c
+++ b/net/ipv6/esp6.c
@@ -345,7 +345,7 @@ int esp6_output_tail(struct xfrm_state *x, struct sk_buff *skb, struct esp_info
 		           (unsigned char *)esph - skb->data,
 		           assoclen + ivlen + esp->clen + alen);
 	if (unlikely(err < 0))
-		goto error;
+		goto error_free;
 
 	if (!esp->inplace) {
 		int allocsize;
@@ -356,7 +356,7 @@ int esp6_output_tail(struct xfrm_state *x, struct sk_buff *skb, struct esp_info
 		spin_lock_bh(&x->lock);
 		if (unlikely(!skb_page_frag_refill(allocsize, pfrag, GFP_ATOMIC))) {
 			spin_unlock_bh(&x->lock);
-			goto error;
+			goto error_free;
 		}
 
 		skb_shinfo(skb)->nr_frags = 1;
@@ -373,7 +373,7 @@ int esp6_output_tail(struct xfrm_state *x, struct sk_buff *skb, struct esp_info
 			           (unsigned char *)esph - skb->data,
 			           assoclen + ivlen + esp->clen + alen);
 		if (unlikely(err < 0))
-			goto error;
+			goto error_free;
 	}
 
 	if ((x->props.flags & XFRM_STATE_ESN))
@@ -406,8 +406,9 @@ int esp6_output_tail(struct xfrm_state *x, struct sk_buff *skb, struct esp_info
 
 	if (sg != dsg)
 		esp_ssg_unref(x, tmp);
-	kfree(tmp);
 
+error_free:
+	kfree(tmp);
 error:
 	return err;
 }
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 6+ messages in thread
* [PATCH 2/4] xfrm: fix null pointer dereference on state and tmpl sort
  2017-08-21  6:07 pull request (net): ipsec 2017-08-21 Steffen Klassert
  2017-08-21  6:08 ` [PATCH 1/4] esp: Fix memleaks on error paths Steffen Klassert
@ 2017-08-21  6:08 ` Steffen Klassert
  2017-08-21  6:08 ` [PATCH 3/4] xfrm: policy: check policy direction value Steffen Klassert
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 6+ messages in thread
From: Steffen Klassert @ 2017-08-21  6:08 UTC (permalink / raw)
  To: David Miller; +Cc: Herbert Xu, Steffen Klassert, netdev

From: Koichiro Den <den@klaipeden.com>

Creating sub policy that matches the same outer flow as main policy does
leads to a null pointer dereference if the outer mode's family is ipv4.
For userspace compatibility, this patch just eliminates the crash i.e.,
does not introduce any new sorting rule, which would fruitlessly affect
all but the aforementioned case.

Signed-off-by: Koichiro Den <den@klaipeden.com>
Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>
---
 net/xfrm/xfrm_state.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/net/xfrm/xfrm_state.c b/net/xfrm/xfrm_state.c
index 6c0956d..a792eff 100644
--- a/net/xfrm/xfrm_state.c
+++ b/net/xfrm/xfrm_state.c
@@ -1620,6 +1620,7 @@ int
 xfrm_tmpl_sort(struct xfrm_tmpl **dst, struct xfrm_tmpl **src, int n,
 	       unsigned short family, struct net *net)
 {
+	int i;
 	int err = 0;
 	struct xfrm_state_afinfo *afinfo = xfrm_state_get_afinfo(family);
 	if (!afinfo)
@@ -1628,6 +1629,9 @@ xfrm_tmpl_sort(struct xfrm_tmpl **dst, struct xfrm_tmpl **src, int n,
 	spin_lock_bh(&net->xfrm.xfrm_state_lock); /*FIXME*/
 	if (afinfo->tmpl_sort)
 		err = afinfo->tmpl_sort(dst, src, n);
+	else
+		for (i = 0; i < n; i++)
+			dst[i] = src[i];
 	spin_unlock_bh(&net->xfrm.xfrm_state_lock);
 	rcu_read_unlock();
 	return err;
@@ -1638,6 +1642,7 @@ int
 xfrm_state_sort(struct xfrm_state **dst, struct xfrm_state **src, int n,
 		unsigned short family)
 {
+	int i;
 	int err = 0;
 	struct xfrm_state_afinfo *afinfo = xfrm_state_get_afinfo(family);
 	struct net *net = xs_net(*src);
@@ -1648,6 +1653,9 @@ xfrm_state_sort(struct xfrm_state **dst, struct xfrm_state **src, int n,
 	spin_lock_bh(&net->xfrm.xfrm_state_lock);
 	if (afinfo->state_sort)
 		err = afinfo->state_sort(dst, src, n);
+	else
+		for (i = 0; i < n; i++)
+			dst[i] = src[i];
 	spin_unlock_bh(&net->xfrm.xfrm_state_lock);
 	rcu_read_unlock();
 	return err;
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 6+ messages in thread
* [PATCH 3/4] xfrm: policy: check policy direction value
  2017-08-21  6:07 pull request (net): ipsec 2017-08-21 Steffen Klassert
  2017-08-21  6:08 ` [PATCH 1/4] esp: Fix memleaks on error paths Steffen Klassert
  2017-08-21  6:08 ` [PATCH 2/4] xfrm: fix null pointer dereference on state and tmpl sort Steffen Klassert
@ 2017-08-21  6:08 ` Steffen Klassert
  2017-08-21  6:08 ` [PATCH 4/4] esp: Fix error handling on layer 2 xmit Steffen Klassert
  2017-08-22 17:27 ` pull request (net): ipsec 2017-08-21 David Miller
  4 siblings, 0 replies; 6+ messages in thread
From: Steffen Klassert @ 2017-08-21  6:08 UTC (permalink / raw)
  To: David Miller; +Cc: Herbert Xu, Steffen Klassert, netdev

From: Vladis Dronov <vdronov@redhat.com>

The 'dir' parameter in xfrm_migrate() is a user-controlled byte which is used
as an array index. This can lead to an out-of-bound access, kernel lockup and
DoS. Add a check for the 'dir' value.

This fixes CVE-2017-11600.

References: https://bugzilla.redhat.com/show_bug.cgi?id=1474928
Fixes: 80c9abaabf42 ("[XFRM]: Extension for dynamic update of endpoint address(es)")
Cc: <stable@vger.kernel.org> # v2.6.21-rc1
Reported-by: "bo Zhang" <zhangbo5891001@gmail.com>
Signed-off-by: Vladis Dronov <vdronov@redhat.com>
Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>
---
 net/xfrm/xfrm_policy.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/net/xfrm/xfrm_policy.c b/net/xfrm/xfrm_policy.c
index ff61d85..6f5a0dad 100644
--- a/net/xfrm/xfrm_policy.c
+++ b/net/xfrm/xfrm_policy.c
@@ -3308,9 +3308,15 @@ int xfrm_migrate(const struct xfrm_selector *sel, u8 dir, u8 type,
 	struct xfrm_state *x_new[XFRM_MAX_DEPTH];
 	struct xfrm_migrate *mp;
 
+	/* Stage 0 - sanity checks */
 	if ((err = xfrm_migrate_check(m, num_migrate)) < 0)
 		goto out;
 
+	if (dir >= XFRM_POLICY_MAX) {
+		err = -EINVAL;
+		goto out;
+	}
+
 	/* Stage 1 - find policy */
 	if ((pol = xfrm_migrate_policy_find(sel, dir, type, net)) == NULL) {
 		err = -ENOENT;
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 6+ messages in thread
* [PATCH 4/4] esp: Fix error handling on layer 2 xmit.
  2017-08-21  6:07 pull request (net): ipsec 2017-08-21 Steffen Klassert
                   ` (2 preceding siblings ...)
  2017-08-21  6:08 ` [PATCH 3/4] xfrm: policy: check policy direction value Steffen Klassert
@ 2017-08-21  6:08 ` Steffen Klassert
  2017-08-22 17:27 ` pull request (net): ipsec 2017-08-21 David Miller
  4 siblings, 0 replies; 6+ messages in thread
From: Steffen Klassert @ 2017-08-21  6:08 UTC (permalink / raw)
  To: David Miller; +Cc: Herbert Xu, Steffen Klassert, netdev

esp_output_tail() and esp6_output_tail() can return negative
and positive error values. We currently treat only negative
values as errors, fix this to treat both cases as error.

Fixes: fca11ebde3f0 ("esp4: Reorganize esp_output")
Fixes: 383d0350f2cc ("esp6: Reorganize esp_output")
Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>
---
 net/ipv4/esp4_offload.c | 2 +-
 net/ipv6/esp6_offload.c | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/net/ipv4/esp4_offload.c b/net/ipv4/esp4_offload.c
index e066601..5011232 100644
--- a/net/ipv4/esp4_offload.c
+++ b/net/ipv4/esp4_offload.c
@@ -257,7 +257,7 @@ static int esp_xmit(struct xfrm_state *x, struct sk_buff *skb,  netdev_features_
 	esp.seqno = cpu_to_be64(xo->seq.low + ((u64)xo->seq.hi << 32));
 
 	err = esp_output_tail(x, skb, &esp);
-	if (err < 0)
+	if (err)
 		return err;
 
 	secpath_reset(skb);
diff --git a/net/ipv6/esp6_offload.c b/net/ipv6/esp6_offload.c
index f02f131..1cf437f 100644
--- a/net/ipv6/esp6_offload.c
+++ b/net/ipv6/esp6_offload.c
@@ -286,7 +286,7 @@ static int esp6_xmit(struct xfrm_state *x, struct sk_buff *skb,  netdev_features
 	esp.seqno = cpu_to_be64(xo->seq.low + ((u64)xo->seq.hi << 32));
 
 	err = esp6_output_tail(x, skb, &esp);
-	if (err < 0)
+	if (err)
 		return err;
 
 	secpath_reset(skb);
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 6+ messages in thread
* Re: pull request (net): ipsec 2017-08-21
  2017-08-21  6:07 pull request (net): ipsec 2017-08-21 Steffen Klassert
                   ` (3 preceding siblings ...)
  2017-08-21  6:08 ` [PATCH 4/4] esp: Fix error handling on layer 2 xmit Steffen Klassert
@ 2017-08-22 17:27 ` David Miller
  4 siblings, 0 replies; 6+ messages in thread
From: David Miller @ 2017-08-22 17:27 UTC (permalink / raw)
  To: steffen.klassert; +Cc: herbert, netdev

From: Steffen Klassert <steffen.klassert@secunet.com>
Date: Mon, 21 Aug 2017 08:07:59 +0200

> 1) Fix memleaks when ESP takes an error path.
> 
> 2) Fix null pointer dereference when creating a sub policy
>    that matches the same outer flow as main policy does.
>    From Koichiro Den.
> 
> 3) Fix possible out-of-bound access in xfrm_migrate.
>    This patch should go to the stable trees too.
>    From Vladis Dronov.
> 
> 4) ESP can return positive and negative error values,
>    so treat both cases as an error.
> 
> Please pull or let me know if there are problems.

Pulled, thanks!

^ permalink raw reply	[flat|nested] 6+ messages in thread
end of thread, other threads:[~2017-08-22 17:27 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-08-21  6:07 pull request (net): ipsec 2017-08-21 Steffen Klassert
2017-08-21  6:08 ` [PATCH 1/4] esp: Fix memleaks on error paths Steffen Klassert
2017-08-21  6:08 ` [PATCH 2/4] xfrm: fix null pointer dereference on state and tmpl sort Steffen Klassert
2017-08-21  6:08 ` [PATCH 3/4] xfrm: policy: check policy direction value Steffen Klassert
2017-08-21  6:08 ` [PATCH 4/4] esp: Fix error handling on layer 2 xmit Steffen Klassert
2017-08-22 17:27 ` pull request (net): ipsec 2017-08-21 David Miller

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.