All of lore.kernel.org
 help / color / mirror / Atom feed
From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
To: linux-kernel@vger.kernel.org
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	stable@vger.kernel.org, Guillaume Nault <g.nault@alphalink.fr>,
	"David S. Miller" <davem@davemloft.net>
Subject: [PATCH 4.13 06/35] l2tp: dont use l2tp_tunnel_find() in l2tp_ip and l2tp_ip6
Date: Wed, 22 Nov 2017 11:12:00 +0100	[thread overview]
Message-ID: <20171122101138.508923876@linuxfoundation.org> (raw)
In-Reply-To: <20171122101137.661212603@linuxfoundation.org>

4.13-stable review patch.  If anyone has any objections, please let me know.

------------------

From: Guillaume Nault <g.nault@alphalink.fr>


[ Upstream commit 8f7dc9ae4a7aece9fbc3e6637bdfa38b36bcdf09 ]

Using l2tp_tunnel_find() in l2tp_ip_recv() is wrong for two reasons:

  * It doesn't take a reference on the returned tunnel, which makes the
    call racy wrt. concurrent tunnel deletion.

  * The lookup is only based on the tunnel identifier, so it can return
    a tunnel that doesn't match the packet's addresses or protocol.

For example, a packet sent to an L2TPv3 over IPv6 tunnel can be
delivered to an L2TPv2 over UDPv4 tunnel. This is worse than a simple
cross-talk: when delivering the packet to an L2TP over UDP tunnel, the
corresponding socket is UDP, where ->sk_backlog_rcv() is NULL. Calling
sk_receive_skb() will then crash the kernel by trying to execute this
callback.

And l2tp_tunnel_find() isn't even needed here. __l2tp_ip_bind_lookup()
properly checks the socket binding and connection settings. It was used
as a fallback mechanism for finding tunnels that didn't have their data
path registered yet. But it's not limited to this case and can be used
to replace l2tp_tunnel_find() in the general case.

Fix l2tp_ip6 in the same way.

Fixes: 0d76751fad77 ("l2tp: Add L2TPv3 IP encapsulation (no UDP) support")
Fixes: a32e0eec7042 ("l2tp: introduce L2TPv3 IP encapsulation support for IPv6")
Signed-off-by: Guillaume Nault <g.nault@alphalink.fr>
Signed-off-by: David S. Miller <davem@davemloft.net>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
 net/l2tp/l2tp_ip.c  |   24 +++++++++---------------
 net/l2tp/l2tp_ip6.c |   24 +++++++++---------------
 2 files changed, 18 insertions(+), 30 deletions(-)

--- a/net/l2tp/l2tp_ip.c
+++ b/net/l2tp/l2tp_ip.c
@@ -123,6 +123,7 @@ static int l2tp_ip_recv(struct sk_buff *
 	unsigned char *ptr, *optr;
 	struct l2tp_session *session;
 	struct l2tp_tunnel *tunnel = NULL;
+	struct iphdr *iph;
 	int length;
 
 	if (!pskb_may_pull(skb, 4))
@@ -178,24 +179,17 @@ pass_up:
 		goto discard;
 
 	tunnel_id = ntohl(*(__be32 *) &skb->data[4]);
-	tunnel = l2tp_tunnel_find(net, tunnel_id);
-	if (tunnel) {
-		sk = tunnel->sock;
-		sock_hold(sk);
-	} else {
-		struct iphdr *iph = (struct iphdr *) skb_network_header(skb);
-
-		read_lock_bh(&l2tp_ip_lock);
-		sk = __l2tp_ip_bind_lookup(net, iph->daddr, iph->saddr,
-					   inet_iif(skb), tunnel_id);
-		if (!sk) {
-			read_unlock_bh(&l2tp_ip_lock);
-			goto discard;
-		}
+	iph = (struct iphdr *)skb_network_header(skb);
 
-		sock_hold(sk);
+	read_lock_bh(&l2tp_ip_lock);
+	sk = __l2tp_ip_bind_lookup(net, iph->daddr, iph->saddr, inet_iif(skb),
+				   tunnel_id);
+	if (!sk) {
 		read_unlock_bh(&l2tp_ip_lock);
+		goto discard;
 	}
+	sock_hold(sk);
+	read_unlock_bh(&l2tp_ip_lock);
 
 	if (!xfrm4_policy_check(sk, XFRM_POLICY_IN, skb))
 		goto discard_put;
--- a/net/l2tp/l2tp_ip6.c
+++ b/net/l2tp/l2tp_ip6.c
@@ -136,6 +136,7 @@ static int l2tp_ip6_recv(struct sk_buff
 	unsigned char *ptr, *optr;
 	struct l2tp_session *session;
 	struct l2tp_tunnel *tunnel = NULL;
+	struct ipv6hdr *iph;
 	int length;
 
 	if (!pskb_may_pull(skb, 4))
@@ -192,24 +193,17 @@ pass_up:
 		goto discard;
 
 	tunnel_id = ntohl(*(__be32 *) &skb->data[4]);
-	tunnel = l2tp_tunnel_find(net, tunnel_id);
-	if (tunnel) {
-		sk = tunnel->sock;
-		sock_hold(sk);
-	} else {
-		struct ipv6hdr *iph = ipv6_hdr(skb);
-
-		read_lock_bh(&l2tp_ip6_lock);
-		sk = __l2tp_ip6_bind_lookup(net, &iph->daddr, &iph->saddr,
-					    inet6_iif(skb), tunnel_id);
-		if (!sk) {
-			read_unlock_bh(&l2tp_ip6_lock);
-			goto discard;
-		}
+	iph = ipv6_hdr(skb);
 
-		sock_hold(sk);
+	read_lock_bh(&l2tp_ip6_lock);
+	sk = __l2tp_ip6_bind_lookup(net, &iph->daddr, &iph->saddr,
+				    inet6_iif(skb), tunnel_id);
+	if (!sk) {
 		read_unlock_bh(&l2tp_ip6_lock);
+		goto discard;
 	}
+	sock_hold(sk);
+	read_unlock_bh(&l2tp_ip6_lock);
 
 	if (!xfrm6_policy_check(sk, XFRM_POLICY_IN, skb))
 		goto discard_put;

  parent reply	other threads:[~2017-11-22 10:17 UTC|newest]

Thread overview: 35+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-11-22 10:11 [PATCH 4.13 00/35] 4.13.16-stable review Greg Kroah-Hartman
2017-11-22 10:11 ` [PATCH 4.13 01/35] tcp_nv: fix division by zero in tcpnv_acked() Greg Kroah-Hartman
2017-11-22 10:11 ` [PATCH 4.13 02/35] net: vrf: correct FRA_L3MDEV encode type Greg Kroah-Hartman
2017-11-22 10:11 ` [PATCH 4.13 03/35] tcp: do not mangle skb->cb[] in tcp_make_synack() Greg Kroah-Hartman
2017-11-22 10:11 ` [PATCH 4.13 04/35] net: systemport: Correct IPG length settings Greg Kroah-Hartman
2017-11-22 10:11 ` [PATCH 4.13 05/35] netfilter/ipvs: clear ipvs_property flag when SKB net namespace changed Greg Kroah-Hartman
2017-11-22 10:12 ` Greg Kroah-Hartman [this message]
2017-11-22 10:12 ` [PATCH 4.13 07/35] bonding: discard lowest hash bit for 802.3ad layer3+4 Greg Kroah-Hartman
2017-11-22 10:12 ` [PATCH 4.13 11/35] net: usb: asix: fill null-ptr-deref in asix_suspend Greg Kroah-Hartman
2017-11-22 10:12 ` [PATCH 4.13 12/35] tcp: gso: avoid refcount_t warning from tcp_gso_segment() Greg Kroah-Hartman
2017-11-22 10:12 ` [PATCH 4.13 13/35] tcp: fix tcp_fastretrans_alert warning Greg Kroah-Hartman
2017-11-22 10:12 ` [PATCH 4.13 14/35] vlan: fix a use-after-free in vlan_device_event() Greg Kroah-Hartman
2017-11-22 10:12 ` [PATCH 4.13 15/35] net/mlx5: Cancel health poll before sending panic teardown command Greg Kroah-Hartman
2017-11-22 10:12 ` [PATCH 4.13 16/35] net/mlx5e: Set page to null in case dma mapping fails Greg Kroah-Hartman
2017-11-22 10:12 ` [PATCH 4.13 17/35] af_netlink: ensure that NLMSG_DONE never fails in dumps Greg Kroah-Hartman
2017-11-22 10:12 ` [PATCH 4.13 18/35] vxlan: fix the issue that neigh proxy blocks all icmpv6 packets Greg Kroah-Hartman
2017-11-22 10:12 ` [PATCH 4.13 20/35] sctp: do not peel off an assoc from one netns to another one Greg Kroah-Hartman
2017-11-22 10:12 ` [PATCH 4.13 21/35] fealnx: Fix building error on MIPS Greg Kroah-Hartman
2017-11-22 10:12 ` [PATCH 4.13 22/35] net/sctp: Always set scope_id in sctp_inet6_skb_msgname Greg Kroah-Hartman
2017-11-22 10:12 ` [PATCH 4.13 23/35] ima: do not update security.ima if appraisal status is not INTEGRITY_PASS Greg Kroah-Hartman
2017-11-22 10:12 ` [PATCH 4.13 24/35] serial: omap: Fix EFR write on RTS deassertion Greg Kroah-Hartman
2017-11-22 10:12 ` [PATCH 4.13 25/35] serial: 8250_fintek: Fix finding base_port with activated SuperIO Greg Kroah-Hartman
2017-11-22 10:12 ` [PATCH 4.13 26/35] tpm-dev-common: Reject too short writes Greg Kroah-Hartman
2017-11-22 10:12 ` [PATCH 4.13 27/35] rcu: Fix up pending cbs check in rcu_prepare_for_idle Greg Kroah-Hartman
2017-11-22 10:12 ` [PATCH 4.13 28/35] mm/pagewalk.c: report holes in hugetlb ranges Greg Kroah-Hartman
2017-11-22 10:12 ` [PATCH 4.13 29/35] ocfs2: fix cluster hang after a node dies Greg Kroah-Hartman
2017-11-22 10:12 ` [PATCH 4.13 30/35] ocfs2: should wait dio before inode lock in ocfs2_setattr() Greg Kroah-Hartman
2017-11-22 10:12 ` [PATCH 4.13 31/35] ipmi: fix unsigned long underflow Greg Kroah-Hartman
2017-11-22 10:12 ` [PATCH 4.13 32/35] mm/page_alloc.c: broken deferred calculation Greg Kroah-Hartman
2017-11-22 10:12 ` [PATCH 4.13 33/35] mm/page_ext.c: check if page_ext is not prepared Greg Kroah-Hartman
2017-11-22 10:12 ` [PATCH 4.13 34/35] x86/cpu/amd: Derive L3 shared_cpu_map from cpu_llc_shared_mask Greg Kroah-Hartman
2017-11-22 10:12 ` [PATCH 4.13 35/35] coda: fix kernel memory exposure attempt in fsync Greg Kroah-Hartman
2017-11-22 16:49 ` [PATCH 4.13 00/35] 4.13.16-stable review Greg Kroah-Hartman
2017-11-22 21:33 ` Guenter Roeck
2017-11-23 14:48 ` Naresh Kamboju

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20171122101138.508923876@linuxfoundation.org \
    --to=gregkh@linuxfoundation.org \
    --cc=davem@davemloft.net \
    --cc=g.nault@alphalink.fr \
    --cc=linux-kernel@vger.kernel.org \
    --cc=stable@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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.