| Message ID | 5e9e60582739bc2b9def6c29abe071913632878a.1490707863.git.g.nault@alphalink.fr |
|---|---|
| State | Changes Requested, archived |
| Delegated to: | David Miller |
| Headers | show |
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.
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.
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);
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(-)