netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* pull request (net): ipsec 2020-07-31
@ 2020-07-31  7:17 Steffen Klassert
  2020-07-31  7:17 ` [PATCH 01/10] xfrm: policy: match with both mark and mask on user interfaces Steffen Klassert
                   ` (10 more replies)
  0 siblings, 11 replies; 12+ messages in thread
From: Steffen Klassert @ 2020-07-31  7:17 UTC (permalink / raw)
  To: David Miller; +Cc: Herbert Xu, Steffen Klassert, netdev

1) Fix policy matching with mark and mask on userspace interfaces.
   From Xin Long.

2) Several fixes for the new ESP in TCP encapsulation.
   From Sabrina Dubroca.

3) Fix crash when the hold queue is used. The assumption that
   xdst->path and dst->child are not a NULL pointer only if dst->xfrm
   is not a NULL pointer is true with the exception of using the
   hold queue. Fix this by checking for hold queue usage before
   dereferencing xdst->path or dst->child.

4) Validate pfkey_dump parameter before sending them.
   From Mark Salyzyn.

5) Fix the location of the transport header with ESP in UDPv6
   encapsulation. From Sabrina Dubroca.

Please pull or let me know if there are problems.

Thanks!

The following changes since commit 0275875530f692c725c6f993aced2eca2d6ac50c:

  Merge branch 'Two-phylink-pause-fixes' (2020-06-23 20:53:28 -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 71b59bf482b2dd662774f34108c5b904efa9e02b:

  espintcp: count packets dropped in espintcp_rcv (2020-07-30 06:51:36 +0200)

----------------------------------------------------------------
Mark Salyzyn (1):
      af_key: pfkey_dump needs parameter validation

Sabrina Dubroca (7):
      xfrm: esp6: fix encapsulation header offset computation
      espintcp: support non-blocking sends
      espintcp: recv() should return 0 when the peer socket is closed
      xfrm: policy: fix IPv6-only espintcp compilation
      xfrm: esp6: fix the location of the transport header with encapsulation
      espintcp: handle short messages instead of breaking the encap socket
      espintcp: count packets dropped in espintcp_rcv

Steffen Klassert (2):
      Merge remote-tracking branch 'origin/testing'
      xfrm: Fix crash when the hold queue is used.

Xin Long (1):
      xfrm: policy: match with both mark and mask on user interfaces

 include/net/xfrm.h     | 15 +++++++-----
 net/ipv6/esp6.c        | 13 ++++++++---
 net/key/af_key.c       | 11 +++++++--
 net/xfrm/espintcp.c    | 62 ++++++++++++++++++++++++++++++++++++++------------
 net/xfrm/xfrm_policy.c | 43 +++++++++++++++-------------------
 net/xfrm/xfrm_user.c   | 18 +++++++++------
 6 files changed, 104 insertions(+), 58 deletions(-)

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

* [PATCH 01/10] xfrm: policy: match with both mark and mask on user interfaces
  2020-07-31  7:17 pull request (net): ipsec 2020-07-31 Steffen Klassert
@ 2020-07-31  7:17 ` Steffen Klassert
  2020-07-31  7:17 ` [PATCH 02/10] xfrm: esp6: fix encapsulation header offset computation Steffen Klassert
                   ` (9 subsequent siblings)
  10 siblings, 0 replies; 12+ messages in thread
From: Steffen Klassert @ 2020-07-31  7:17 UTC (permalink / raw)
  To: David Miller; +Cc: Herbert Xu, Steffen Klassert, netdev

From: Xin Long <lucien.xin@gmail.com>

In commit ed17b8d377ea ("xfrm: fix a warning in xfrm_policy_insert_list"),
it would take 'priority' to make a policy unique, and allow duplicated
policies with different 'priority' to be added, which is not expected
by userland, as Tobias reported in strongswan.

To fix this duplicated policies issue, and also fix the issue in
commit ed17b8d377ea ("xfrm: fix a warning in xfrm_policy_insert_list"),
when doing add/del/get/update on user interfaces, this patch is to change
to look up a policy with both mark and mask by doing:

  mark.v == pol->mark.v && mark.m == pol->mark.m

and leave the check:

  (mark & pol->mark.m) == pol->mark.v

for tx/rx path only.

As the userland expects an exact mark and mask match to manage policies.

v1->v2:
  - make xfrm_policy_mark_match inline and fix the changelog as
    Tobias suggested.

Fixes: 295fae568885 ("xfrm: Allow user space manipulation of SPD mark")
Fixes: ed17b8d377ea ("xfrm: fix a warning in xfrm_policy_insert_list")
Reported-by: Tobias Brunner <tobias@strongswan.org>
Tested-by: Tobias Brunner <tobias@strongswan.org>
Signed-off-by: Xin Long <lucien.xin@gmail.com>
Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>
---
 include/net/xfrm.h     | 11 +++++++----
 net/key/af_key.c       |  4 ++--
 net/xfrm/xfrm_policy.c | 39 ++++++++++++++++-----------------------
 net/xfrm/xfrm_user.c   | 18 +++++++++++-------
 4 files changed, 36 insertions(+), 36 deletions(-)

diff --git a/include/net/xfrm.h b/include/net/xfrm.h
index c7d213c9f9d8..5c20953c8deb 100644
--- a/include/net/xfrm.h
+++ b/include/net/xfrm.h
@@ -1630,13 +1630,16 @@ int xfrm_policy_walk(struct net *net, struct xfrm_policy_walk *walk,
 		     void *);
 void xfrm_policy_walk_done(struct xfrm_policy_walk *walk, struct net *net);
 int xfrm_policy_insert(int dir, struct xfrm_policy *policy, int excl);
-struct xfrm_policy *xfrm_policy_bysel_ctx(struct net *net, u32 mark, u32 if_id,
-					  u8 type, int dir,
+struct xfrm_policy *xfrm_policy_bysel_ctx(struct net *net,
+					  const struct xfrm_mark *mark,
+					  u32 if_id, u8 type, int dir,
 					  struct xfrm_selector *sel,
 					  struct xfrm_sec_ctx *ctx, int delete,
 					  int *err);
-struct xfrm_policy *xfrm_policy_byid(struct net *net, u32 mark, u32 if_id, u8,
-				     int dir, u32 id, int delete, int *err);
+struct xfrm_policy *xfrm_policy_byid(struct net *net,
+				     const struct xfrm_mark *mark, u32 if_id,
+				     u8 type, int dir, u32 id, int delete,
+				     int *err);
 int xfrm_policy_flush(struct net *net, u8 type, bool task_valid);
 void xfrm_policy_hash_rebuild(struct net *net);
 u32 xfrm_get_acqseq(void);
diff --git a/net/key/af_key.c b/net/key/af_key.c
index b67ed3a8486c..979c579afc63 100644
--- a/net/key/af_key.c
+++ b/net/key/af_key.c
@@ -2400,7 +2400,7 @@ static int pfkey_spddelete(struct sock *sk, struct sk_buff *skb, const struct sa
 			return err;
 	}
 
-	xp = xfrm_policy_bysel_ctx(net, DUMMY_MARK, 0, XFRM_POLICY_TYPE_MAIN,
+	xp = xfrm_policy_bysel_ctx(net, &dummy_mark, 0, XFRM_POLICY_TYPE_MAIN,
 				   pol->sadb_x_policy_dir - 1, &sel, pol_ctx,
 				   1, &err);
 	security_xfrm_policy_free(pol_ctx);
@@ -2651,7 +2651,7 @@ static int pfkey_spdget(struct sock *sk, struct sk_buff *skb, const struct sadb_
 		return -EINVAL;
 
 	delete = (hdr->sadb_msg_type == SADB_X_SPDDELETE2);
-	xp = xfrm_policy_byid(net, DUMMY_MARK, 0, XFRM_POLICY_TYPE_MAIN,
+	xp = xfrm_policy_byid(net, &dummy_mark, 0, XFRM_POLICY_TYPE_MAIN,
 			      dir, pol->sadb_x_policy_id, delete, &err);
 	if (xp == NULL)
 		return -ENOENT;
diff --git a/net/xfrm/xfrm_policy.c b/net/xfrm/xfrm_policy.c
index 564aa6492e7c..6847b3579f54 100644
--- a/net/xfrm/xfrm_policy.c
+++ b/net/xfrm/xfrm_policy.c
@@ -1433,14 +1433,10 @@ static void xfrm_policy_requeue(struct xfrm_policy *old,
 	spin_unlock_bh(&pq->hold_queue.lock);
 }
 
-static bool xfrm_policy_mark_match(struct xfrm_policy *policy,
-				   struct xfrm_policy *pol)
+static inline bool xfrm_policy_mark_match(const struct xfrm_mark *mark,
+					  struct xfrm_policy *pol)
 {
-	if (policy->mark.v == pol->mark.v &&
-	    policy->priority == pol->priority)
-		return true;
-
-	return false;
+	return mark->v == pol->mark.v && mark->m == pol->mark.m;
 }
 
 static u32 xfrm_pol_bin_key(const void *data, u32 len, u32 seed)
@@ -1503,7 +1499,7 @@ static void xfrm_policy_insert_inexact_list(struct hlist_head *chain,
 		if (pol->type == policy->type &&
 		    pol->if_id == policy->if_id &&
 		    !selector_cmp(&pol->selector, &policy->selector) &&
-		    xfrm_policy_mark_match(policy, pol) &&
+		    xfrm_policy_mark_match(&policy->mark, pol) &&
 		    xfrm_sec_ctx_match(pol->security, policy->security) &&
 		    !WARN_ON(delpol)) {
 			delpol = pol;
@@ -1538,7 +1534,7 @@ static struct xfrm_policy *xfrm_policy_insert_list(struct hlist_head *chain,
 		if (pol->type == policy->type &&
 		    pol->if_id == policy->if_id &&
 		    !selector_cmp(&pol->selector, &policy->selector) &&
-		    xfrm_policy_mark_match(policy, pol) &&
+		    xfrm_policy_mark_match(&policy->mark, pol) &&
 		    xfrm_sec_ctx_match(pol->security, policy->security) &&
 		    !WARN_ON(delpol)) {
 			if (excl)
@@ -1610,9 +1606,8 @@ int xfrm_policy_insert(int dir, struct xfrm_policy *policy, int excl)
 EXPORT_SYMBOL(xfrm_policy_insert);
 
 static struct xfrm_policy *
-__xfrm_policy_bysel_ctx(struct hlist_head *chain, u32 mark, u32 if_id,
-			u8 type, int dir,
-			struct xfrm_selector *sel,
+__xfrm_policy_bysel_ctx(struct hlist_head *chain, const struct xfrm_mark *mark,
+			u32 if_id, u8 type, int dir, struct xfrm_selector *sel,
 			struct xfrm_sec_ctx *ctx)
 {
 	struct xfrm_policy *pol;
@@ -1623,7 +1618,7 @@ __xfrm_policy_bysel_ctx(struct hlist_head *chain, u32 mark, u32 if_id,
 	hlist_for_each_entry(pol, chain, bydst) {
 		if (pol->type == type &&
 		    pol->if_id == if_id &&
-		    (mark & pol->mark.m) == pol->mark.v &&
+		    xfrm_policy_mark_match(mark, pol) &&
 		    !selector_cmp(sel, &pol->selector) &&
 		    xfrm_sec_ctx_match(ctx, pol->security))
 			return pol;
@@ -1632,11 +1627,10 @@ __xfrm_policy_bysel_ctx(struct hlist_head *chain, u32 mark, u32 if_id,
 	return NULL;
 }
 
-struct xfrm_policy *xfrm_policy_bysel_ctx(struct net *net, u32 mark, u32 if_id,
-					  u8 type, int dir,
-					  struct xfrm_selector *sel,
-					  struct xfrm_sec_ctx *ctx, int delete,
-					  int *err)
+struct xfrm_policy *
+xfrm_policy_bysel_ctx(struct net *net, const struct xfrm_mark *mark, u32 if_id,
+		      u8 type, int dir, struct xfrm_selector *sel,
+		      struct xfrm_sec_ctx *ctx, int delete, int *err)
 {
 	struct xfrm_pol_inexact_bin *bin = NULL;
 	struct xfrm_policy *pol, *ret = NULL;
@@ -1703,9 +1697,9 @@ struct xfrm_policy *xfrm_policy_bysel_ctx(struct net *net, u32 mark, u32 if_id,
 }
 EXPORT_SYMBOL(xfrm_policy_bysel_ctx);
 
-struct xfrm_policy *xfrm_policy_byid(struct net *net, u32 mark, u32 if_id,
-				     u8 type, int dir, u32 id, int delete,
-				     int *err)
+struct xfrm_policy *
+xfrm_policy_byid(struct net *net, const struct xfrm_mark *mark, u32 if_id,
+		 u8 type, int dir, u32 id, int delete, int *err)
 {
 	struct xfrm_policy *pol, *ret;
 	struct hlist_head *chain;
@@ -1720,8 +1714,7 @@ struct xfrm_policy *xfrm_policy_byid(struct net *net, u32 mark, u32 if_id,
 	ret = NULL;
 	hlist_for_each_entry(pol, chain, byidx) {
 		if (pol->type == type && pol->index == id &&
-		    pol->if_id == if_id &&
-		    (mark & pol->mark.m) == pol->mark.v) {
+		    pol->if_id == if_id && xfrm_policy_mark_match(mark, pol)) {
 			xfrm_pol_hold(pol);
 			if (delete) {
 				*err = security_xfrm_policy_delete(
diff --git a/net/xfrm/xfrm_user.c b/net/xfrm/xfrm_user.c
index e6cfaa680ef3..fbb7d9d06478 100644
--- a/net/xfrm/xfrm_user.c
+++ b/net/xfrm/xfrm_user.c
@@ -1863,7 +1863,6 @@ static int xfrm_get_policy(struct sk_buff *skb, struct nlmsghdr *nlh,
 	struct km_event c;
 	int delete;
 	struct xfrm_mark m;
-	u32 mark = xfrm_mark_get(attrs, &m);
 	u32 if_id = 0;
 
 	p = nlmsg_data(nlh);
@@ -1880,8 +1879,11 @@ static int xfrm_get_policy(struct sk_buff *skb, struct nlmsghdr *nlh,
 	if (attrs[XFRMA_IF_ID])
 		if_id = nla_get_u32(attrs[XFRMA_IF_ID]);
 
+	xfrm_mark_get(attrs, &m);
+
 	if (p->index)
-		xp = xfrm_policy_byid(net, mark, if_id, type, p->dir, p->index, delete, &err);
+		xp = xfrm_policy_byid(net, &m, if_id, type, p->dir,
+				      p->index, delete, &err);
 	else {
 		struct nlattr *rt = attrs[XFRMA_SEC_CTX];
 		struct xfrm_sec_ctx *ctx;
@@ -1898,8 +1900,8 @@ static int xfrm_get_policy(struct sk_buff *skb, struct nlmsghdr *nlh,
 			if (err)
 				return err;
 		}
-		xp = xfrm_policy_bysel_ctx(net, mark, if_id, type, p->dir, &p->sel,
-					   ctx, delete, &err);
+		xp = xfrm_policy_bysel_ctx(net, &m, if_id, type, p->dir,
+					   &p->sel, ctx, delete, &err);
 		security_xfrm_policy_free(ctx);
 	}
 	if (xp == NULL)
@@ -2166,7 +2168,6 @@ static int xfrm_add_pol_expire(struct sk_buff *skb, struct nlmsghdr *nlh,
 	u8 type = XFRM_POLICY_TYPE_MAIN;
 	int err = -ENOENT;
 	struct xfrm_mark m;
-	u32 mark = xfrm_mark_get(attrs, &m);
 	u32 if_id = 0;
 
 	err = copy_from_user_policy_type(&type, attrs);
@@ -2180,8 +2181,11 @@ static int xfrm_add_pol_expire(struct sk_buff *skb, struct nlmsghdr *nlh,
 	if (attrs[XFRMA_IF_ID])
 		if_id = nla_get_u32(attrs[XFRMA_IF_ID]);
 
+	xfrm_mark_get(attrs, &m);
+
 	if (p->index)
-		xp = xfrm_policy_byid(net, mark, if_id, type, p->dir, p->index, 0, &err);
+		xp = xfrm_policy_byid(net, &m, if_id, type, p->dir, p->index,
+				      0, &err);
 	else {
 		struct nlattr *rt = attrs[XFRMA_SEC_CTX];
 		struct xfrm_sec_ctx *ctx;
@@ -2198,7 +2202,7 @@ static int xfrm_add_pol_expire(struct sk_buff *skb, struct nlmsghdr *nlh,
 			if (err)
 				return err;
 		}
-		xp = xfrm_policy_bysel_ctx(net, mark, if_id, type, p->dir,
+		xp = xfrm_policy_bysel_ctx(net, &m, if_id, type, p->dir,
 					   &p->sel, ctx, 0, &err);
 		security_xfrm_policy_free(ctx);
 	}
-- 
2.17.1


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

* [PATCH 02/10] xfrm: esp6: fix encapsulation header offset computation
  2020-07-31  7:17 pull request (net): ipsec 2020-07-31 Steffen Klassert
  2020-07-31  7:17 ` [PATCH 01/10] xfrm: policy: match with both mark and mask on user interfaces Steffen Klassert
@ 2020-07-31  7:17 ` Steffen Klassert
  2020-07-31  7:17 ` [PATCH 03/10] espintcp: support non-blocking sends Steffen Klassert
                   ` (8 subsequent siblings)
  10 siblings, 0 replies; 12+ messages in thread
From: Steffen Klassert @ 2020-07-31  7:17 UTC (permalink / raw)
  To: David Miller; +Cc: Herbert Xu, Steffen Klassert, netdev

From: Sabrina Dubroca <sd@queasysnail.net>

In commit 0146dca70b87, I incorrectly adapted the code that computes
the location of the UDP or TCP encapsulation header from IPv4 to
IPv6. In esp6_input_done2, skb->transport_header points to the ESP
header, so by adding skb_network_header_len, uh and th will point to
the ESP header, not the encapsulation header that's in front of it.

Since the TCP header's size can change with options, we have to start
from the IPv6 header and walk past possible extensions.

Fixes: 0146dca70b87 ("xfrm: add support for UDPv6 encapsulation of ESP")
Fixes: 26333c37fc28 ("xfrm: add IPv6 support for espintcp")
Reported-by: Tobias Brunner <tobias@strongswan.org>
Tested-by: Tobias Brunner <tobias@strongswan.org>
Signed-off-by: Sabrina Dubroca <sd@queasysnail.net>
Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>
---
 net/ipv6/esp6.c | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/net/ipv6/esp6.c b/net/ipv6/esp6.c
index c43592771126..55ae70be91b3 100644
--- a/net/ipv6/esp6.c
+++ b/net/ipv6/esp6.c
@@ -805,10 +805,16 @@ int esp6_input_done2(struct sk_buff *skb, int err)
 
 	if (x->encap) {
 		const struct ipv6hdr *ip6h = ipv6_hdr(skb);
+		int offset = skb_network_offset(skb) + sizeof(*ip6h);
 		struct xfrm_encap_tmpl *encap = x->encap;
-		struct udphdr *uh = (void *)(skb_network_header(skb) + hdr_len);
-		struct tcphdr *th = (void *)(skb_network_header(skb) + hdr_len);
-		__be16 source;
+		u8 nexthdr = ip6h->nexthdr;
+		__be16 frag_off, source;
+		struct udphdr *uh;
+		struct tcphdr *th;
+
+		offset = ipv6_skip_exthdr(skb, offset, &nexthdr, &frag_off);
+		uh = (void *)(skb->data + offset);
+		th = (void *)(skb->data + offset);
 
 		switch (x->encap->encap_type) {
 		case TCP_ENCAP_ESPINTCP:
-- 
2.17.1


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

* [PATCH 03/10] espintcp: support non-blocking sends
  2020-07-31  7:17 pull request (net): ipsec 2020-07-31 Steffen Klassert
  2020-07-31  7:17 ` [PATCH 01/10] xfrm: policy: match with both mark and mask on user interfaces Steffen Klassert
  2020-07-31  7:17 ` [PATCH 02/10] xfrm: esp6: fix encapsulation header offset computation Steffen Klassert
@ 2020-07-31  7:17 ` Steffen Klassert
  2020-07-31  7:17 ` [PATCH 04/10] espintcp: recv() should return 0 when the peer socket is closed Steffen Klassert
                   ` (7 subsequent siblings)
  10 siblings, 0 replies; 12+ messages in thread
From: Steffen Klassert @ 2020-07-31  7:17 UTC (permalink / raw)
  To: David Miller; +Cc: Herbert Xu, Steffen Klassert, netdev

From: Sabrina Dubroca <sd@queasysnail.net>

Currently, non-blocking sends from userspace result in EOPNOTSUPP.

To support this, we need to tell espintcp_sendskb_locked() and
espintcp_sendskmsg_locked() that non-blocking operation was requested
from espintcp_sendmsg().

Fixes: e27cca96cd68 ("xfrm: add espintcp (RFC 8229)")
Reported-by: Andrew Cagney <cagney@libreswan.org>
Tested-by: Andrew Cagney <cagney@libreswan.org>
Signed-off-by: Sabrina Dubroca <sd@queasysnail.net>
Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>
---
 net/xfrm/espintcp.c | 26 +++++++++++++-------------
 1 file changed, 13 insertions(+), 13 deletions(-)

diff --git a/net/xfrm/espintcp.c b/net/xfrm/espintcp.c
index 100e29682b48..5d3d2b98c62d 100644
--- a/net/xfrm/espintcp.c
+++ b/net/xfrm/espintcp.c
@@ -213,7 +213,7 @@ static int espintcp_sendskmsg_locked(struct sock *sk,
 	return 0;
 }
 
-static int espintcp_push_msgs(struct sock *sk)
+static int espintcp_push_msgs(struct sock *sk, int flags)
 {
 	struct espintcp_ctx *ctx = espintcp_getctx(sk);
 	struct espintcp_msg *emsg = &ctx->partial;
@@ -227,12 +227,12 @@ static int espintcp_push_msgs(struct sock *sk)
 	ctx->tx_running = 1;
 
 	if (emsg->skb)
-		err = espintcp_sendskb_locked(sk, emsg, 0);
+		err = espintcp_sendskb_locked(sk, emsg, flags);
 	else
-		err = espintcp_sendskmsg_locked(sk, emsg, 0);
+		err = espintcp_sendskmsg_locked(sk, emsg, flags);
 	if (err == -EAGAIN) {
 		ctx->tx_running = 0;
-		return 0;
+		return flags & MSG_DONTWAIT ? -EAGAIN : 0;
 	}
 	if (!err)
 		memset(emsg, 0, sizeof(*emsg));
@@ -257,7 +257,7 @@ int espintcp_push_skb(struct sock *sk, struct sk_buff *skb)
 	offset = skb_transport_offset(skb);
 	len = skb->len - offset;
 
-	espintcp_push_msgs(sk);
+	espintcp_push_msgs(sk, 0);
 
 	if (emsg->len) {
 		kfree_skb(skb);
@@ -270,7 +270,7 @@ int espintcp_push_skb(struct sock *sk, struct sk_buff *skb)
 	emsg->len = len;
 	emsg->skb = skb;
 
-	espintcp_push_msgs(sk);
+	espintcp_push_msgs(sk, 0);
 
 	return 0;
 }
@@ -287,7 +287,7 @@ static int espintcp_sendmsg(struct sock *sk, struct msghdr *msg, size_t size)
 	char buf[2] = {0};
 	int err, end;
 
-	if (msg->msg_flags)
+	if (msg->msg_flags & ~MSG_DONTWAIT)
 		return -EOPNOTSUPP;
 
 	if (size > MAX_ESPINTCP_MSG)
@@ -298,9 +298,10 @@ static int espintcp_sendmsg(struct sock *sk, struct msghdr *msg, size_t size)
 
 	lock_sock(sk);
 
-	err = espintcp_push_msgs(sk);
+	err = espintcp_push_msgs(sk, msg->msg_flags & MSG_DONTWAIT);
 	if (err < 0) {
-		err = -ENOBUFS;
+		if (err != -EAGAIN || !(msg->msg_flags & MSG_DONTWAIT))
+			err = -ENOBUFS;
 		goto unlock;
 	}
 
@@ -337,10 +338,9 @@ static int espintcp_sendmsg(struct sock *sk, struct msghdr *msg, size_t size)
 
 	tcp_rate_check_app_limited(sk);
 
-	err = espintcp_push_msgs(sk);
+	err = espintcp_push_msgs(sk, msg->msg_flags & MSG_DONTWAIT);
 	/* this message could be partially sent, keep it */
-	if (err < 0)
-		goto unlock;
+
 	release_sock(sk);
 
 	return size;
@@ -374,7 +374,7 @@ static void espintcp_tx_work(struct work_struct *work)
 
 	lock_sock(sk);
 	if (!ctx->tx_running)
-		espintcp_push_msgs(sk);
+		espintcp_push_msgs(sk, 0);
 	release_sock(sk);
 }
 
-- 
2.17.1


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

* [PATCH 04/10] espintcp: recv() should return 0 when the peer socket is closed
  2020-07-31  7:17 pull request (net): ipsec 2020-07-31 Steffen Klassert
                   ` (2 preceding siblings ...)
  2020-07-31  7:17 ` [PATCH 03/10] espintcp: support non-blocking sends Steffen Klassert
@ 2020-07-31  7:17 ` Steffen Klassert
  2020-07-31  7:17 ` [PATCH 05/10] xfrm: policy: fix IPv6-only espintcp compilation Steffen Klassert
                   ` (6 subsequent siblings)
  10 siblings, 0 replies; 12+ messages in thread
From: Steffen Klassert @ 2020-07-31  7:17 UTC (permalink / raw)
  To: David Miller; +Cc: Herbert Xu, Steffen Klassert, netdev

From: Sabrina Dubroca <sd@queasysnail.net>

man 2 recv says:

    RETURN VALUE

    When a stream socket peer has performed an orderly shutdown, the
    return value will be 0 (the traditional "end-of-file" return).

Currently, this works for blocking reads, but non-blocking reads will
return -EAGAIN. This patch overwrites that return value when the peer
won't send us any more data.

Fixes: e27cca96cd68 ("xfrm: add espintcp (RFC 8229)")
Reported-by: Andrew Cagney <cagney@libreswan.org>
Tested-by: Andrew Cagney <cagney@libreswan.org>
Signed-off-by: Sabrina Dubroca <sd@queasysnail.net>
Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>
---
 net/xfrm/espintcp.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/net/xfrm/espintcp.c b/net/xfrm/espintcp.c
index 5d3d2b98c62d..cb83e3664680 100644
--- a/net/xfrm/espintcp.c
+++ b/net/xfrm/espintcp.c
@@ -109,8 +109,11 @@ static int espintcp_recvmsg(struct sock *sk, struct msghdr *msg, size_t len,
 	flags |= nonblock ? MSG_DONTWAIT : 0;
 
 	skb = __skb_recv_datagram(sk, &ctx->ike_queue, flags, &off, &err);
-	if (!skb)
+	if (!skb) {
+		if (err == -EAGAIN && sk->sk_shutdown & RCV_SHUTDOWN)
+			return 0;
 		return err;
+	}
 
 	copied = len;
 	if (copied > skb->len)
-- 
2.17.1


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

* [PATCH 05/10] xfrm: policy: fix IPv6-only espintcp compilation
  2020-07-31  7:17 pull request (net): ipsec 2020-07-31 Steffen Klassert
                   ` (3 preceding siblings ...)
  2020-07-31  7:17 ` [PATCH 04/10] espintcp: recv() should return 0 when the peer socket is closed Steffen Klassert
@ 2020-07-31  7:17 ` Steffen Klassert
  2020-07-31  7:18 ` [PATCH 06/10] xfrm: Fix crash when the hold queue is used Steffen Klassert
                   ` (5 subsequent siblings)
  10 siblings, 0 replies; 12+ messages in thread
From: Steffen Klassert @ 2020-07-31  7:17 UTC (permalink / raw)
  To: David Miller; +Cc: Herbert Xu, Steffen Klassert, netdev

From: Sabrina Dubroca <sd@queasysnail.net>

In case we're compiling espintcp support only for IPv6, we should
still initialize the common code.

Fixes: 26333c37fc28 ("xfrm: add IPv6 support for espintcp")
Signed-off-by: Sabrina Dubroca <sd@queasysnail.net>
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 6847b3579f54..19c5e0fa3f44 100644
--- a/net/xfrm/xfrm_policy.c
+++ b/net/xfrm/xfrm_policy.c
@@ -39,7 +39,7 @@
 #ifdef CONFIG_XFRM_STATISTICS
 #include <net/snmp.h>
 #endif
-#ifdef CONFIG_INET_ESPINTCP
+#ifdef CONFIG_XFRM_ESPINTCP
 #include <net/espintcp.h>
 #endif
 
@@ -4149,7 +4149,7 @@ void __init xfrm_init(void)
 	seqcount_init(&xfrm_policy_hash_generation);
 	xfrm_input_init();
 
-#ifdef CONFIG_INET_ESPINTCP
+#ifdef CONFIG_XFRM_ESPINTCP
 	espintcp_init();
 #endif
 
-- 
2.17.1


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

* [PATCH 06/10] xfrm: Fix crash when the hold queue is used.
  2020-07-31  7:17 pull request (net): ipsec 2020-07-31 Steffen Klassert
                   ` (4 preceding siblings ...)
  2020-07-31  7:17 ` [PATCH 05/10] xfrm: policy: fix IPv6-only espintcp compilation Steffen Klassert
@ 2020-07-31  7:18 ` Steffen Klassert
  2020-07-31  7:18 ` [PATCH 07/10] af_key: pfkey_dump needs parameter validation Steffen Klassert
                   ` (4 subsequent siblings)
  10 siblings, 0 replies; 12+ messages in thread
From: Steffen Klassert @ 2020-07-31  7:18 UTC (permalink / raw)
  To: David Miller; +Cc: Herbert Xu, Steffen Klassert, netdev

The commits "xfrm: Move dst->path into struct xfrm_dst"
and "net: Create and use new helper xfrm_dst_child()."
changed xfrm bundle handling under the assumption
that xdst->path and dst->child are not a NULL pointer
only if dst->xfrm is not a NULL pointer. That is true
with one exception. If the xfrm hold queue is used
to wait until a SA is installed by the key manager,
we create a dummy bundle without a valid dst->xfrm
pointer. The current xfrm bundle handling crashes
in that case. Fix this by extending the NULL check
of dst->xfrm with a test of the DST_XFRM_QUEUE flag.

Fixes: 0f6c480f23f4 ("xfrm: Move dst->path into struct xfrm_dst")
Fixes: b92cf4aab8e6 ("net: Create and use new helper xfrm_dst_child().")
Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>
---
 include/net/xfrm.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/include/net/xfrm.h b/include/net/xfrm.h
index 5c20953c8deb..51f65d23ebaf 100644
--- a/include/net/xfrm.h
+++ b/include/net/xfrm.h
@@ -941,7 +941,7 @@ struct xfrm_dst {
 static inline struct dst_entry *xfrm_dst_path(const struct dst_entry *dst)
 {
 #ifdef CONFIG_XFRM
-	if (dst->xfrm) {
+	if (dst->xfrm || (dst->flags & DST_XFRM_QUEUE)) {
 		const struct xfrm_dst *xdst = (const struct xfrm_dst *) dst;
 
 		return xdst->path;
@@ -953,7 +953,7 @@ static inline struct dst_entry *xfrm_dst_path(const struct dst_entry *dst)
 static inline struct dst_entry *xfrm_dst_child(const struct dst_entry *dst)
 {
 #ifdef CONFIG_XFRM
-	if (dst->xfrm) {
+	if (dst->xfrm || (dst->flags & DST_XFRM_QUEUE)) {
 		struct xfrm_dst *xdst = (struct xfrm_dst *) dst;
 		return xdst->child;
 	}
-- 
2.17.1


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

* [PATCH 07/10] af_key: pfkey_dump needs parameter validation
  2020-07-31  7:17 pull request (net): ipsec 2020-07-31 Steffen Klassert
                   ` (5 preceding siblings ...)
  2020-07-31  7:18 ` [PATCH 06/10] xfrm: Fix crash when the hold queue is used Steffen Klassert
@ 2020-07-31  7:18 ` Steffen Klassert
  2020-07-31  7:18 ` [PATCH 08/10] xfrm: esp6: fix the location of the transport header with encapsulation Steffen Klassert
                   ` (3 subsequent siblings)
  10 siblings, 0 replies; 12+ messages in thread
From: Steffen Klassert @ 2020-07-31  7:18 UTC (permalink / raw)
  To: David Miller; +Cc: Herbert Xu, Steffen Klassert, netdev

From: Mark Salyzyn <salyzyn@android.com>

In pfkey_dump() dplen and splen can both be specified to access the
xfrm_address_t structure out of bounds in__xfrm_state_filter_match()
when it calls addr_match() with the indexes.  Return EINVAL if either
are out of range.

Signed-off-by: Mark Salyzyn <salyzyn@android.com>
Cc: netdev@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Cc: kernel-team@android.com
Cc: Steffen Klassert <steffen.klassert@secunet.com>
Cc: Herbert Xu <herbert@gondor.apana.org.au>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: Jakub Kicinski <kuba@kernel.org>
Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>
---
 net/key/af_key.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/net/key/af_key.c b/net/key/af_key.c
index 979c579afc63..a915bc86620a 100644
--- a/net/key/af_key.c
+++ b/net/key/af_key.c
@@ -1849,6 +1849,13 @@ static int pfkey_dump(struct sock *sk, struct sk_buff *skb, const struct sadb_ms
 	if (ext_hdrs[SADB_X_EXT_FILTER - 1]) {
 		struct sadb_x_filter *xfilter = ext_hdrs[SADB_X_EXT_FILTER - 1];
 
+		if ((xfilter->sadb_x_filter_splen >=
+			(sizeof(xfrm_address_t) << 3)) ||
+		    (xfilter->sadb_x_filter_dplen >=
+			(sizeof(xfrm_address_t) << 3))) {
+			mutex_unlock(&pfk->dump_lock);
+			return -EINVAL;
+		}
 		filter = kmalloc(sizeof(*filter), GFP_KERNEL);
 		if (filter == NULL) {
 			mutex_unlock(&pfk->dump_lock);
-- 
2.17.1


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

* [PATCH 08/10] xfrm: esp6: fix the location of the transport header with encapsulation
  2020-07-31  7:17 pull request (net): ipsec 2020-07-31 Steffen Klassert
                   ` (6 preceding siblings ...)
  2020-07-31  7:18 ` [PATCH 07/10] af_key: pfkey_dump needs parameter validation Steffen Klassert
@ 2020-07-31  7:18 ` Steffen Klassert
  2020-07-31  7:18 ` [PATCH 09/10] espintcp: handle short messages instead of breaking the encap socket Steffen Klassert
                   ` (2 subsequent siblings)
  10 siblings, 0 replies; 12+ messages in thread
From: Steffen Klassert @ 2020-07-31  7:18 UTC (permalink / raw)
  To: David Miller; +Cc: Herbert Xu, Steffen Klassert, netdev

From: Sabrina Dubroca <sd@queasysnail.net>

commit 17175d1a27c6 ("xfrm: esp6: fix encapsulation header offset
computation") changed esp6_input_done2 to correctly find the size of
the IPv6 header that precedes the TCP/UDP encapsulation header, but
didn't adjust the final call to skb_set_transport_header, which I
assumed was correct in using skb_network_header_len.

Xiumei Mu reported that when we create xfrm states that include port
numbers in the selector, traffic from the user sockets is dropped. It
turns out that we get a state mismatch in __xfrm_policy_check, because
we end up trying to compare the encapsulation header's ports with the
selector that's based on user traffic ports.

Fixes: 0146dca70b87 ("xfrm: add support for UDPv6 encapsulation of ESP")
Fixes: 26333c37fc28 ("xfrm: add IPv6 support for espintcp")
Reported-by: Xiumei Mu <xmu@redhat.com>
Signed-off-by: Sabrina Dubroca <sd@queasysnail.net>
Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>
---
 net/ipv6/esp6.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/net/ipv6/esp6.c b/net/ipv6/esp6.c
index 55ae70be91b3..52c2f063529f 100644
--- a/net/ipv6/esp6.c
+++ b/net/ipv6/esp6.c
@@ -815,6 +815,7 @@ int esp6_input_done2(struct sk_buff *skb, int err)
 		offset = ipv6_skip_exthdr(skb, offset, &nexthdr, &frag_off);
 		uh = (void *)(skb->data + offset);
 		th = (void *)(skb->data + offset);
+		hdr_len += offset;
 
 		switch (x->encap->encap_type) {
 		case TCP_ENCAP_ESPINTCP:
-- 
2.17.1


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

* [PATCH 09/10] espintcp: handle short messages instead of breaking the encap socket
  2020-07-31  7:17 pull request (net): ipsec 2020-07-31 Steffen Klassert
                   ` (7 preceding siblings ...)
  2020-07-31  7:18 ` [PATCH 08/10] xfrm: esp6: fix the location of the transport header with encapsulation Steffen Klassert
@ 2020-07-31  7:18 ` Steffen Klassert
  2020-07-31  7:18 ` [PATCH 10/10] espintcp: count packets dropped in espintcp_rcv Steffen Klassert
  2020-08-01  0:11 ` pull request (net): ipsec 2020-07-31 David Miller
  10 siblings, 0 replies; 12+ messages in thread
From: Steffen Klassert @ 2020-07-31  7:18 UTC (permalink / raw)
  To: David Miller; +Cc: Herbert Xu, Steffen Klassert, netdev

From: Sabrina Dubroca <sd@queasysnail.net>

Currently, short messages (less than 4 bytes after the length header)
will break the stream of messages. This is unnecessary, since we can
still parse messages even if they're too short to contain any usable
data. This is also bogus, as keepalive messages (a single 0xff byte),
though not needed with TCP encapsulation, should be allowed.

This patch changes the stream parser so that short messages are
accepted and dropped in the kernel. Messages that contain a valid SPI
or non-ESP header are processed as before.

Fixes: e27cca96cd68 ("xfrm: add espintcp (RFC 8229)")
Reported-by: Andrew Cagney <cagney@libreswan.org>
Signed-off-by: Sabrina Dubroca <sd@queasysnail.net>
Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>
---
 net/xfrm/espintcp.c | 25 ++++++++++++++++++++++++-
 1 file changed, 24 insertions(+), 1 deletion(-)

diff --git a/net/xfrm/espintcp.c b/net/xfrm/espintcp.c
index cb83e3664680..0a91b07f2b43 100644
--- a/net/xfrm/espintcp.c
+++ b/net/xfrm/espintcp.c
@@ -49,9 +49,32 @@ static void espintcp_rcv(struct strparser *strp, struct sk_buff *skb)
 	struct espintcp_ctx *ctx = container_of(strp, struct espintcp_ctx,
 						strp);
 	struct strp_msg *rxm = strp_msg(skb);
+	int len = rxm->full_len - 2;
 	u32 nonesp_marker;
 	int err;
 
+	/* keepalive packet? */
+	if (unlikely(len == 1)) {
+		u8 data;
+
+		err = skb_copy_bits(skb, rxm->offset + 2, &data, 1);
+		if (err < 0) {
+			kfree_skb(skb);
+			return;
+		}
+
+		if (data == 0xff) {
+			kfree_skb(skb);
+			return;
+		}
+	}
+
+	/* drop other short messages */
+	if (unlikely(len <= sizeof(nonesp_marker))) {
+		kfree_skb(skb);
+		return;
+	}
+
 	err = skb_copy_bits(skb, rxm->offset + 2, &nonesp_marker,
 			    sizeof(nonesp_marker));
 	if (err < 0) {
@@ -91,7 +114,7 @@ static int espintcp_parse(struct strparser *strp, struct sk_buff *skb)
 		return err;
 
 	len = be16_to_cpu(blen);
-	if (len < 6)
+	if (len < 2)
 		return -EINVAL;
 
 	return len;
-- 
2.17.1


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

* [PATCH 10/10] espintcp: count packets dropped in espintcp_rcv
  2020-07-31  7:17 pull request (net): ipsec 2020-07-31 Steffen Klassert
                   ` (8 preceding siblings ...)
  2020-07-31  7:18 ` [PATCH 09/10] espintcp: handle short messages instead of breaking the encap socket Steffen Klassert
@ 2020-07-31  7:18 ` Steffen Klassert
  2020-08-01  0:11 ` pull request (net): ipsec 2020-07-31 David Miller
  10 siblings, 0 replies; 12+ messages in thread
From: Steffen Klassert @ 2020-07-31  7:18 UTC (permalink / raw)
  To: David Miller; +Cc: Herbert Xu, Steffen Klassert, netdev

From: Sabrina Dubroca <sd@queasysnail.net>

Currently, espintcp_rcv drops packets silently, which makes debugging
issues difficult. Count packets as either XfrmInHdrError (when the
packet was too short or contained invalid data) or XfrmInError (for
other issues).

Signed-off-by: Sabrina Dubroca <sd@queasysnail.net>
Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>
---
 net/xfrm/espintcp.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/net/xfrm/espintcp.c b/net/xfrm/espintcp.c
index 0a91b07f2b43..827ccdf2db57 100644
--- a/net/xfrm/espintcp.c
+++ b/net/xfrm/espintcp.c
@@ -15,6 +15,7 @@ static void handle_nonesp(struct espintcp_ctx *ctx, struct sk_buff *skb,
 {
 	if (atomic_read(&sk->sk_rmem_alloc) >= sk->sk_rcvbuf ||
 	    !sk_rmem_schedule(sk, skb, skb->truesize)) {
+		XFRM_INC_STATS(sock_net(sk), LINUX_MIB_XFRMINERROR);
 		kfree_skb(skb);
 		return;
 	}
@@ -59,6 +60,7 @@ static void espintcp_rcv(struct strparser *strp, struct sk_buff *skb)
 
 		err = skb_copy_bits(skb, rxm->offset + 2, &data, 1);
 		if (err < 0) {
+			XFRM_INC_STATS(sock_net(strp->sk), LINUX_MIB_XFRMINHDRERROR);
 			kfree_skb(skb);
 			return;
 		}
@@ -71,6 +73,7 @@ static void espintcp_rcv(struct strparser *strp, struct sk_buff *skb)
 
 	/* drop other short messages */
 	if (unlikely(len <= sizeof(nonesp_marker))) {
+		XFRM_INC_STATS(sock_net(strp->sk), LINUX_MIB_XFRMINHDRERROR);
 		kfree_skb(skb);
 		return;
 	}
@@ -78,17 +81,20 @@ static void espintcp_rcv(struct strparser *strp, struct sk_buff *skb)
 	err = skb_copy_bits(skb, rxm->offset + 2, &nonesp_marker,
 			    sizeof(nonesp_marker));
 	if (err < 0) {
+		XFRM_INC_STATS(sock_net(strp->sk), LINUX_MIB_XFRMINHDRERROR);
 		kfree_skb(skb);
 		return;
 	}
 
 	/* remove header, leave non-ESP marker/SPI */
 	if (!__pskb_pull(skb, rxm->offset + 2)) {
+		XFRM_INC_STATS(sock_net(strp->sk), LINUX_MIB_XFRMINERROR);
 		kfree_skb(skb);
 		return;
 	}
 
 	if (pskb_trim(skb, rxm->full_len - 2) != 0) {
+		XFRM_INC_STATS(sock_net(strp->sk), LINUX_MIB_XFRMINERROR);
 		kfree_skb(skb);
 		return;
 	}
-- 
2.17.1


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

* Re: pull request (net): ipsec 2020-07-31
  2020-07-31  7:17 pull request (net): ipsec 2020-07-31 Steffen Klassert
                   ` (9 preceding siblings ...)
  2020-07-31  7:18 ` [PATCH 10/10] espintcp: count packets dropped in espintcp_rcv Steffen Klassert
@ 2020-08-01  0:11 ` David Miller
  10 siblings, 0 replies; 12+ messages in thread
From: David Miller @ 2020-08-01  0:11 UTC (permalink / raw)
  To: steffen.klassert; +Cc: herbert, netdev

From: Steffen Klassert <steffen.klassert@secunet.com>
Date: Fri, 31 Jul 2020 09:17:54 +0200

> 1) Fix policy matching with mark and mask on userspace interfaces.
>    From Xin Long.
> 
> 2) Several fixes for the new ESP in TCP encapsulation.
>    From Sabrina Dubroca.
> 
> 3) Fix crash when the hold queue is used. The assumption that
>    xdst->path and dst->child are not a NULL pointer only if dst->xfrm
>    is not a NULL pointer is true with the exception of using the
>    hold queue. Fix this by checking for hold queue usage before
>    dereferencing xdst->path or dst->child.
> 
> 4) Validate pfkey_dump parameter before sending them.
>    From Mark Salyzyn.
> 
> 5) Fix the location of the transport header with ESP in UDPv6
>    encapsulation. From Sabrina Dubroca.
> 
> Please pull or let me know if there are problems.

Pulled, thanks a lot Steffen.

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

end of thread, other threads:[~2020-08-01  0:11 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-07-31  7:17 pull request (net): ipsec 2020-07-31 Steffen Klassert
2020-07-31  7:17 ` [PATCH 01/10] xfrm: policy: match with both mark and mask on user interfaces Steffen Klassert
2020-07-31  7:17 ` [PATCH 02/10] xfrm: esp6: fix encapsulation header offset computation Steffen Klassert
2020-07-31  7:17 ` [PATCH 03/10] espintcp: support non-blocking sends Steffen Klassert
2020-07-31  7:17 ` [PATCH 04/10] espintcp: recv() should return 0 when the peer socket is closed Steffen Klassert
2020-07-31  7:17 ` [PATCH 05/10] xfrm: policy: fix IPv6-only espintcp compilation Steffen Klassert
2020-07-31  7:18 ` [PATCH 06/10] xfrm: Fix crash when the hold queue is used Steffen Klassert
2020-07-31  7:18 ` [PATCH 07/10] af_key: pfkey_dump needs parameter validation Steffen Klassert
2020-07-31  7:18 ` [PATCH 08/10] xfrm: esp6: fix the location of the transport header with encapsulation Steffen Klassert
2020-07-31  7:18 ` [PATCH 09/10] espintcp: handle short messages instead of breaking the encap socket Steffen Klassert
2020-07-31  7:18 ` [PATCH 10/10] espintcp: count packets dropped in espintcp_rcv Steffen Klassert
2020-08-01  0:11 ` pull request (net): ipsec 2020-07-31 David Miller

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).