* [PATCH 0/1] Missing l2tp patch in v4.9.y series
@ 2019-11-27 10:02 Nicolas Schier
2019-11-27 10:02 ` [PATCH 1/1] l2tp: don't use l2tp_tunnel_find() in l2tp_ip and l2tp_ip6 Nicolas Schier
0 siblings, 1 reply; 3+ messages in thread
From: Nicolas Schier @ 2019-11-27 10:02 UTC (permalink / raw)
To: stable; +Cc: Nicolas Schier, Guillaume Nault, David S . Miller
Hi,
the following l2tp patch originates from v4.14 and has been backported
to 3.16.y and 4.13.y but it is missing in the 4.9.y branch
(accidentally?). As it applies cleanly to 4.9.y and as I couldn't find
any mails discouraging the inclusion into 4.9, I am now asking for
inclusion into 4.9.y.
Kind regards,
Nicolas
Guillaume Nault (1):
l2tp: don't use l2tp_tunnel_find() in l2tp_ip and l2tp_ip6
net/l2tp/l2tp_ip.c | 24 +++++++++---------------
net/l2tp/l2tp_ip6.c | 24 +++++++++---------------
2 files changed, 18 insertions(+), 30 deletions(-)
--
2.24.0
^ permalink raw reply [flat|nested] 3+ messages in thread
* [PATCH 1/1] l2tp: don't use l2tp_tunnel_find() in l2tp_ip and l2tp_ip6
2019-11-27 10:02 [PATCH 0/1] Missing l2tp patch in v4.9.y series Nicolas Schier
@ 2019-11-27 10:02 ` Nicolas Schier
2019-11-27 10:28 ` Greg KH
0 siblings, 1 reply; 3+ messages in thread
From: Nicolas Schier @ 2019-11-27 10:02 UTC (permalink / raw)
To: stable; +Cc: Nicolas Schier, Guillaume Nault, David S . Miller
From: Guillaume Nault <g.nault@alphalink.fr>
commit 8f7dc9ae4a7aece9fbc3e6637bdfa38b36bcdf09 upstream.
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: Nicolas Schier <n.schier@avm.de>
---
Please consider queuing this patch for v4.9.y.
net/l2tp/l2tp_ip.c | 24 +++++++++---------------
net/l2tp/l2tp_ip6.c | 24 +++++++++---------------
2 files changed, 18 insertions(+), 30 deletions(-)
diff --git a/net/l2tp/l2tp_ip.c b/net/l2tp/l2tp_ip.c
index 03a696d3bcd9..4a88c4eb2301 100644
--- a/net/l2tp/l2tp_ip.c
+++ b/net/l2tp/l2tp_ip.c
@@ -116,6 +116,7 @@ static int l2tp_ip_recv(struct sk_buff *skb)
unsigned char *ptr, *optr;
struct l2tp_session *session;
struct l2tp_tunnel *tunnel = NULL;
+ struct iphdr *iph;
int length;
if (!pskb_may_pull(skb, 4))
@@ -174,24 +175,17 @@ static int l2tp_ip_recv(struct sk_buff *skb)
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;
diff --git a/net/l2tp/l2tp_ip6.c b/net/l2tp/l2tp_ip6.c
index 8d412b9b0214..423cb095ad37 100644
--- a/net/l2tp/l2tp_ip6.c
+++ b/net/l2tp/l2tp_ip6.c
@@ -128,6 +128,7 @@ static int l2tp_ip6_recv(struct sk_buff *skb)
unsigned char *ptr, *optr;
struct l2tp_session *session;
struct l2tp_tunnel *tunnel = NULL;
+ struct ipv6hdr *iph;
int length;
if (!pskb_may_pull(skb, 4))
@@ -187,24 +188,17 @@ static int l2tp_ip6_recv(struct sk_buff *skb)
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;
--
2.24.0
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH 1/1] l2tp: don't use l2tp_tunnel_find() in l2tp_ip and l2tp_ip6
2019-11-27 10:02 ` [PATCH 1/1] l2tp: don't use l2tp_tunnel_find() in l2tp_ip and l2tp_ip6 Nicolas Schier
@ 2019-11-27 10:28 ` Greg KH
0 siblings, 0 replies; 3+ messages in thread
From: Greg KH @ 2019-11-27 10:28 UTC (permalink / raw)
To: Nicolas Schier; +Cc: stable, Guillaume Nault, David S . Miller
On Wed, Nov 27, 2019 at 11:02:49AM +0100, Nicolas Schier wrote:
> From: Guillaume Nault <g.nault@alphalink.fr>
>
> commit 8f7dc9ae4a7aece9fbc3e6637bdfa38b36bcdf09 upstream.
>
> 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: Nicolas Schier <n.schier@avm.de>
> ---
> Please consider queuing this patch for v4.9.y.
Now queued up, thanks.
greg k-h
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2019-11-27 10:29 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-11-27 10:02 [PATCH 0/1] Missing l2tp patch in v4.9.y series Nicolas Schier
2019-11-27 10:02 ` [PATCH 1/1] l2tp: don't use l2tp_tunnel_find() in l2tp_ip and l2tp_ip6 Nicolas Schier
2019-11-27 10:28 ` Greg KH
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).