]>
Commit | Line | Data |
---|---|---|
e1e69077 GKH |
1 | From 40cad6ae6c20055eb7a7c06055c257dc6460217c Mon Sep 17 00:00:00 2001 |
2 | From: James Chapman <jchapman@katalix.com> | |
3 | Date: Tue, 16 Mar 2010 06:29:20 +0000 | |
4 | Subject: l2tp: Fix UDP socket reference count bugs in the pppol2tp driver | |
5 | ||
6 | From: James Chapman <jchapman@katalix.com> | |
7 | ||
8 | [ Upstream commit c3259c8a7060d480e8eb2166da0a99d6879146b4 ] | |
9 | ||
10 | This patch fixes UDP socket refcnt bugs in the pppol2tp driver. | |
11 | ||
12 | A bug can cause a kernel stack trace when a tunnel socket is closed. | |
13 | ||
14 | A way to reproduce the issue is to prepare the UDP socket for L2TP (by | |
15 | opening a tunnel pppol2tp socket) and then close it before any L2TP | |
16 | sessions are added to it. The sequence is | |
17 | ||
18 | Create UDP socket | |
19 | Create tunnel pppol2tp socket to prepare UDP socket for L2TP | |
20 | pppol2tp_connect: session_id=0, peer_session_id=0 | |
21 | L2TP SCCRP control frame received (tunnel_id==0) | |
22 | pppol2tp_recv_core: sock_hold() | |
23 | pppol2tp_recv_core: sock_put | |
24 | L2TP ZLB control frame received (tunnel_id=nnn) | |
25 | pppol2tp_recv_core: sock_hold() | |
26 | pppol2tp_recv_core: sock_put | |
27 | Close tunnel management socket | |
28 | pppol2tp_release: session_id=0, peer_session_id=0 | |
29 | Close UDP socket | |
30 | udp_lib_close: BUG | |
31 | ||
32 | The addition of sock_hold() in pppol2tp_connect() solves the problem. | |
33 | ||
34 | For data frames, two sock_put() calls were added to plug a refcnt leak | |
35 | per received data frame. The ref that is grabbed at the top of | |
36 | pppol2tp_recv_core() must always be released, but this wasn't done for | |
37 | accepted data frames or data frames discarded because of bad UDP | |
38 | checksums. This leak meant that any UDP socket that had passed L2TP | |
39 | data traffic (i.e. L2TP data frames, not just L2TP control frames) | |
40 | using pppol2tp would not be released by the kernel. | |
41 | ||
42 | WARNING: at include/net/sock.h:435 udp_lib_unhash+0x117/0x120() | |
43 | Pid: 1086, comm: openl2tpd Not tainted 2.6.33-rc1 #8 | |
44 | Call Trace: | |
45 | [<c119e9b7>] ? udp_lib_unhash+0x117/0x120 | |
46 | [<c101b871>] ? warn_slowpath_common+0x71/0xd0 | |
47 | [<c119e9b7>] ? udp_lib_unhash+0x117/0x120 | |
48 | [<c101b8e3>] ? warn_slowpath_null+0x13/0x20 | |
49 | [<c119e9b7>] ? udp_lib_unhash+0x117/0x120 | |
50 | [<c11598a7>] ? sk_common_release+0x17/0x90 | |
51 | [<c11a5e33>] ? inet_release+0x33/0x60 | |
52 | [<c11577b0>] ? sock_release+0x10/0x60 | |
53 | [<c115780f>] ? sock_close+0xf/0x30 | |
54 | [<c106e542>] ? __fput+0x52/0x150 | |
55 | [<c106b68e>] ? filp_close+0x3e/0x70 | |
56 | [<c101d2e2>] ? put_files_struct+0x62/0xb0 | |
57 | [<c101eaf7>] ? do_exit+0x5e7/0x650 | |
58 | [<c1081623>] ? mntput_no_expire+0x13/0x70 | |
59 | [<c106b68e>] ? filp_close+0x3e/0x70 | |
60 | [<c101eb8a>] ? do_group_exit+0x2a/0x70 | |
61 | [<c101ebe1>] ? sys_exit_group+0x11/0x20 | |
62 | [<c10029b0>] ? sysenter_do_call+0x12/0x26 | |
63 | ||
64 | Signed-off-by: James Chapman <jchapman@katalix.com> | |
65 | Signed-off-by: David S. Miller <davem@davemloft.net> | |
66 | Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de> | |
67 | ||
68 | --- | |
69 | drivers/net/pppol2tp.c | 3 +++ | |
70 | 1 file changed, 3 insertions(+) | |
71 | ||
72 | --- a/drivers/net/pppol2tp.c | |
73 | +++ b/drivers/net/pppol2tp.c | |
74 | @@ -756,6 +756,7 @@ static int pppol2tp_recv_core(struct soc | |
75 | ||
76 | /* Try to dequeue as many skbs from reorder_q as we can. */ | |
77 | pppol2tp_recv_dequeue(session); | |
78 | + sock_put(sock); | |
79 | ||
80 | return 0; | |
81 | ||
82 | @@ -772,6 +773,7 @@ discard_bad_csum: | |
83 | UDP_INC_STATS_USER(&init_net, UDP_MIB_INERRORS, 0); | |
84 | tunnel->stats.rx_errors++; | |
85 | kfree_skb(skb); | |
86 | + sock_put(sock); | |
87 | ||
88 | return 0; | |
89 | ||
90 | @@ -1662,6 +1664,7 @@ static int pppol2tp_connect(struct socke | |
91 | if (tunnel_sock == NULL) | |
92 | goto end; | |
93 | ||
94 | + sock_hold(tunnel_sock); | |
95 | tunnel = tunnel_sock->sk_user_data; | |
96 | } else { | |
97 | tunnel = pppol2tp_tunnel_find(sock_net(sk), sp->pppol2tp.s_tunnel); |