* [PATCH net] l2tp: hold tunnel socket when handling control frames in l2tp_ip and l2tp_ip6
@ 2017-03-28 13:32 Guillaume Nault
2017-03-29 4:34 ` David Miller
0 siblings, 1 reply; 5+ messages in thread
From: Guillaume Nault @ 2017-03-28 13:32 UTC (permalink / raw)
To: netdev; +Cc: James Chapman
The code following l2tp_tunnel_find() expects that a new reference is
held on sk. Either sk_receive_skb() or the discard_put error path will
drop a reference from the tunnel's socket.
This issue exists in both l2tp_ip and l2tp_ip6.
Signed-off-by: Guillaume Nault <g.nault@alphalink.fr>
---
net/l2tp/l2tp_ip.c | 5 +++--
net/l2tp/l2tp_ip6.c | 5 +++--
2 files changed, 6 insertions(+), 4 deletions(-)
diff --git a/net/l2tp/l2tp_ip.c b/net/l2tp/l2tp_ip.c
index d25038cfd64e..7208fbe5856b 100644
--- a/net/l2tp/l2tp_ip.c
+++ b/net/l2tp/l2tp_ip.c
@@ -178,9 +178,10 @@ static int l2tp_ip_recv(struct sk_buff *skb)
tunnel_id = ntohl(*(__be32 *) &skb->data[4]);
tunnel = l2tp_tunnel_find(net, tunnel_id);
- if (tunnel != NULL)
+ if (tunnel) {
sk = tunnel->sock;
- else {
+ sock_hold(sk);
+ } else {
struct iphdr *iph = (struct iphdr *) skb_network_header(skb);
read_lock_bh(&l2tp_ip_lock);
diff --git a/net/l2tp/l2tp_ip6.c b/net/l2tp/l2tp_ip6.c
index a4abcbc4c09a..516d7ce24ba7 100644
--- a/net/l2tp/l2tp_ip6.c
+++ b/net/l2tp/l2tp_ip6.c
@@ -191,9 +191,10 @@ static int l2tp_ip6_recv(struct sk_buff *skb)
tunnel_id = ntohl(*(__be32 *) &skb->data[4]);
tunnel = l2tp_tunnel_find(net, tunnel_id);
- if (tunnel != NULL)
+ if (tunnel) {
sk = tunnel->sock;
- else {
+ sock_hold(sk);
+ } else {
struct ipv6hdr *iph = ipv6_hdr(skb);
read_lock_bh(&l2tp_ip6_lock);
--
2.11.0
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH net] l2tp: hold tunnel socket when handling control frames in l2tp_ip and l2tp_ip6
2017-03-28 13:32 Guillaume Nault
@ 2017-03-29 4:34 ` David Miller
2017-03-29 6:36 ` Guillaume Nault
0 siblings, 1 reply; 5+ messages in thread
From: David Miller @ 2017-03-29 4:34 UTC (permalink / raw)
To: g.nault; +Cc: netdev, jchapman
From: Guillaume Nault <g.nault@alphalink.fr>
Date: Tue, 28 Mar 2017 15:32:35 +0200
> The code following l2tp_tunnel_find() expects that a new reference is
> held on sk. Either sk_receive_skb() or the discard_put error path will
> drop a reference from the tunnel's socket.
>
> This issue exists in both l2tp_ip and l2tp_ip6.
>
> Signed-off-by: Guillaume Nault <g.nault@alphalink.fr>
You introduced this bug in commit:
====================
commit a3c18422a4b4e108bcf6a2328f48867e1003fd95
Author: Guillaume Nault <g.nault@alphalink.fr>
Date: Tue Nov 29 13:09:45 2016 +0100
l2tp: hold socket before dropping lock in l2tp_ip{, 6}_recv()
====================
Therefore you should make this clear with a proper "Fixes: " tag
such as:
Fixes: a3c18422a4b4 ("l2tp: hold socket before dropping lock in l2tp_ip{, 6}_recv()")
on a line right before your signoff.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH net] l2tp: hold tunnel socket when handling control frames in l2tp_ip and l2tp_ip6
2017-03-29 4:34 ` David Miller
@ 2017-03-29 6:36 ` Guillaume Nault
0 siblings, 0 replies; 5+ messages in thread
From: Guillaume Nault @ 2017-03-29 6:36 UTC (permalink / raw)
To: David Miller; +Cc: netdev, jchapman
On Tue, Mar 28, 2017 at 09:34:16PM -0700, David Miller wrote:
> You introduced this bug in commit:
>
> ====================
> commit a3c18422a4b4e108bcf6a2328f48867e1003fd95
> Author: Guillaume Nault <g.nault@alphalink.fr>
> Date: Tue Nov 29 13:09:45 2016 +0100
>
> l2tp: hold socket before dropping lock in l2tp_ip{, 6}_recv()
> ====================
>
> Therefore you should make this clear with a proper "Fixes: " tag
> such as:
>
> Fixes: a3c18422a4b4 ("l2tp: hold socket before dropping lock in l2tp_ip{, 6}_recv()")
>
> on a line right before your signoff.
Indeed, I should have realised that earlier, sorry. I'll resubmit.
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH net] l2tp: hold tunnel socket when handling control frames in l2tp_ip and l2tp_ip6
@ 2017-03-29 6:44 Guillaume Nault
2017-03-29 16:26 ` David Miller
0 siblings, 1 reply; 5+ messages in thread
From: Guillaume Nault @ 2017-03-29 6:44 UTC (permalink / raw)
To: netdev; +Cc: James Chapman
The code following l2tp_tunnel_find() expects that a new reference is
held on sk. Either sk_receive_skb() or the discard_put error path will
drop a reference from the tunnel's socket.
This issue exists in both l2tp_ip and l2tp_ip6.
Fixes: a3c18422a4b4 ("l2tp: hold socket before dropping lock in l2tp_ip{, 6}_recv()")
Signed-off-by: Guillaume Nault <g.nault@alphalink.fr>
---
net/l2tp/l2tp_ip.c | 5 +++--
net/l2tp/l2tp_ip6.c | 5 +++--
2 files changed, 6 insertions(+), 4 deletions(-)
diff --git a/net/l2tp/l2tp_ip.c b/net/l2tp/l2tp_ip.c
index d25038cfd64e..7208fbe5856b 100644
--- a/net/l2tp/l2tp_ip.c
+++ b/net/l2tp/l2tp_ip.c
@@ -178,9 +178,10 @@ static int l2tp_ip_recv(struct sk_buff *skb)
tunnel_id = ntohl(*(__be32 *) &skb->data[4]);
tunnel = l2tp_tunnel_find(net, tunnel_id);
- if (tunnel != NULL)
+ if (tunnel) {
sk = tunnel->sock;
- else {
+ sock_hold(sk);
+ } else {
struct iphdr *iph = (struct iphdr *) skb_network_header(skb);
read_lock_bh(&l2tp_ip_lock);
diff --git a/net/l2tp/l2tp_ip6.c b/net/l2tp/l2tp_ip6.c
index a4abcbc4c09a..516d7ce24ba7 100644
--- a/net/l2tp/l2tp_ip6.c
+++ b/net/l2tp/l2tp_ip6.c
@@ -191,9 +191,10 @@ static int l2tp_ip6_recv(struct sk_buff *skb)
tunnel_id = ntohl(*(__be32 *) &skb->data[4]);
tunnel = l2tp_tunnel_find(net, tunnel_id);
- if (tunnel != NULL)
+ if (tunnel) {
sk = tunnel->sock;
- else {
+ sock_hold(sk);
+ } else {
struct ipv6hdr *iph = ipv6_hdr(skb);
read_lock_bh(&l2tp_ip6_lock);
--
2.11.0
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH net] l2tp: hold tunnel socket when handling control frames in l2tp_ip and l2tp_ip6
2017-03-29 6:44 [PATCH net] l2tp: hold tunnel socket when handling control frames in l2tp_ip and l2tp_ip6 Guillaume Nault
@ 2017-03-29 16:26 ` David Miller
0 siblings, 0 replies; 5+ messages in thread
From: David Miller @ 2017-03-29 16:26 UTC (permalink / raw)
To: g.nault; +Cc: netdev, jchapman
From: Guillaume Nault <g.nault@alphalink.fr>
Date: Wed, 29 Mar 2017 08:44:59 +0200
> The code following l2tp_tunnel_find() expects that a new reference is
> held on sk. Either sk_receive_skb() or the discard_put error path will
> drop a reference from the tunnel's socket.
>
> This issue exists in both l2tp_ip and l2tp_ip6.
>
> Fixes: a3c18422a4b4 ("l2tp: hold socket before dropping lock in l2tp_ip{, 6}_recv()")
> Signed-off-by: Guillaume Nault <g.nault@alphalink.fr>
Applied.
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2017-03-29 16:27 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-03-29 6:44 [PATCH net] l2tp: hold tunnel socket when handling control frames in l2tp_ip and l2tp_ip6 Guillaume Nault
2017-03-29 16:26 ` David Miller
-- strict thread matches above, loose matches on Subject: below --
2017-03-28 13:32 Guillaume Nault
2017-03-29 4:34 ` David Miller
2017-03-29 6:36 ` Guillaume Nault
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).