All of lore.kernel.org
 help / color / mirror / Atom feed
* pull request (net): ipsec 2017-08-29
@ 2017-08-29 10:31 Steffen Klassert
  2017-08-29 10:31 ` [PATCH 1/7] net: xfrm: don't double-hold dst when sk_policy in use Steffen Klassert
                   ` (7 more replies)
  0 siblings, 8 replies; 9+ messages in thread
From: Steffen Klassert @ 2017-08-29 10:31 UTC (permalink / raw)
  To: David Miller; +Cc: Herbert Xu, Steffen Klassert, netdev

1) Fix dst_entry refcount imbalance when using socket policies.
   From Lorenzo Colitti.

2) Fix locking when adding the ESP trailers.

3) Fix tailroom calculation for the ESP trailer by using
   skb_tailroom instead of skb_availroom.

4) Fix some info leaks in xfrm_user.
   From Mathias Krause.

Please pull or let me know if there are problems.

Thanks!

The following changes since commit 2b33bc8aa236b75d6e86a8a79126fd9739e4a5bd:

  net: dsa: use consume_skb() (2017-08-23 22:13:34 -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 931e79d7a7ddee4709c56b39de169a36804589a1:

  xfrm_user: fix info leak in build_aevent() (2017-08-28 10:58:02 +0200)

----------------------------------------------------------------
Lorenzo Colitti (1):
      net: xfrm: don't double-hold dst when sk_policy in use.

Mathias Krause (4):
      xfrm_user: fix info leak in copy_user_offload()
      xfrm_user: fix info leak in xfrm_notify_sa()
      xfrm_user: fix info leak in build_expire()
      xfrm_user: fix info leak in build_aevent()

Steffen Klassert (2):
      esp: Fix locking on page fragment allocation
      esp: Fix skb tailroom calculation

 net/ipv4/esp4.c        | 7 ++++---
 net/ipv6/esp6.c        | 7 ++++---
 net/xfrm/xfrm_policy.c | 1 -
 net/xfrm/xfrm_user.c   | 6 +++++-
 4 files changed, 13 insertions(+), 8 deletions(-)

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH 1/7] net: xfrm: don't double-hold dst when sk_policy in use.
  2017-08-29 10:31 pull request (net): ipsec 2017-08-29 Steffen Klassert
@ 2017-08-29 10:31 ` Steffen Klassert
  2017-08-29 10:31 ` [PATCH 2/7] esp: Fix locking on page fragment allocation Steffen Klassert
                   ` (6 subsequent siblings)
  7 siblings, 0 replies; 9+ messages in thread
From: Steffen Klassert @ 2017-08-29 10:31 UTC (permalink / raw)
  To: David Miller; +Cc: Herbert Xu, Steffen Klassert, netdev

From: Lorenzo Colitti <lorenzo@google.com>

While removing dst_entry garbage collection, commit 52df157f17e5
("xfrm: take refcnt of dst when creating struct xfrm_dst bundle")
changed xfrm_resolve_and_create_bundle so it returns an xdst with
a refcount of 1 instead of 0.

However, it did not delete the dst_hold performed by xfrm_lookup
when a per-socket policy is in use. This means that when a
socket policy is in use, dst entries returned by xfrm_lookup have
a refcount of 2, and are not freed when no longer in use.

Cc: Wei Wang <weiwan@google.com>
Fixes: 52df157f17 ("xfrm: take refcnt of dst when creating struct xfrm_dst bundle")
Tested: https://android-review.googlesource.com/417481
Tested: https://android-review.googlesource.com/418659
Tested: https://android-review.googlesource.com/424463
Tested: https://android-review.googlesource.com/452776 passes on net-next
Signed-off-by: Lorenzo Colitti <lorenzo@google.com>
Acked-by: Wei Wang <weiwan@google.com>
Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>
---
 net/xfrm/xfrm_policy.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/net/xfrm/xfrm_policy.c b/net/xfrm/xfrm_policy.c
index 6f5a0dad..69b16ee 100644
--- a/net/xfrm/xfrm_policy.c
+++ b/net/xfrm/xfrm_policy.c
@@ -2226,7 +2226,6 @@ struct dst_entry *xfrm_lookup(struct net *net, struct dst_entry *dst_orig,
 				goto no_transform;
 			}
 
-			dst_hold(&xdst->u.dst);
 			route = xdst->route;
 		}
 	}
-- 
2.7.4

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH 2/7] esp: Fix locking on page fragment allocation
  2017-08-29 10:31 pull request (net): ipsec 2017-08-29 Steffen Klassert
  2017-08-29 10:31 ` [PATCH 1/7] net: xfrm: don't double-hold dst when sk_policy in use Steffen Klassert
@ 2017-08-29 10:31 ` Steffen Klassert
  2017-08-29 10:31 ` [PATCH 3/7] esp: Fix skb tailroom calculation Steffen Klassert
                   ` (5 subsequent siblings)
  7 siblings, 0 replies; 9+ messages in thread
From: Steffen Klassert @ 2017-08-29 10:31 UTC (permalink / raw)
  To: David Miller; +Cc: Herbert Xu, Steffen Klassert, netdev

We allocate the page fragment for the ESP trailer inside
a spinlock, but consume it outside of the lock. This
is racy as some other cou could get the same page fragment
then. Fix this by consuming the page fragment inside the
lock too.

Fixes: cac2661c53f3 ("esp4: Avoid skb_cow_data whenever possible")
Fixes: 03e2a30f6a27 ("esp6: Avoid skb_cow_data whenever possible")
Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>
---
 net/ipv4/esp4.c | 5 +++--
 net/ipv6/esp6.c | 5 +++--
 2 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/net/ipv4/esp4.c b/net/ipv4/esp4.c
index dbb31a9..a8ddb95 100644
--- a/net/ipv4/esp4.c
+++ b/net/ipv4/esp4.c
@@ -292,8 +292,6 @@ int esp_output_head(struct xfrm_state *x, struct sk_buff *skb, struct esp_info *
 
 			kunmap_atomic(vaddr);
 
-			spin_unlock_bh(&x->lock);
-
 			nfrags = skb_shinfo(skb)->nr_frags;
 
 			__skb_fill_page_desc(skb, nfrags, page, pfrag->offset,
@@ -301,6 +299,9 @@ int esp_output_head(struct xfrm_state *x, struct sk_buff *skb, struct esp_info *
 			skb_shinfo(skb)->nr_frags = ++nfrags;
 
 			pfrag->offset = pfrag->offset + allocsize;
+
+			spin_unlock_bh(&x->lock);
+
 			nfrags++;
 
 			skb->len += tailen;
diff --git a/net/ipv6/esp6.c b/net/ipv6/esp6.c
index 392def1..4e3fdc88 100644
--- a/net/ipv6/esp6.c
+++ b/net/ipv6/esp6.c
@@ -260,8 +260,6 @@ int esp6_output_head(struct xfrm_state *x, struct sk_buff *skb, struct esp_info
 
 			kunmap_atomic(vaddr);
 
-			spin_unlock_bh(&x->lock);
-
 			nfrags = skb_shinfo(skb)->nr_frags;
 
 			__skb_fill_page_desc(skb, nfrags, page, pfrag->offset,
@@ -269,6 +267,9 @@ int esp6_output_head(struct xfrm_state *x, struct sk_buff *skb, struct esp_info
 			skb_shinfo(skb)->nr_frags = ++nfrags;
 
 			pfrag->offset = pfrag->offset + allocsize;
+
+			spin_unlock_bh(&x->lock);
+
 			nfrags++;
 
 			skb->len += tailen;
-- 
2.7.4

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH 3/7] esp: Fix skb tailroom calculation
  2017-08-29 10:31 pull request (net): ipsec 2017-08-29 Steffen Klassert
  2017-08-29 10:31 ` [PATCH 1/7] net: xfrm: don't double-hold dst when sk_policy in use Steffen Klassert
  2017-08-29 10:31 ` [PATCH 2/7] esp: Fix locking on page fragment allocation Steffen Klassert
@ 2017-08-29 10:31 ` Steffen Klassert
  2017-08-29 10:31 ` [PATCH 4/7] xfrm_user: fix info leak in copy_user_offload() Steffen Klassert
                   ` (4 subsequent siblings)
  7 siblings, 0 replies; 9+ messages in thread
From: Steffen Klassert @ 2017-08-29 10:31 UTC (permalink / raw)
  To: David Miller; +Cc: Herbert Xu, Steffen Klassert, netdev

We use skb_availroom to calculate the skb tailroom for the
ESP trailer. skb_availroom calculates the tailroom and
subtracts this value by reserved_tailroom. However
reserved_tailroom is a union with the skb mark. This means
that we subtract the tailroom by the skb mark if set.
Fix this by using skb_tailroom instead.

Fixes: cac2661c53f3 ("esp4: Avoid skb_cow_data whenever possible")
Fixes: 03e2a30f6a27 ("esp6: Avoid skb_cow_data whenever possible")
Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>
---
 net/ipv4/esp4.c | 2 +-
 net/ipv6/esp6.c | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/net/ipv4/esp4.c b/net/ipv4/esp4.c
index a8ddb95..df68963 100644
--- a/net/ipv4/esp4.c
+++ b/net/ipv4/esp4.c
@@ -258,7 +258,7 @@ int esp_output_head(struct xfrm_state *x, struct sk_buff *skb, struct esp_info *
 		esp_output_udp_encap(x, skb, esp);
 
 	if (!skb_cloned(skb)) {
-		if (tailen <= skb_availroom(skb)) {
+		if (tailen <= skb_tailroom(skb)) {
 			nfrags = 1;
 			trailer = skb;
 			tail = skb_tail_pointer(trailer);
diff --git a/net/ipv6/esp6.c b/net/ipv6/esp6.c
index 4e3fdc88..ab64f36 100644
--- a/net/ipv6/esp6.c
+++ b/net/ipv6/esp6.c
@@ -226,7 +226,7 @@ int esp6_output_head(struct xfrm_state *x, struct sk_buff *skb, struct esp_info
 	int tailen = esp->tailen;
 
 	if (!skb_cloned(skb)) {
-		if (tailen <= skb_availroom(skb)) {
+		if (tailen <= skb_tailroom(skb)) {
 			nfrags = 1;
 			trailer = skb;
 			tail = skb_tail_pointer(trailer);
-- 
2.7.4

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH 4/7] xfrm_user: fix info leak in copy_user_offload()
  2017-08-29 10:31 pull request (net): ipsec 2017-08-29 Steffen Klassert
                   ` (2 preceding siblings ...)
  2017-08-29 10:31 ` [PATCH 3/7] esp: Fix skb tailroom calculation Steffen Klassert
@ 2017-08-29 10:31 ` Steffen Klassert
  2017-08-29 10:31 ` [PATCH 5/7] xfrm_user: fix info leak in xfrm_notify_sa() Steffen Klassert
                   ` (3 subsequent siblings)
  7 siblings, 0 replies; 9+ messages in thread
From: Steffen Klassert @ 2017-08-29 10:31 UTC (permalink / raw)
  To: David Miller; +Cc: Herbert Xu, Steffen Klassert, netdev

From: Mathias Krause <minipli@googlemail.com>

The memory reserved to dump the xfrm offload state includes padding
bytes of struct xfrm_user_offload added by the compiler for alignment.
Add an explicit memset(0) before filling the buffer to avoid the heap
info leak.

Cc: Steffen Klassert <steffen.klassert@secunet.com>
Fixes: d77e38e612a0 ("xfrm: Add an IPsec hardware offloading API")
Signed-off-by: Mathias Krause <minipli@googlemail.com>
Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>
---
 net/xfrm/xfrm_user.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/xfrm/xfrm_user.c b/net/xfrm/xfrm_user.c
index 2be4c6a..3259555 100644
--- a/net/xfrm/xfrm_user.c
+++ b/net/xfrm/xfrm_user.c
@@ -796,7 +796,7 @@ static int copy_user_offload(struct xfrm_state_offload *xso, struct sk_buff *skb
 		return -EMSGSIZE;
 
 	xuo = nla_data(attr);
-
+	memset(xuo, 0, sizeof(*xuo));
 	xuo->ifindex = xso->dev->ifindex;
 	xuo->flags = xso->flags;
 
-- 
2.7.4

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH 5/7] xfrm_user: fix info leak in xfrm_notify_sa()
  2017-08-29 10:31 pull request (net): ipsec 2017-08-29 Steffen Klassert
                   ` (3 preceding siblings ...)
  2017-08-29 10:31 ` [PATCH 4/7] xfrm_user: fix info leak in copy_user_offload() Steffen Klassert
@ 2017-08-29 10:31 ` Steffen Klassert
  2017-08-29 10:31 ` [PATCH 6/7] xfrm_user: fix info leak in build_expire() Steffen Klassert
                   ` (2 subsequent siblings)
  7 siblings, 0 replies; 9+ messages in thread
From: Steffen Klassert @ 2017-08-29 10:31 UTC (permalink / raw)
  To: David Miller; +Cc: Herbert Xu, Steffen Klassert, netdev

From: Mathias Krause <minipli@googlemail.com>

The memory reserved to dump the ID of the xfrm state includes a padding
byte in struct xfrm_usersa_id added by the compiler for alignment. To
prevent the heap info leak, memset(0) the whole struct before filling
it.

Cc: Herbert Xu <herbert@gondor.apana.org.au>
Fixes: 0603eac0d6b7 ("[IPSEC]: Add XFRMA_SA/XFRMA_POLICY for delete notification")
Signed-off-by: Mathias Krause <minipli@googlemail.com>
Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>
---
 net/xfrm/xfrm_user.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/net/xfrm/xfrm_user.c b/net/xfrm/xfrm_user.c
index 3259555..c33516e 100644
--- a/net/xfrm/xfrm_user.c
+++ b/net/xfrm/xfrm_user.c
@@ -2715,6 +2715,7 @@ static int xfrm_notify_sa(struct xfrm_state *x, const struct km_event *c)
 		struct nlattr *attr;
 
 		id = nlmsg_data(nlh);
+		memset(id, 0, sizeof(*id));
 		memcpy(&id->daddr, &x->id.daddr, sizeof(id->daddr));
 		id->spi = x->id.spi;
 		id->family = x->props.family;
-- 
2.7.4

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH 6/7] xfrm_user: fix info leak in build_expire()
  2017-08-29 10:31 pull request (net): ipsec 2017-08-29 Steffen Klassert
                   ` (4 preceding siblings ...)
  2017-08-29 10:31 ` [PATCH 5/7] xfrm_user: fix info leak in xfrm_notify_sa() Steffen Klassert
@ 2017-08-29 10:31 ` Steffen Klassert
  2017-08-29 10:31 ` [PATCH 7/7] xfrm_user: fix info leak in build_aevent() Steffen Klassert
  2017-08-29 16:37 ` pull request (net): ipsec 2017-08-29 David Miller
  7 siblings, 0 replies; 9+ messages in thread
From: Steffen Klassert @ 2017-08-29 10:31 UTC (permalink / raw)
  To: David Miller; +Cc: Herbert Xu, Steffen Klassert, netdev

From: Mathias Krause <minipli@googlemail.com>

The memory reserved to dump the expired xfrm state includes padding
bytes in struct xfrm_user_expire added by the compiler for alignment. To
prevent the heap info leak, memset(0) the remainder of the struct.
Initializing the whole structure isn't needed as copy_to_user_state()
already takes care of clearing the padding bytes within the 'state'
member.

Signed-off-by: Mathias Krause <minipli@googlemail.com>
Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>
---
 net/xfrm/xfrm_user.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/net/xfrm/xfrm_user.c b/net/xfrm/xfrm_user.c
index c33516e..2cbdc81 100644
--- a/net/xfrm/xfrm_user.c
+++ b/net/xfrm/xfrm_user.c
@@ -2578,6 +2578,8 @@ static int build_expire(struct sk_buff *skb, struct xfrm_state *x, const struct
 	ue = nlmsg_data(nlh);
 	copy_to_user_state(x, &ue->state);
 	ue->hard = (c->data.hard != 0) ? 1 : 0;
+	/* clear the padding bytes */
+	memset(&ue->hard + 1, 0, sizeof(*ue) - offsetofend(typeof(*ue), hard));
 
 	err = xfrm_mark_put(skb, &x->mark);
 	if (err)
-- 
2.7.4

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH 7/7] xfrm_user: fix info leak in build_aevent()
  2017-08-29 10:31 pull request (net): ipsec 2017-08-29 Steffen Klassert
                   ` (5 preceding siblings ...)
  2017-08-29 10:31 ` [PATCH 6/7] xfrm_user: fix info leak in build_expire() Steffen Klassert
@ 2017-08-29 10:31 ` Steffen Klassert
  2017-08-29 16:37 ` pull request (net): ipsec 2017-08-29 David Miller
  7 siblings, 0 replies; 9+ messages in thread
From: Steffen Klassert @ 2017-08-29 10:31 UTC (permalink / raw)
  To: David Miller; +Cc: Herbert Xu, Steffen Klassert, netdev

From: Mathias Krause <minipli@googlemail.com>

The memory reserved to dump the ID of the xfrm state includes a padding
byte in struct xfrm_usersa_id added by the compiler for alignment. To
prevent the heap info leak, memset(0) the sa_id before filling it.

Cc: Jamal Hadi Salim <jhs@mojatatu.com>
Fixes: d51d081d6504 ("[IPSEC]: Sync series - user")
Signed-off-by: Mathias Krause <minipli@googlemail.com>
Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>
---
 net/xfrm/xfrm_user.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/net/xfrm/xfrm_user.c b/net/xfrm/xfrm_user.c
index 2cbdc81..9391ced 100644
--- a/net/xfrm/xfrm_user.c
+++ b/net/xfrm/xfrm_user.c
@@ -1869,6 +1869,7 @@ static int build_aevent(struct sk_buff *skb, struct xfrm_state *x, const struct
 		return -EMSGSIZE;
 
 	id = nlmsg_data(nlh);
+	memset(&id->sa_id, 0, sizeof(id->sa_id));
 	memcpy(&id->sa_id.daddr, &x->id.daddr, sizeof(x->id.daddr));
 	id->sa_id.spi = x->id.spi;
 	id->sa_id.family = x->props.family;
-- 
2.7.4

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: pull request (net): ipsec 2017-08-29
  2017-08-29 10:31 pull request (net): ipsec 2017-08-29 Steffen Klassert
                   ` (6 preceding siblings ...)
  2017-08-29 10:31 ` [PATCH 7/7] xfrm_user: fix info leak in build_aevent() Steffen Klassert
@ 2017-08-29 16:37 ` David Miller
  7 siblings, 0 replies; 9+ messages in thread
From: David Miller @ 2017-08-29 16:37 UTC (permalink / raw)
  To: steffen.klassert; +Cc: herbert, netdev

From: Steffen Klassert <steffen.klassert@secunet.com>
Date: Tue, 29 Aug 2017 12:31:27 +0200

> 1) Fix dst_entry refcount imbalance when using socket policies.
>    From Lorenzo Colitti.
> 
> 2) Fix locking when adding the ESP trailers.
> 
> 3) Fix tailroom calculation for the ESP trailer by using
>    skb_tailroom instead of skb_availroom.
> 
> 4) Fix some info leaks in xfrm_user.
>    From Mathias Krause.
> 
> Please pull or let me know if there are problems.

Pulled, thank you.

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2017-08-29 16:37 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-08-29 10:31 pull request (net): ipsec 2017-08-29 Steffen Klassert
2017-08-29 10:31 ` [PATCH 1/7] net: xfrm: don't double-hold dst when sk_policy in use Steffen Klassert
2017-08-29 10:31 ` [PATCH 2/7] esp: Fix locking on page fragment allocation Steffen Klassert
2017-08-29 10:31 ` [PATCH 3/7] esp: Fix skb tailroom calculation Steffen Klassert
2017-08-29 10:31 ` [PATCH 4/7] xfrm_user: fix info leak in copy_user_offload() Steffen Klassert
2017-08-29 10:31 ` [PATCH 5/7] xfrm_user: fix info leak in xfrm_notify_sa() Steffen Klassert
2017-08-29 10:31 ` [PATCH 6/7] xfrm_user: fix info leak in build_expire() Steffen Klassert
2017-08-29 10:31 ` [PATCH 7/7] xfrm_user: fix info leak in build_aevent() Steffen Klassert
2017-08-29 16:37 ` pull request (net): ipsec 2017-08-29 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.