]>
Commit | Line | Data |
---|---|---|
91e32d44 GKH |
1 | From 9c3f3794926a997b1cab6c42480ff300efa2d162 Mon Sep 17 00:00:00 2001 |
2 | From: Liping Zhang <zlpnobody@gmail.com> | |
3 | Date: Sat, 25 Mar 2017 16:35:29 +0800 | |
4 | Subject: netfilter: nf_ct_ext: fix possible panic after nf_ct_extend_unregister | |
5 | ||
6 | From: Liping Zhang <zlpnobody@gmail.com> | |
7 | ||
8 | commit 9c3f3794926a997b1cab6c42480ff300efa2d162 upstream. | |
9 | ||
10 | If one cpu is doing nf_ct_extend_unregister while another cpu is doing | |
11 | __nf_ct_ext_add_length, then we may hit BUG_ON(t == NULL). Moreover, | |
12 | there's no synchronize_rcu invocation after set nf_ct_ext_types[id] to | |
13 | NULL, so it's possible that we may access invalid pointer. | |
14 | ||
15 | But actually, most of the ct extends are built-in, so the problem listed | |
16 | above will not happen. However, there are two exceptions: NF_CT_EXT_NAT | |
17 | and NF_CT_EXT_SYNPROXY. | |
18 | ||
19 | For _EXT_NAT, the panic will not happen, since adding the nat extend and | |
20 | unregistering the nat extend are located in the same file(nf_nat_core.c), | |
21 | this means that after the nat module is removed, we cannot add the nat | |
22 | extend too. | |
23 | ||
24 | For _EXT_SYNPROXY, synproxy extend may be added by init_conntrack, while | |
25 | synproxy extend unregister will be done by synproxy_core_exit. So after | |
26 | nf_synproxy_core.ko is removed, we may still try to add the synproxy | |
27 | extend, then kernel panic may happen. | |
28 | ||
29 | I know it's very hard to reproduce this issue, but I can play a tricky | |
30 | game to make it happen very easily :) | |
31 | ||
32 | Step 1. Enable SYNPROXY for tcp dport 1234 at FORWARD hook: | |
33 | # iptables -I FORWARD -p tcp --dport 1234 -j SYNPROXY | |
34 | Step 2. Queue the syn packet to the userspace at raw table OUTPUT hook. | |
35 | Also note, in the userspace we only add a 20s' delay, then | |
36 | reinject the syn packet to the kernel: | |
37 | # iptables -t raw -I OUTPUT -p tcp --syn -j NFQUEUE --queue-num 1 | |
38 | Step 3. Using "nc 2.2.2.2 1234" to connect the server. | |
39 | Step 4. Now remove the nf_synproxy_core.ko quickly: | |
40 | # iptables -F FORWARD | |
41 | # rmmod ipt_SYNPROXY | |
42 | # rmmod nf_synproxy_core | |
43 | Step 5. After 20s' delay, the syn packet is reinjected to the kernel. | |
44 | ||
45 | Now you will see the panic like this: | |
46 | kernel BUG at net/netfilter/nf_conntrack_extend.c:91! | |
47 | Call Trace: | |
48 | ? __nf_ct_ext_add_length+0x53/0x3c0 [nf_conntrack] | |
49 | init_conntrack+0x12b/0x600 [nf_conntrack] | |
50 | nf_conntrack_in+0x4cc/0x580 [nf_conntrack] | |
51 | ipv4_conntrack_local+0x48/0x50 [nf_conntrack_ipv4] | |
52 | nf_reinject+0x104/0x270 | |
53 | nfqnl_recv_verdict+0x3e1/0x5f9 [nfnetlink_queue] | |
54 | ? nfqnl_recv_verdict+0x5/0x5f9 [nfnetlink_queue] | |
55 | ? nla_parse+0xa0/0x100 | |
56 | nfnetlink_rcv_msg+0x175/0x6a9 [nfnetlink] | |
57 | [...] | |
58 | ||
59 | One possible solution is to make NF_CT_EXT_SYNPROXY extend built-in, i.e. | |
60 | introduce nf_conntrack_synproxy.c and only do ct extend register and | |
61 | unregister in it, similar to nf_conntrack_timeout.c. | |
62 | ||
63 | But having such a obscure restriction of nf_ct_extend_unregister is not a | |
64 | good idea, so we should invoke synchronize_rcu after set nf_ct_ext_types | |
65 | to NULL, and check the NULL pointer when do __nf_ct_ext_add_length. Then | |
66 | it will be easier if we add new ct extend in the future. | |
67 | ||
68 | Last, we use kfree_rcu to free nf_ct_ext, so rcu_barrier() is unnecessary | |
69 | anymore, remove it too. | |
70 | ||
71 | Signed-off-by: Liping Zhang <zlpnobody@gmail.com> | |
72 | Acked-by: Florian Westphal <fw@strlen.de> | |
73 | Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org> | |
74 | Cc: Stefan Bader <stefan.bader@canonical.com> | |
75 | Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> | |
76 | ||
77 | --- | |
78 | net/netfilter/nf_conntrack_extend.c | 13 ++++++++++--- | |
79 | 1 file changed, 10 insertions(+), 3 deletions(-) | |
80 | ||
81 | --- a/net/netfilter/nf_conntrack_extend.c | |
82 | +++ b/net/netfilter/nf_conntrack_extend.c | |
83 | @@ -53,7 +53,11 @@ nf_ct_ext_create(struct nf_ct_ext **ext, | |
84 | ||
85 | rcu_read_lock(); | |
86 | t = rcu_dereference(nf_ct_ext_types[id]); | |
87 | - BUG_ON(t == NULL); | |
88 | + if (!t) { | |
89 | + rcu_read_unlock(); | |
90 | + return NULL; | |
91 | + } | |
92 | + | |
93 | off = ALIGN(sizeof(struct nf_ct_ext), t->align); | |
94 | len = off + t->len + var_alloc_len; | |
95 | alloc_size = t->alloc_size + var_alloc_len; | |
96 | @@ -88,7 +92,10 @@ void *__nf_ct_ext_add_length(struct nf_c | |
97 | ||
98 | rcu_read_lock(); | |
99 | t = rcu_dereference(nf_ct_ext_types[id]); | |
100 | - BUG_ON(t == NULL); | |
101 | + if (!t) { | |
102 | + rcu_read_unlock(); | |
103 | + return NULL; | |
104 | + } | |
105 | ||
106 | newoff = ALIGN(old->len, t->align); | |
107 | newlen = newoff + t->len + var_alloc_len; | |
108 | @@ -175,6 +182,6 @@ void nf_ct_extend_unregister(struct nf_c | |
109 | RCU_INIT_POINTER(nf_ct_ext_types[type->id], NULL); | |
110 | update_alloc_size(type); | |
111 | mutex_unlock(&nf_ct_ext_type_mutex); | |
112 | - rcu_barrier(); /* Wait for completion of call_rcu()'s */ | |
113 | + synchronize_rcu(); | |
114 | } | |
115 | EXPORT_SYMBOL_GPL(nf_ct_extend_unregister); |