panic on rmmod of nf_conntrack_irc

Message ID	49E47C2D.1050508@cosmosbay.com
State	Not Applicable, archived
Delegated to:	David Miller
Headers	show Return-Path: <netdev-owner@vger.kernel.org> Message-ID: <49E47C2D.1050508@cosmosbay.com> Date: Tue, 14 Apr 2009 14:06:05 +0200 From: Eric Dumazet <dada1@cosmosbay.com> User-Agent: Thunderbird 2.0.0.21 (Windows/20090302) MIME-Version: 1.0 To: Patrick McHardy <kaber@trash.net> CC: Mariusz Kozlowski <m.kozlowski@tuxland.pl>, Kernel Testers List <kernel-testers@vger.kernel.org>, "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>, Netfilter Development Mailinglist <netfilter-devel@vger.kernel.org>, Linux Netdev List <netdev@vger.kernel.org> Subject: Re: panic on rmmod of nf_conntrack_irc References: <20090410191736.21efab8c@mako-desktop> <49E4744D.5090205@trash.net> In-Reply-To: <49E4744D.5090205@trash.net> Content-Type: text/plain; charset=ISO-8859-15 Content-Transfer-Encoding: 8BIT Sender: netdev-owner@vger.kernel.org Precedence: bulk

On Tue, 14 Apr 2009 21:19:46 +0200 Mariusz Kozlowski <m.kozlowski@tuxland.pl> wrote: > On Tue, 14 Apr 2009 14:16:37 +0200 > Patrick McHardy <kaber@trash.net> wrote: > > > Eric Dumazet wrote: > > > Patrick McHardy a écrit : > > >> Mariusz Kozlowski wrote: > > >>> netfilter: nf_conntrack: use SLAB_DESTROY_BY_RCU and get rid of > > >>> call_rcu() > > >> Thanks for the report. Does this patch fix it? > > >> > > > > > > Hi Patrick, sorry for the delay, I was in holidays. > > > > No problem, me too :) > > > > > I should have used different fields names (from "next", "first", ...) to catch this > > > kind of errors at compile time :( > > > > > > Something like : > > > > Thanks Eric. I guess at this point it doesn't really matter anymore > > for the upstream kernel, but I'll apply your patch after getting > > confirmation from Mariusz to make sure that people maintaining > > external patches will notice the change (and won't send me broken > > patches :)). > > Ok good. It doesn't panic now on nf_conntrack_irc rmmod So feel free to add my rb and tb lines to it :) > but I found another bug. > Now it's NULL pointer dereference I when doing rmmod of ebt_log. To reproduce simply: > > # modprobe ebt_log && rmmod ebt_log > > This is from the mainline kernel + your (Erics) patch: > > BUG: unable to handle kernel NULL pointer dereference at 0000000000000008 > IP: [<ffffffff8048490a>] nf_log_unregister+0x2a/0x90 > PGD 6f0e8067 PUD 6a2e0067 PMD 0 > Oops: 0002 [#1] PREEMPT SMP > last sysfs file: /sys/devices/LNXSYSTM:00/device:00/PNP0A08:00/device:01/PNP0C09:00/PNP0C0A:00/power_supply/BAT0/energy_full > CPU 0 > Modules linked in: ebt_log(-) nfs lockd auth_rpcgss sunrpc netconsole af_packet i915 drm i2c_algo_bit i2c_core kvm_intel kvm ppdev acpi_cpufreq cpufreq_stats cpufreq_powersave cpufreq_userspace cpufreq_conservative cpufreq_ondemand freq_table fan sbs sbshc container pci_slot iptable_filter ip_tables x_tables sbp2 lp fuse snd_hda_codec_analog arc4 ecb snd_hda_intel snd_hda_codec snd_pcm_oss snd_mixer_oss iwl3945 iwlcore thinkpad_acpi sr_mod cdrom pcmcia mac80211 snd_pcm rfkill led_class snd_seq_dummy snd_seq_oss snd_seq_midi_event evdev thermal uhci_hcd ehci_hcd parport_pc ohci1394 nvram yenta_socket rsrc_nonstatic pcmcia_core snd_seq pcspkr psmouse serio_raw usbcore cfg80211 e1000e ieee1394 sg parport battery ac button processor snd_timer snd_seq_device intel_agp snd soundcore snd_page_alloc > Pid: 10249, comm: rmmod Not tainted 2.6.30-rc1-00204-gb0cbc86-dirty #33 7667Y24 > RIP: 0010:[<ffffffff8048490a>] [<ffffffff8048490a>] nf_log_unregister+0x2a/0x90 > RSP: 0018:ffff88006a331ec8 EFLAGS: 00010207 > RAX: 0000000000000000 RBX: ffffffffa02d1d00 RCX: 0000000000000000 > RDX: 0000000000000000 RSI: 0000000000000002 RDI: ffffffff8065bb80 > RBP: ffff88006a331ed8 R08: 0000000000000002 R09: ffffffff80676600 > R10: 0000000000000000 R11: 0000000000000000 R12: 0000000000000000 > R13: 0000000000000880 R14: 0000000000000000 R15: 0000000000000000 > FS: 0000000000000000(0000) GS:ffff880001010000(0063) knlGS:00000000f7fc46b0 > CS: 0010 DS: 002b ES: 002b CR0: 000000008005003b > CR2: 0000000000000008 CR3: 000000006f9f9000 CR4: 00000000000026a0 > DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000 > DR3: 0000000000000000 DR6: 00000000ffff0ff0 DR7: 0000000000000400 > Process rmmod (pid: 10249, threadinfo ffff88006a330000, task ffff88006f3c8000) > Stack: > ffffffffa02d1e80 ffffffffa02d1e80 ffff88006a331ee8 ffffffffa02d1518 > ffff88006a331f78 ffffffff8026ddbb 00676f6c5f746265 ffffffff80229e77 > 0000000000000282 ffff88006a331f58 ffff88006f3c8000 0000000000000000 > Call Trace: > [<ffffffffa02d1518>] ebt_log_fini+0x10/0x1e [ebt_log] > [<ffffffff8026ddbb>] sys_delete_module+0x18b/0x240 > [<ffffffff80229e77>] ? do_page_fault+0x187/0x2a0 > [<ffffffff8022f08b>] sysenter_dispatch+0x7/0x32 > Code: 90 55 48 89 e5 53 48 89 fb 48 83 ec 08 48 c7 c7 80 bb 65 80 e8 78 a9 06 00 31 c9 eb 31 0f 1f 40 00 48 8b 54 4b 18 48 8b 44 4b 20 <48> 89 42 08 48 89 10 48 c7 44 4b 20 00 02 20 00 48 c7 44 4b 18 > RIP [<ffffffff8048490a>] nf_log_unregister+0x2a/0x90 > RSP <ffff88006a331ec8> > CR2: 0000000000000008 > > If this is not related or you don't have a clue I can bisect it. The same happens for ebt_ulog on clean latest mainline (as of 6f66cbc6) so just for reference: BUG: unable to handle kernel NULL pointer dereference at 0000000000000008 IP: [<ffffffff80484b4a>] nf_log_unregister+0x2a/0x90 PGD 6f0ff067 PUD 6fb1a067 PMD 0 Oops: 0002 [#1] PREEMPT SMP last sysfs file: /sys/devices/LNXSYSTM:00/device:00/PNP0A08:00/device:01/PNP0C09:00/PNP0C0A:00/power_supply/BAT0/energy_full CPU 1 Modules linked in: ebt_ulog(-) netconsole af_packet i915 drm i2c_algo_bit i2c_core kvm_intel kvm ppdev acpi_cpufreq cpufreq_stats cpufreq_powersave cpufreq_userspace cpufreq_conservative cpufreq_ondemand freq_table fan sbs sbshc container pci_slot iptable_filter ip_tables x_tables sbp2 lp fuse snd_hda_codec_analog snd_hda_intel snd_hda_codec snd_pcm_oss snd_mixer_oss snd_pcm snd_seq_dummy arc4 snd_seq_oss ecb snd_seq_midi_event thinkpad_acpi pcmcia snd_seq rfkill sr_mod cdrom iwl3945 iwlcore mac80211 snd_timer snd_seq_device ehci_hcd uhci_hcd ohci1394 pcspkr led_class yenta_socket rsrc_nonstatic evdev parport_pc serio_raw psmouse ieee1394 sg nvram pcmcia_core parport ac battery cfg80211 processor thermal button e1000e usbcore snd intel_agp soundcore snd_page_alloc Pid: 4986, comm: rmmod Not tainted 2.6.30-rc1-00501-g6f66cbc #34 7667Y24 RIP: 0010:[<ffffffff80484b4a>] [<ffffffff80484b4a>] nf_log_unregister+0x2a/0x90 RSP: 0018:ffff88006f91deb8 EFLAGS: 00010203 RAX: 0000000000000000 RBX: ffffffffa038d580 RCX: 0000000000000000 RDX: 0000000000000000 RSI: 0000000000000002 RDI: ffffffff8065cc40 RBP: ffff88006f91dec8 R08: 0000000000000002 R09: ffffffff80677600 R10: 0000000000000000 R11: 0000000000000000 R12: 0000000000000000 R13: 0000000000000880 R14: 0000000000000000 R15: 0000000000000000 FS: 0000000000000000(0000) GS:ffff880001029000(0063) knlGS:00000000f7f0b6b0 CS: 0010 DS: 002b ES: 002b CR0: 000000008005003b CR2: 0000000000000008 CR3: 000000006f0ab000 CR4: 00000000000026a0 DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000 DR3: 0000000000000000 DR6: 00000000ffff0ff0 DR7: 0000000000000400 Process rmmod (pid: 4986, threadinfo ffff88006f91c000, task ffff88006f1241a0) Stack: ffff88006f91dee8 ffffffffa038d920 ffff88006f91dee8 ffffffffa038c6ca ffffffffa038d700 0000000000000000 ffff88006f91df78 ffffffff8026deab 676f6c755f746265 ffffffff80229e00 0000000000000282 ffff88006f91df58 Call Trace: [<ffffffffa038c6ca>] ebt_ulog_fini+0x1a/0x7e [ebt_ulog] [<ffffffff8026deab>] sys_delete_module+0x18b/0x240 [<ffffffff80229e00>] ? do_page_fault+0xd0/0x2a0 [<ffffffff8022f12b>] sysenter_dispatch+0x7/0x32 Code: 90 55 48 89 e5 53 48 89 fb 48 83 ec 08 48 c7 c7 40 cc 65 80 e8 88 a9 06 00 31 c9 eb 31 0f 1f 40 00 48 8b 54 4b 18 48 8b 44 4b 20 <48> 89 42 08 48 89 10 48 c7 44 4b 20 00 02 20 00 48 c7 44 4b 18 RIP [<ffffffff80484b4a>] nf_log_unregister+0x2a/0x90 RSP <ffff88006f91deb8> CR2: 0000000000000008 (adding Bart and ebtables-devel to cc) Mariusz -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html

diff --git a/include/linux/list_nulls.h b/include/linux/list_nulls.h index 93150ec..b66c553 100644 --- a/include/linux/list_nulls.h +++ b/include/linux/list_nulls.h @@ -15,14 +15,14 @@ */ struct hlist_nulls_head { - struct hlist_nulls_node *first; + struct hlist_nulls_node *nfirst; }; struct hlist_nulls_node { - struct hlist_nulls_node *next, **pprev; + struct hlist_nulls_node *nnext, **npprev; }; #define INIT_HLIST_NULLS_HEAD(ptr, nulls) \ - ((ptr)->first = (struct hlist_nulls_node *) (1UL | (((long)nulls) << 1))) + ((ptr)->nfirst = (struct hlist_nulls_node *) (1UL | (((long)nulls) << 1))) #define hlist_nulls_entry(ptr, type, member) container_of(ptr,type,member) /** @@ -48,21 +48,21 @@ static inline unsigned long get_nulls_value(const struct hlist_nulls_node *ptr) static inline int hlist_nulls_unhashed(const struct hlist_nulls_node *h) { - return !h->pprev; + return !h->npprev; } static inline int hlist_nulls_empty(const struct hlist_nulls_head *h) { - return is_a_nulls(h->first); + return is_a_nulls(h->nfirst); } static inline void __hlist_nulls_del(struct hlist_nulls_node *n) { - struct hlist_nulls_node *next = n->next; - struct hlist_nulls_node **pprev = n->pprev; + struct hlist_nulls_node *next = n->nnext; + struct hlist_nulls_node **pprev = n->npprev; *pprev = next; if (!is_a_nulls(next)) - next->pprev = pprev; + next->npprev = pprev; } /** @@ -74,10 +74,10 @@ static inline void __hlist_nulls_del(struct hlist_nulls_node *n) * */ #define hlist_nulls_for_each_entry(tpos, pos, head, member) \ - for (pos = (head)->first; \ + for (pos = (head)->nfirst; \ (!is_a_nulls(pos)) && \ ({ tpos = hlist_nulls_entry(pos, typeof(*tpos), member); 1;}); \ - pos = pos->next) + pos = pos->nnext) /** * hlist_nulls_for_each_entry_from - iterate over a hlist continuing from current point @@ -89,6 +89,6 @@ static inline void __hlist_nulls_del(struct hlist_nulls_node *n) #define hlist_nulls_for_each_entry_from(tpos, pos, member) \ for (; (!is_a_nulls(pos)) && \ ({ tpos = hlist_nulls_entry(pos, typeof(*tpos), member); 1;}); \ - pos = pos->next) + pos = pos->nnext) #endif diff --git a/include/linux/rculist_nulls.h b/include/linux/rculist_nulls.h index f9ddd03..2378c7c 100644 --- a/include/linux/rculist_nulls.h +++ b/include/linux/rculist_nulls.h @@ -33,7 +33,7 @@ static inline void hlist_nulls_del_init_rcu(struct hlist_nulls_node *n) { if (!hlist_nulls_unhashed(n)) { __hlist_nulls_del(n); - n->pprev = NULL; + n->npprev = NULL; } } @@ -59,7 +59,7 @@ static inline void hlist_nulls_del_init_rcu(struct hlist_nulls_node *n) static inline void hlist_nulls_del_rcu(struct hlist_nulls_node *n) { __hlist_nulls_del(n); - n->pprev = LIST_POISON2; + n->npprev = LIST_POISON2; } /** @@ -84,13 +84,13 @@ static inline void hlist_nulls_del_rcu(struct hlist_nulls_node *n) static inline void hlist_nulls_add_head_rcu(struct hlist_nulls_node *n, struct hlist_nulls_head *h) { - struct hlist_nulls_node *first = h->first; + struct hlist_nulls_node *first = h->nfirst; - n->next = first; - n->pprev = &h->first; - rcu_assign_pointer(h->first, n); + n->nnext = first; + n->npprev = &h->nfirst; + rcu_assign_pointer(h->nfirst, n); if (!is_a_nulls(first)) - first->pprev = &n->next; + first->npprev = &n->nnext; } /** * hlist_nulls_for_each_entry_rcu - iterate over rcu list of given type @@ -101,10 +101,10 @@ static inline void hlist_nulls_add_head_rcu(struct hlist_nulls_node *n, * */ #define hlist_nulls_for_each_entry_rcu(tpos, pos, head, member) \ - for (pos = rcu_dereference((head)->first); \ + for (pos = rcu_dereference((head)->nfirst); \ (!is_a_nulls(pos)) && \ ({ tpos = hlist_nulls_entry(pos, typeof(*tpos), member); 1; }); \ - pos = rcu_dereference(pos->next)) + pos = rcu_dereference(pos->nnext)) #endif #endif diff --git a/include/net/sock.h b/include/net/sock.h index 4bb1ff9..5d94186 100644 --- a/include/net/sock.h +++ b/include/net/sock.h @@ -309,7 +309,7 @@ static inline struct sock *sk_head(const struct hlist_head *head) static inline struct sock *__sk_nulls_head(const struct hlist_nulls_head *head) { - return hlist_nulls_entry(head->first, struct sock, sk_nulls_node); + return hlist_nulls_entry(head->nfirst, struct sock, sk_nulls_node); } static inline struct sock *sk_nulls_head(const struct hlist_nulls_head *head) @@ -325,8 +325,8 @@ static inline struct sock *sk_next(const struct sock *sk) static inline struct sock *sk_nulls_next(const struct sock *sk) { - return (!is_a_nulls(sk->sk_nulls_node.next)) ? - hlist_nulls_entry(sk->sk_nulls_node.next, + return (!is_a_nulls(sk->sk_nulls_node.nnext)) ? + hlist_nulls_entry(sk->sk_nulls_node.nnext, struct sock, sk_nulls_node) : NULL; } @@ -348,7 +348,7 @@ static __inline__ void sk_node_init(struct hlist_node *node) static __inline__ void sk_nulls_node_init(struct hlist_nulls_node *node) { - node->pprev = NULL; + node->npprev = NULL; } static __inline__ void __sk_del_node(struct sock *sk) diff --git a/net/ipv4/netfilter/nf_conntrack_l3proto_ipv4_compat.c b/net/ipv4/netfilter/nf_conntrack_l3proto_ipv4_compat.c index 8668a3d..0bb6059 100644 --- a/net/ipv4/netfilter/nf_conntrack_l3proto_ipv4_compat.c +++ b/net/ipv4/netfilter/nf_conntrack_l3proto_ipv4_compat.c @@ -34,7 +34,7 @@ static struct hlist_nulls_node *ct_get_first(struct seq_file *seq) for (st->bucket = 0; st->bucket < nf_conntrack_htable_size; st->bucket++) { - n = rcu_dereference(net->ct.hash[st->bucket].first); + n = rcu_dereference(net->ct.hash[st->bucket].nfirst); if (!is_a_nulls(n)) return n; } @@ -47,13 +47,13 @@ static struct hlist_nulls_node *ct_get_next(struct seq_file *seq, struct net *net = seq_file_net(seq); struct ct_iter_state *st = seq->private; - head = rcu_dereference(head->next); + head = rcu_dereference(head->nnext); while (is_a_nulls(head)) { if (likely(get_nulls_value(head) == st->bucket)) { if (++st->bucket >= nf_conntrack_htable_size) return NULL; } - head = rcu_dereference(net->ct.hash[st->bucket].first); + head = rcu_dereference(net->ct.hash[st->bucket].nfirst); } return head; } diff --git a/net/ipv4/tcp_ipv4.c b/net/ipv4/tcp_ipv4.c index 5d427f8..1219f2d 100644 --- a/net/ipv4/tcp_ipv4.c +++ b/net/ipv4/tcp_ipv4.c @@ -1851,13 +1851,13 @@ EXPORT_SYMBOL(tcp_v4_destroy_sock); static inline struct inet_timewait_sock *tw_head(struct hlist_nulls_head *head) { return hlist_nulls_empty(head) ? NULL : - list_entry(head->first, struct inet_timewait_sock, tw_node); + list_entry(head->nfirst, struct inet_timewait_sock, tw_node); } static inline struct inet_timewait_sock *tw_next(struct inet_timewait_sock *tw) { - return !is_a_nulls(tw->tw_node.next) ? - hlist_nulls_entry(tw->tw_node.next, typeof(*tw), tw_node) : NULL; + return !is_a_nulls(tw->tw_node.nnext) ? + hlist_nulls_entry(tw->tw_node.nnext, typeof(*tw), tw_node) : NULL; } static void *listening_get_next(struct seq_file *seq, void *cur) diff --git a/net/netfilter/nf_conntrack_core.c b/net/netfilter/nf_conntrack_core.c index 8020db6..56f9dc3 100644 --- a/net/netfilter/nf_conntrack_core.c +++ b/net/netfilter/nf_conntrack_core.c @@ -1140,7 +1140,7 @@ int nf_conntrack_set_hashsize(const char *val, struct kernel_param *kp) spin_lock_bh(&nf_conntrack_lock); for (i = 0; i < nf_conntrack_htable_size; i++) { while (!hlist_nulls_empty(&init_net.ct.hash[i])) { - h = hlist_nulls_entry(init_net.ct.hash[i].first, + h = hlist_nulls_entry(init_net.ct.hash[i].nfirst, struct nf_conntrack_tuple_hash, hnnode); hlist_nulls_del_rcu(&h->hnnode); bucket = __hash_conntrack(&h->tuple, hashsize, rnd); diff --git a/net/netfilter/nf_conntrack_helper.c b/net/netfilter/nf_conntrack_helper.c index 30b8e90..0fa5a42 100644 --- a/net/netfilter/nf_conntrack_helper.c +++ b/net/netfilter/nf_conntrack_helper.c @@ -176,7 +176,7 @@ static void __nf_conntrack_helper_unregister(struct nf_conntrack_helper *me, } /* Get rid of expecteds, set helpers to NULL. */ - hlist_for_each_entry(h, nn, &net->ct.unconfirmed, hnnode) + hlist_nulls_for_each_entry(h, nn, &net->ct.unconfirmed, hnnode) unhelp(h, me); for (i = 0; i < nf_conntrack_htable_size; i++) { hlist_nulls_for_each_entry(h, nn, &net->ct.hash[i], hnnode) diff --git a/net/netfilter/nf_conntrack_standalone.c b/net/netfilter/nf_conntrack_standalone.c index 1935153..e6881db 100644 --- a/net/netfilter/nf_conntrack_standalone.c +++ b/net/netfilter/nf_conntrack_standalone.c @@ -53,7 +53,7 @@ static struct hlist_nulls_node *ct_get_first(struct seq_file *seq) for (st->bucket = 0; st->bucket < nf_conntrack_htable_size; st->bucket++) { - n = rcu_dereference(net->ct.hash[st->bucket].first); + n = rcu_dereference(net->ct.hash[st->bucket].nfirst); if (!is_a_nulls(n)) return n; } @@ -66,13 +66,13 @@ static struct hlist_nulls_node *ct_get_next(struct seq_file *seq, struct net *net = seq_file_net(seq); struct ct_iter_state *st = seq->private; - head = rcu_dereference(head->next); + head = rcu_dereference(head->nnext); while (is_a_nulls(head)) { if (likely(get_nulls_value(head) == st->bucket)) { if (++st->bucket >= nf_conntrack_htable_size) return NULL; } - head = rcu_dereference(net->ct.hash[st->bucket].first); + head = rcu_dereference(net->ct.hash[st->bucket].nfirst); } return head; }

panic on rmmod of nf_conntrack_irc

Commit Message

Comments

Patch