All of lore.kernel.org
 help / color / mirror / Atom feed
* pull request (net): ipsec 2018-01-11
@ 2018-01-11 11:37 Steffen Klassert
  2018-01-11 11:37 ` [PATCH 01/11] xfrm: Forbid state updates from changing encap type Steffen Klassert
                   ` (11 more replies)
  0 siblings, 12 replies; 13+ messages in thread
From: Steffen Klassert @ 2018-01-11 11:37 UTC (permalink / raw)
  To: David Miller; +Cc: Herbert Xu, Steffen Klassert, netdev

1) Don't allow to change the encap type on state updates.
   The encap type is set on state initialization and
   should not change anymore. From Herbert Xu.

2) Skip dead policies when rehashing to fix a
   slab-out-of-bounds bug in xfrm_hash_rebuild.
   From Florian Westphal.

3) Two buffer overread fixes in pfkey.
   From Eric Biggers.

4) Fix rcu usage in xfrm_get_type_offload,
   request_module can sleep, so can't be used
   under rcu_read_lock. From Sabrina Dubroca.

5) Fix an uninitialized lock in xfrm_trans_queue.
   Use __skb_queue_tail instead of skb_queue_tail
   in xfrm_trans_queue as we don't need the lock.
   From Herbert Xu.

6) Currently it is possible to create an xfrm state with an
   unknown encap type in ESP IPv4. Fix this by returning an
   error on unknown encap types. Also from Herbert Xu.

7) Fix sleeping inside a spinlock in xfrm_policy_cache_flush.
   From Florian Westphal.

8) Fix ESP GRO when the headers not fully in the linear part
   of the skb. We need to pull before we can access them.

9) Fix a skb leak on error in key_notify_policy.

10) Fix a race in the xdst pcpu cache, we need to
    run the resolver routines with bottom halfes
    off like the old flowcache did.

Please pull or let me know if there are problems.

Thanks!

The following changes since commit 2758b3e3e630ba304fc4aca434d591e70e528298:

  Merge git://git.kernel.org/pub/scm/linux/kernel/git/davem/net (2017-12-28 23:20:21 -0800)

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 76a4201191814a0061cb5c861fafb9ecaa764846:

  xfrm: Fix a race in the xdst pcpu cache. (2018-01-10 12:14:28 +0100)

----------------------------------------------------------------
Eric Biggers (2):
      af_key: fix buffer overread in verify_address_len()
      af_key: fix buffer overread in parse_exthdrs()

Florian Westphal (2):
      xfrm: skip policies marked as dead while rehashing
      xfrm: don't call xfrm_policy_cache_flush while holding spinlock

Herbert Xu (3):
      xfrm: Forbid state updates from changing encap type
      xfrm: Use __skb_queue_tail in xfrm_trans_queue
      xfrm: Return error on unknown encap_type in init_state

Sabrina Dubroca (1):
      xfrm: fix rcu usage in xfrm_get_type_offload

Steffen Klassert (3):
      esp: Fix GRO when the headers not fully in the linear part of the skb.
      af_key: Fix memory leak in key_notify_policy.
      xfrm: Fix a race in the xdst pcpu cache.

 net/ipv4/esp4.c         |  1 +
 net/ipv4/esp4_offload.c |  3 ++-
 net/ipv6/esp6.c         |  3 +--
 net/ipv6/esp6_offload.c |  3 ++-
 net/key/af_key.c        | 12 +++++++++++-
 net/xfrm/xfrm_input.c   |  2 +-
 net/xfrm/xfrm_policy.c  | 15 +++++++++++----
 net/xfrm/xfrm_state.c   | 11 +++++++++--
 8 files changed, 38 insertions(+), 12 deletions(-)

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

* [PATCH 01/11] xfrm: Forbid state updates from changing encap type
  2018-01-11 11:37 pull request (net): ipsec 2018-01-11 Steffen Klassert
@ 2018-01-11 11:37 ` Steffen Klassert
  2018-01-11 11:37 ` [PATCH 02/11] xfrm: skip policies marked as dead while rehashing Steffen Klassert
                   ` (10 subsequent siblings)
  11 siblings, 0 replies; 13+ messages in thread
From: Steffen Klassert @ 2018-01-11 11:37 UTC (permalink / raw)
  To: David Miller; +Cc: Herbert Xu, Steffen Klassert, netdev

From: Herbert Xu <herbert@gondor.apana.org.au>

Currently we allow state updates to competely replace the contents
of x->encap.  This is bad because on the user side ESP only sets up
header lengths depending on encap_type once when the state is first
created.  This could result in the header lengths getting out of
sync with the actual state configuration.

In practice key managers will never do a state update to change the
encapsulation type.  Only the port numbers need to be changed as the
peer NAT entry is updated.

Therefore this patch adds a check in xfrm_state_update to forbid
any changes to the encap_type.

Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>
---
 net/xfrm/xfrm_state.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/net/xfrm/xfrm_state.c b/net/xfrm/xfrm_state.c
index 500b3391f474..1e80f68e2266 100644
--- a/net/xfrm/xfrm_state.c
+++ b/net/xfrm/xfrm_state.c
@@ -1534,8 +1534,12 @@ int xfrm_state_update(struct xfrm_state *x)
 	err = -EINVAL;
 	spin_lock_bh(&x1->lock);
 	if (likely(x1->km.state == XFRM_STATE_VALID)) {
-		if (x->encap && x1->encap)
+		if (x->encap && x1->encap &&
+		    x->encap->encap_type == x1->encap->encap_type)
 			memcpy(x1->encap, x->encap, sizeof(*x1->encap));
+		else if (x->encap || x1->encap)
+			goto fail;
+
 		if (x->coaddr && x1->coaddr) {
 			memcpy(x1->coaddr, x->coaddr, sizeof(*x1->coaddr));
 		}
@@ -1552,6 +1556,8 @@ int xfrm_state_update(struct xfrm_state *x)
 		x->km.state = XFRM_STATE_DEAD;
 		__xfrm_state_put(x);
 	}
+
+fail:
 	spin_unlock_bh(&x1->lock);
 
 	xfrm_state_put(x1);
-- 
2.14.1

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

* [PATCH 02/11] xfrm: skip policies marked as dead while rehashing
  2018-01-11 11:37 pull request (net): ipsec 2018-01-11 Steffen Klassert
  2018-01-11 11:37 ` [PATCH 01/11] xfrm: Forbid state updates from changing encap type Steffen Klassert
@ 2018-01-11 11:37 ` Steffen Klassert
  2018-01-11 11:37 ` [PATCH 03/11] af_key: fix buffer overread in verify_address_len() Steffen Klassert
                   ` (9 subsequent siblings)
  11 siblings, 0 replies; 13+ messages in thread
From: Steffen Klassert @ 2018-01-11 11:37 UTC (permalink / raw)
  To: David Miller; +Cc: Herbert Xu, Steffen Klassert, netdev

From: Florian Westphal <fw@strlen.de>

syzkaller triggered following KASAN splat:

BUG: KASAN: slab-out-of-bounds in xfrm_hash_rebuild+0xdbe/0xf00 net/xfrm/xfrm_policy.c:618
read of size 2 at addr ffff8801c8e92fe4 by task kworker/1:1/23 [..]
Workqueue: events xfrm_hash_rebuild [..]
 __asan_report_load2_noabort+0x14/0x20 mm/kasan/report.c:428
 xfrm_hash_rebuild+0xdbe/0xf00 net/xfrm/xfrm_policy.c:618
 process_one_work+0xbbf/0x1b10 kernel/workqueue.c:2112
 worker_thread+0x223/0x1990 kernel/workqueue.c:2246 [..]

The reproducer triggers:
1016                 if (error) {
1017                         list_move_tail(&walk->walk.all, &x->all);
1018                         goto out;
1019                 }

in xfrm_policy_walk() via pfkey (it sets tiny rcv space, dump
callback returns -ENOBUFS).

In this case, *walk is located the pfkey socket struct, so this socket
becomes visible in the global policy list.

It looks like this is intentional -- phony walker has walk.dead set to 1
and all other places skip such "policies".

Ccing original authors of the two commits that seem to expose this
issue (first patch missed ->dead check, second patch adds pfkey
sockets to policies dumper list).

Fixes: 880a6fab8f6ba5b ("xfrm: configure policy hash table thresholds by netlink")
Fixes: 12a169e7d8f4b1c ("ipsec: Put dumpers on the dump list")
Cc: Herbert Xu <herbert@gondor.apana.org.au>
Cc: Timo Teras <timo.teras@iki.fi>
Cc: Christophe Gouault <christophe.gouault@6wind.com>
Reported-by: syzbot <bot+c028095236fcb6f4348811565b75084c754dc729@syzkaller.appspotmail.com>
Signed-off-by: Florian Westphal <fw@strlen.de>
Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>
---
 net/xfrm/xfrm_policy.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/net/xfrm/xfrm_policy.c b/net/xfrm/xfrm_policy.c
index 70aa5cb0c659..2ef6db98e9ba 100644
--- a/net/xfrm/xfrm_policy.c
+++ b/net/xfrm/xfrm_policy.c
@@ -609,7 +609,8 @@ static void xfrm_hash_rebuild(struct work_struct *work)
 
 	/* re-insert all policies by order of creation */
 	list_for_each_entry_reverse(policy, &net->xfrm.policy_all, walk.all) {
-		if (xfrm_policy_id2dir(policy->index) >= XFRM_POLICY_MAX) {
+		if (policy->walk.dead ||
+		    xfrm_policy_id2dir(policy->index) >= XFRM_POLICY_MAX) {
 			/* skip socket policies */
 			continue;
 		}
-- 
2.14.1

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

* [PATCH 03/11] af_key: fix buffer overread in verify_address_len()
  2018-01-11 11:37 pull request (net): ipsec 2018-01-11 Steffen Klassert
  2018-01-11 11:37 ` [PATCH 01/11] xfrm: Forbid state updates from changing encap type Steffen Klassert
  2018-01-11 11:37 ` [PATCH 02/11] xfrm: skip policies marked as dead while rehashing Steffen Klassert
@ 2018-01-11 11:37 ` Steffen Klassert
  2018-01-11 11:37 ` [PATCH 04/11] af_key: fix buffer overread in parse_exthdrs() Steffen Klassert
                   ` (8 subsequent siblings)
  11 siblings, 0 replies; 13+ messages in thread
From: Steffen Klassert @ 2018-01-11 11:37 UTC (permalink / raw)
  To: David Miller; +Cc: Herbert Xu, Steffen Klassert, netdev

From: Eric Biggers <ebiggers@google.com>

If a message sent to a PF_KEY socket ended with one of the extensions
that takes a 'struct sadb_address' but there were not enough bytes
remaining in the message for the ->sa_family member of the 'struct
sockaddr' which is supposed to follow, then verify_address_len() read
past the end of the message, into uninitialized memory.  Fix it by
returning -EINVAL in this case.

This bug was found using syzkaller with KMSAN.

Reproducer:

	#include <linux/pfkeyv2.h>
	#include <sys/socket.h>
	#include <unistd.h>

	int main()
	{
		int sock = socket(PF_KEY, SOCK_RAW, PF_KEY_V2);
		char buf[24] = { 0 };
		struct sadb_msg *msg = (void *)buf;
		struct sadb_address *addr = (void *)(msg + 1);

		msg->sadb_msg_version = PF_KEY_V2;
		msg->sadb_msg_type = SADB_DELETE;
		msg->sadb_msg_len = 3;
		addr->sadb_address_len = 1;
		addr->sadb_address_exttype = SADB_EXT_ADDRESS_SRC;

		write(sock, buf, 24);
	}

Reported-by: Alexander Potapenko <glider@google.com>
Cc: stable@vger.kernel.org
Signed-off-by: Eric Biggers <ebiggers@google.com>
Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>
---
 net/key/af_key.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/net/key/af_key.c b/net/key/af_key.c
index 3dffb892d52c..596499cc8b2f 100644
--- a/net/key/af_key.c
+++ b/net/key/af_key.c
@@ -401,6 +401,11 @@ static int verify_address_len(const void *p)
 #endif
 	int len;
 
+	if (sp->sadb_address_len <
+	    DIV_ROUND_UP(sizeof(*sp) + offsetofend(typeof(*addr), sa_family),
+			 sizeof(uint64_t)))
+		return -EINVAL;
+
 	switch (addr->sa_family) {
 	case AF_INET:
 		len = DIV_ROUND_UP(sizeof(*sp) + sizeof(*sin), sizeof(uint64_t));
-- 
2.14.1

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

* [PATCH 04/11] af_key: fix buffer overread in parse_exthdrs()
  2018-01-11 11:37 pull request (net): ipsec 2018-01-11 Steffen Klassert
                   ` (2 preceding siblings ...)
  2018-01-11 11:37 ` [PATCH 03/11] af_key: fix buffer overread in verify_address_len() Steffen Klassert
@ 2018-01-11 11:37 ` Steffen Klassert
  2018-01-11 11:37 ` [PATCH 05/11] xfrm: fix rcu usage in xfrm_get_type_offload Steffen Klassert
                   ` (7 subsequent siblings)
  11 siblings, 0 replies; 13+ messages in thread
From: Steffen Klassert @ 2018-01-11 11:37 UTC (permalink / raw)
  To: David Miller; +Cc: Herbert Xu, Steffen Klassert, netdev

From: Eric Biggers <ebiggers@google.com>

If a message sent to a PF_KEY socket ended with an incomplete extension
header (fewer than 4 bytes remaining), then parse_exthdrs() read past
the end of the message, into uninitialized memory.  Fix it by returning
-EINVAL in this case.

Reproducer:

	#include <linux/pfkeyv2.h>
	#include <sys/socket.h>
	#include <unistd.h>

	int main()
	{
		int sock = socket(PF_KEY, SOCK_RAW, PF_KEY_V2);
		char buf[17] = { 0 };
		struct sadb_msg *msg = (void *)buf;

		msg->sadb_msg_version = PF_KEY_V2;
		msg->sadb_msg_type = SADB_DELETE;
		msg->sadb_msg_len = 2;

		write(sock, buf, 17);
	}

Cc: stable@vger.kernel.org
Signed-off-by: Eric Biggers <ebiggers@google.com>
Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>
---
 net/key/af_key.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/net/key/af_key.c b/net/key/af_key.c
index 596499cc8b2f..d40861a048fe 100644
--- a/net/key/af_key.c
+++ b/net/key/af_key.c
@@ -516,6 +516,9 @@ static int parse_exthdrs(struct sk_buff *skb, const struct sadb_msg *hdr, void *
 		uint16_t ext_type;
 		int ext_len;
 
+		if (len < sizeof(*ehdr))
+			return -EINVAL;
+
 		ext_len  = ehdr->sadb_ext_len;
 		ext_len *= sizeof(uint64_t);
 		ext_type = ehdr->sadb_ext_type;
-- 
2.14.1

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

* [PATCH 05/11] xfrm: fix rcu usage in xfrm_get_type_offload
  2018-01-11 11:37 pull request (net): ipsec 2018-01-11 Steffen Klassert
                   ` (3 preceding siblings ...)
  2018-01-11 11:37 ` [PATCH 04/11] af_key: fix buffer overread in parse_exthdrs() Steffen Klassert
@ 2018-01-11 11:37 ` Steffen Klassert
  2018-01-11 11:37 ` [PATCH 06/11] xfrm: Use __skb_queue_tail in xfrm_trans_queue Steffen Klassert
                   ` (6 subsequent siblings)
  11 siblings, 0 replies; 13+ messages in thread
From: Steffen Klassert @ 2018-01-11 11:37 UTC (permalink / raw)
  To: David Miller; +Cc: Herbert Xu, Steffen Klassert, netdev

From: Sabrina Dubroca <sd@queasysnail.net>

request_module can sleep, thus we cannot hold rcu_read_lock() while
calling it. The function also jumps back and takes rcu_read_lock()
again (in xfrm_state_get_afinfo()), resulting in an imbalance.

This codepath is triggered whenever a new offloaded state is created.

Fixes: ffdb5211da1c ("xfrm: Auto-load xfrm offload modules")
Reported-by: syzbot+ca425f44816d749e8eb49755567a75ee48cf4a30@syzkaller.appspotmail.com
Signed-off-by: Sabrina Dubroca <sd@queasysnail.net>
Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>
---
 net/xfrm/xfrm_state.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/net/xfrm/xfrm_state.c b/net/xfrm/xfrm_state.c
index 1e80f68e2266..429957412633 100644
--- a/net/xfrm/xfrm_state.c
+++ b/net/xfrm/xfrm_state.c
@@ -313,13 +313,14 @@ xfrm_get_type_offload(u8 proto, unsigned short family, bool try_load)
 	if ((type && !try_module_get(type->owner)))
 		type = NULL;
 
+	rcu_read_unlock();
+
 	if (!type && try_load) {
 		request_module("xfrm-offload-%d-%d", family, proto);
 		try_load = 0;
 		goto retry;
 	}
 
-	rcu_read_unlock();
 	return type;
 }
 
-- 
2.14.1

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

* [PATCH 06/11] xfrm: Use __skb_queue_tail in xfrm_trans_queue
  2018-01-11 11:37 pull request (net): ipsec 2018-01-11 Steffen Klassert
                   ` (4 preceding siblings ...)
  2018-01-11 11:37 ` [PATCH 05/11] xfrm: fix rcu usage in xfrm_get_type_offload Steffen Klassert
@ 2018-01-11 11:37 ` Steffen Klassert
  2018-01-11 11:37 ` [PATCH 07/11] xfrm: Return error on unknown encap_type in init_state Steffen Klassert
                   ` (5 subsequent siblings)
  11 siblings, 0 replies; 13+ messages in thread
From: Steffen Klassert @ 2018-01-11 11:37 UTC (permalink / raw)
  To: David Miller; +Cc: Herbert Xu, Steffen Klassert, netdev

From: Herbert Xu <herbert@gondor.apana.org.au>

We do not need locking in xfrm_trans_queue because it is designed
to use per-CPU buffers.  However, the original code incorrectly
used skb_queue_tail which takes the lock.  This patch switches
it to __skb_queue_tail instead.

Reported-and-tested-by: Artem Savkov <asavkov@redhat.com>
Fixes: acf568ee859f ("xfrm: Reinject transport-mode packets...")
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>
---
 net/xfrm/xfrm_input.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/xfrm/xfrm_input.c b/net/xfrm/xfrm_input.c
index 3f6f6f8c9fa5..5b2409746ae0 100644
--- a/net/xfrm/xfrm_input.c
+++ b/net/xfrm/xfrm_input.c
@@ -518,7 +518,7 @@ int xfrm_trans_queue(struct sk_buff *skb,
 		return -ENOBUFS;
 
 	XFRM_TRANS_SKB_CB(skb)->finish = finish;
-	skb_queue_tail(&trans->queue, skb);
+	__skb_queue_tail(&trans->queue, skb);
 	tasklet_schedule(&trans->tasklet);
 	return 0;
 }
-- 
2.14.1

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

* [PATCH 07/11] xfrm: Return error on unknown encap_type in init_state
  2018-01-11 11:37 pull request (net): ipsec 2018-01-11 Steffen Klassert
                   ` (5 preceding siblings ...)
  2018-01-11 11:37 ` [PATCH 06/11] xfrm: Use __skb_queue_tail in xfrm_trans_queue Steffen Klassert
@ 2018-01-11 11:37 ` Steffen Klassert
  2018-01-11 11:37 ` [PATCH 08/11] xfrm: don't call xfrm_policy_cache_flush while holding spinlock Steffen Klassert
                   ` (4 subsequent siblings)
  11 siblings, 0 replies; 13+ messages in thread
From: Steffen Klassert @ 2018-01-11 11:37 UTC (permalink / raw)
  To: David Miller; +Cc: Herbert Xu, Steffen Klassert, netdev

From: Herbert Xu <herbert@gondor.apana.org.au>

Currently esp will happily create an xfrm state with an unknown
encap type for IPv4, without setting the necessary state parameters.
This patch fixes it by returning -EINVAL.

There is a similar problem in IPv6 where if the mode is unknown
we will skip initialisation while returning zero.  However, this
is harmless as the mode has already been checked further up the
stack.  This patch removes this anomaly by aligning the IPv6
behaviour with IPv4 and treating unknown modes (which cannot
actually happen) as transport mode.

Fixes: 38320c70d282 ("[IPSEC]: Use crypto_aead and authenc in ESP")
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>
---
 net/ipv4/esp4.c | 1 +
 net/ipv6/esp6.c | 3 +--
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/net/ipv4/esp4.c b/net/ipv4/esp4.c
index d57aa64fa7c7..61fe6e4d23fc 100644
--- a/net/ipv4/esp4.c
+++ b/net/ipv4/esp4.c
@@ -981,6 +981,7 @@ static int esp_init_state(struct xfrm_state *x)
 
 		switch (encap->encap_type) {
 		default:
+			err = -EINVAL;
 			goto error;
 		case UDP_ENCAP_ESPINUDP:
 			x->props.header_len += sizeof(struct udphdr);
diff --git a/net/ipv6/esp6.c b/net/ipv6/esp6.c
index a902ff8f59be..1a7f00cd4803 100644
--- a/net/ipv6/esp6.c
+++ b/net/ipv6/esp6.c
@@ -890,13 +890,12 @@ static int esp6_init_state(struct xfrm_state *x)
 			x->props.header_len += IPV4_BEET_PHMAXLEN +
 					       (sizeof(struct ipv6hdr) - sizeof(struct iphdr));
 		break;
+	default:
 	case XFRM_MODE_TRANSPORT:
 		break;
 	case XFRM_MODE_TUNNEL:
 		x->props.header_len += sizeof(struct ipv6hdr);
 		break;
-	default:
-		goto error;
 	}
 
 	align = ALIGN(crypto_aead_blocksize(aead), 4);
-- 
2.14.1

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

* [PATCH 08/11] xfrm: don't call xfrm_policy_cache_flush while holding spinlock
  2018-01-11 11:37 pull request (net): ipsec 2018-01-11 Steffen Klassert
                   ` (6 preceding siblings ...)
  2018-01-11 11:37 ` [PATCH 07/11] xfrm: Return error on unknown encap_type in init_state Steffen Klassert
@ 2018-01-11 11:37 ` Steffen Klassert
  2018-01-11 11:37 ` [PATCH 09/11] esp: Fix GRO when the headers not fully in the linear part of the skb Steffen Klassert
                   ` (3 subsequent siblings)
  11 siblings, 0 replies; 13+ messages in thread
From: Steffen Klassert @ 2018-01-11 11:37 UTC (permalink / raw)
  To: David Miller; +Cc: Herbert Xu, Steffen Klassert, netdev

From: Florian Westphal <fw@strlen.de>

xfrm_policy_cache_flush can sleep, so it cannot be called while holding
a spinlock.  We could release the lock first, but I don't see why we need
to invoke this function here in first place, the packet path won't reuse
an xdst entry unless its still valid.

While at it, add an annotation to xfrm_policy_cache_flush, it would
have probably caught this bug sooner.

Fixes: ec30d78c14a813 ("xfrm: add xdst pcpu cache")
Reported-by: syzbot+e149f7d1328c26f9c12f@syzkaller.appspotmail.com
Signed-off-by: Florian Westphal <fw@strlen.de>
Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>
---
 net/xfrm/xfrm_policy.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/net/xfrm/xfrm_policy.c b/net/xfrm/xfrm_policy.c
index 2ef6db98e9ba..bc5eae12fb09 100644
--- a/net/xfrm/xfrm_policy.c
+++ b/net/xfrm/xfrm_policy.c
@@ -975,8 +975,6 @@ int xfrm_policy_flush(struct net *net, u8 type, bool task_valid)
 	}
 	if (!cnt)
 		err = -ESRCH;
-	else
-		xfrm_policy_cache_flush();
 out:
 	spin_unlock_bh(&net->xfrm.xfrm_policy_lock);
 	return err;
@@ -1744,6 +1742,8 @@ void xfrm_policy_cache_flush(void)
 	bool found = 0;
 	int cpu;
 
+	might_sleep();
+
 	local_bh_disable();
 	rcu_read_lock();
 	for_each_possible_cpu(cpu) {
-- 
2.14.1

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

* [PATCH 09/11] esp: Fix GRO when the headers not fully in the linear part of the skb.
  2018-01-11 11:37 pull request (net): ipsec 2018-01-11 Steffen Klassert
                   ` (7 preceding siblings ...)
  2018-01-11 11:37 ` [PATCH 08/11] xfrm: don't call xfrm_policy_cache_flush while holding spinlock Steffen Klassert
@ 2018-01-11 11:37 ` Steffen Klassert
  2018-01-11 11:37 ` [PATCH 10/11] af_key: Fix memory leak in key_notify_policy Steffen Klassert
                   ` (2 subsequent siblings)
  11 siblings, 0 replies; 13+ messages in thread
From: Steffen Klassert @ 2018-01-11 11:37 UTC (permalink / raw)
  To: David Miller; +Cc: Herbert Xu, Steffen Klassert, netdev

The GRO layer does not necessarily pull the complete headers
into the linear part of the skb, a part may remain on the
first page fragment. This can lead to a crash if we try to
pull the headers, so make sure we have them on the linear
part before pulling.

Fixes: 7785bba299a8 ("esp: Add a software GRO codepath")
Reported-by: syzbot+82bbd65569c49c6c0c4d@syzkaller.appspotmail.com
Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>
---
 net/ipv4/esp4_offload.c | 3 ++-
 net/ipv6/esp6_offload.c | 3 ++-
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/net/ipv4/esp4_offload.c b/net/ipv4/esp4_offload.c
index f8b918c766b0..b1338e576d00 100644
--- a/net/ipv4/esp4_offload.c
+++ b/net/ipv4/esp4_offload.c
@@ -38,7 +38,8 @@ static struct sk_buff **esp4_gro_receive(struct sk_buff **head,
 	__be32 spi;
 	int err;
 
-	skb_pull(skb, offset);
+	if (!pskb_pull(skb, offset))
+		return NULL;
 
 	if ((err = xfrm_parse_spi(skb, IPPROTO_ESP, &spi, &seq)) != 0)
 		goto out;
diff --git a/net/ipv6/esp6_offload.c b/net/ipv6/esp6_offload.c
index 333a478aa161..dd9627490c7c 100644
--- a/net/ipv6/esp6_offload.c
+++ b/net/ipv6/esp6_offload.c
@@ -60,7 +60,8 @@ static struct sk_buff **esp6_gro_receive(struct sk_buff **head,
 	int nhoff;
 	int err;
 
-	skb_pull(skb, offset);
+	if (!pskb_pull(skb, offset))
+		return NULL;
 
 	if ((err = xfrm_parse_spi(skb, IPPROTO_ESP, &spi, &seq)) != 0)
 		goto out;
-- 
2.14.1

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

* [PATCH 10/11] af_key: Fix memory leak in key_notify_policy.
  2018-01-11 11:37 pull request (net): ipsec 2018-01-11 Steffen Klassert
                   ` (8 preceding siblings ...)
  2018-01-11 11:37 ` [PATCH 09/11] esp: Fix GRO when the headers not fully in the linear part of the skb Steffen Klassert
@ 2018-01-11 11:37 ` Steffen Klassert
  2018-01-11 11:37 ` [PATCH 11/11] xfrm: Fix a race in the xdst pcpu cache Steffen Klassert
  2018-01-12 15:33 ` pull request (net): ipsec 2018-01-11 David Miller
  11 siblings, 0 replies; 13+ messages in thread
From: Steffen Klassert @ 2018-01-11 11:37 UTC (permalink / raw)
  To: David Miller; +Cc: Herbert Xu, Steffen Klassert, netdev

We leak the allocated out_skb in case
pfkey_xfrm_policy2msg() fails. Fix this
by freeing it on error.

Reported-by: Dmitry Vyukov <dvyukov@google.com>
Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>
---
 net/key/af_key.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/net/key/af_key.c b/net/key/af_key.c
index d40861a048fe..7e2e7188e7f4 100644
--- a/net/key/af_key.c
+++ b/net/key/af_key.c
@@ -2202,8 +2202,10 @@ static int key_notify_policy(struct xfrm_policy *xp, int dir, const struct km_ev
 		return PTR_ERR(out_skb);
 
 	err = pfkey_xfrm_policy2msg(out_skb, xp, dir);
-	if (err < 0)
+	if (err < 0) {
+		kfree_skb(out_skb);
 		return err;
+	}
 
 	out_hdr = (struct sadb_msg *) out_skb->data;
 	out_hdr->sadb_msg_version = PF_KEY_V2;
-- 
2.14.1

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

* [PATCH 11/11] xfrm: Fix a race in the xdst pcpu cache.
  2018-01-11 11:37 pull request (net): ipsec 2018-01-11 Steffen Klassert
                   ` (9 preceding siblings ...)
  2018-01-11 11:37 ` [PATCH 10/11] af_key: Fix memory leak in key_notify_policy Steffen Klassert
@ 2018-01-11 11:37 ` Steffen Klassert
  2018-01-12 15:33 ` pull request (net): ipsec 2018-01-11 David Miller
  11 siblings, 0 replies; 13+ messages in thread
From: Steffen Klassert @ 2018-01-11 11:37 UTC (permalink / raw)
  To: David Miller; +Cc: Herbert Xu, Steffen Klassert, netdev

We need to run xfrm_resolve_and_create_bundle() with
bottom halves off. Otherwise we may reuse an already
released dst_enty when the xfrm lookup functions are
called from process context.

Fixes: c30d78c14a813db39a647b6a348b428 ("xfrm: add xdst pcpu cache")
Reported-by: Darius Ski <darius.ski@gmail.com>
Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>
---
 net/xfrm/xfrm_policy.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/net/xfrm/xfrm_policy.c b/net/xfrm/xfrm_policy.c
index bc5eae12fb09..bd6b0e7a0ee4 100644
--- a/net/xfrm/xfrm_policy.c
+++ b/net/xfrm/xfrm_policy.c
@@ -2063,8 +2063,11 @@ xfrm_bundle_lookup(struct net *net, const struct flowi *fl, u16 family, u8 dir,
 	if (num_xfrms <= 0)
 		goto make_dummy_bundle;
 
+	local_bh_disable();
 	xdst = xfrm_resolve_and_create_bundle(pols, num_pols, fl, family,
-						  xflo->dst_orig);
+					      xflo->dst_orig);
+	local_bh_enable();
+
 	if (IS_ERR(xdst)) {
 		err = PTR_ERR(xdst);
 		if (err != -EAGAIN)
@@ -2151,9 +2154,12 @@ struct dst_entry *xfrm_lookup(struct net *net, struct dst_entry *dst_orig,
 				goto no_transform;
 			}
 
+			local_bh_disable();
 			xdst = xfrm_resolve_and_create_bundle(
 					pols, num_pols, fl,
 					family, dst_orig);
+			local_bh_enable();
+
 			if (IS_ERR(xdst)) {
 				xfrm_pols_put(pols, num_pols);
 				err = PTR_ERR(xdst);
-- 
2.14.1

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

* Re: pull request (net): ipsec 2018-01-11
  2018-01-11 11:37 pull request (net): ipsec 2018-01-11 Steffen Klassert
                   ` (10 preceding siblings ...)
  2018-01-11 11:37 ` [PATCH 11/11] xfrm: Fix a race in the xdst pcpu cache Steffen Klassert
@ 2018-01-12 15:33 ` David Miller
  11 siblings, 0 replies; 13+ messages in thread
From: David Miller @ 2018-01-12 15:33 UTC (permalink / raw)
  To: steffen.klassert; +Cc: herbert, netdev

From: Steffen Klassert <steffen.klassert@secunet.com>
Date: Thu, 11 Jan 2018 12:37:35 +0100

> 1) Don't allow to change the encap type on state updates.
>    The encap type is set on state initialization and
>    should not change anymore. From Herbert Xu.
> 
> 2) Skip dead policies when rehashing to fix a
>    slab-out-of-bounds bug in xfrm_hash_rebuild.
>    From Florian Westphal.
> 
> 3) Two buffer overread fixes in pfkey.
>    From Eric Biggers.
> 
> 4) Fix rcu usage in xfrm_get_type_offload,
>    request_module can sleep, so can't be used
>    under rcu_read_lock. From Sabrina Dubroca.
> 
> 5) Fix an uninitialized lock in xfrm_trans_queue.
>    Use __skb_queue_tail instead of skb_queue_tail
>    in xfrm_trans_queue as we don't need the lock.
>    From Herbert Xu.
> 
> 6) Currently it is possible to create an xfrm state with an
>    unknown encap type in ESP IPv4. Fix this by returning an
>    error on unknown encap types. Also from Herbert Xu.
> 
> 7) Fix sleeping inside a spinlock in xfrm_policy_cache_flush.
>    From Florian Westphal.
> 
> 8) Fix ESP GRO when the headers not fully in the linear part
>    of the skb. We need to pull before we can access them.
> 
> 9) Fix a skb leak on error in key_notify_policy.
> 
> 10) Fix a race in the xdst pcpu cache, we need to
>     run the resolver routines with bottom halfes
>     off like the old flowcache did.
> 
> Please pull or let me know if there are problems.

Pulled, thanks Steffen.

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

end of thread, other threads:[~2018-01-12 15:33 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-01-11 11:37 pull request (net): ipsec 2018-01-11 Steffen Klassert
2018-01-11 11:37 ` [PATCH 01/11] xfrm: Forbid state updates from changing encap type Steffen Klassert
2018-01-11 11:37 ` [PATCH 02/11] xfrm: skip policies marked as dead while rehashing Steffen Klassert
2018-01-11 11:37 ` [PATCH 03/11] af_key: fix buffer overread in verify_address_len() Steffen Klassert
2018-01-11 11:37 ` [PATCH 04/11] af_key: fix buffer overread in parse_exthdrs() Steffen Klassert
2018-01-11 11:37 ` [PATCH 05/11] xfrm: fix rcu usage in xfrm_get_type_offload Steffen Klassert
2018-01-11 11:37 ` [PATCH 06/11] xfrm: Use __skb_queue_tail in xfrm_trans_queue Steffen Klassert
2018-01-11 11:37 ` [PATCH 07/11] xfrm: Return error on unknown encap_type in init_state Steffen Klassert
2018-01-11 11:37 ` [PATCH 08/11] xfrm: don't call xfrm_policy_cache_flush while holding spinlock Steffen Klassert
2018-01-11 11:37 ` [PATCH 09/11] esp: Fix GRO when the headers not fully in the linear part of the skb Steffen Klassert
2018-01-11 11:37 ` [PATCH 10/11] af_key: Fix memory leak in key_notify_policy Steffen Klassert
2018-01-11 11:37 ` [PATCH 11/11] xfrm: Fix a race in the xdst pcpu cache Steffen Klassert
2018-01-12 15:33 ` pull request (net): ipsec 2018-01-11 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.