Re: panic on rmmod of nf_conntrack_irc

2009-04-15 Thread Patrick McHardy

Eric Dumazet wrote:

I should have used different fields names (from next, first, ...) to catch 
this
kind of errors at compile time :(

Something like :

diff --git a/include/linux/list_nulls.h b/include/linux/list_nulls.h


Eric, on second thought, could you please resubmit this to Dave?
It touches too many files outside of netfilter to go through my
tree.
--
To unsubscribe from this list: send the line unsubscribe kernel-testers in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: panic on rmmod of nf_conntrack_irc

2009-04-15 Thread Patrick McHardy

Eric Dumazet wrote:

Patrick McHardy a écrit :

Eric, on second thought, could you please resubmit this to Dave?
It touches too many files outside of netfilter to go through my
tree.


Sure, but its a cleanup (and was an RFC to show my point)
so should wait a litle bit until David opens his -next tree (for 2.6.31)


Fine with me :)


Please just submit your patch for 2.6.30 ?


OK, I'll do that.
--
To unsubscribe from this list: send the line unsubscribe kernel-testers in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: panic on rmmod of nf_conntrack_irc

2009-04-15 Thread David Miller
From: Eric Dumazet da...@cosmosbay.com
Date: Wed, 15 Apr 2009 12:42:27 +0200

 Sure, but its a cleanup (and was an RFC to show my point)
 so should wait a litle bit until David opens his -next tree (for 2.6.31)

The net-next-2.6 tree is open for business since 2.6.30-rc1 :-)

 Please just submit your patch for 2.6.30 ?

I'll let Patrick voice his opinion on this, I don't care either
way.
--
To unsubscribe from this list: send the line unsubscribe kernel-testers in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: panic on rmmod of nf_conntrack_irc

2009-04-14 Thread Mariusz Kozlowski
On Tue, 14 Apr 2009 22:14:20 +0200
Eric Dumazet da...@cosmosbay.com wrote:

 Mariusz Kozlowski a écrit :
  On Tue, 14 Apr 2009 14:16:37 +0200
  Patrick McHardy ka...@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 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 0008
  IP: [8048490a] 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:[8048490a]  [8048490a] 
  nf_log_unregister+0x2a/0x90
  RSP: 0018:88006a331ec8  EFLAGS: 00010207
  RAX:  RBX: a02d1d00 RCX: 
  RDX:  RSI: 0002 RDI: 8065bb80
  RBP: 88006a331ed8 R08: 0002 R09: 80676600
  R10:  R11:  R12: 
  R13: 0880 R14:  R15: 
  FS:  () GS:88000101(0063) knlGS:f7fc46b0
  CS:  0010 DS: 002b ES: 002b CR0: 8005003b
  CR2: 0008 CR3: 6f9f9000 CR4: 26a0
  DR0:  DR1:  DR2: 
  DR3:  DR6: 0ff0 DR7: 0400
  Process rmmod (pid: 10249, threadinfo 88006a33, task 
  88006f3c8000)
  Stack:
   a02d1e80 a02d1e80 88006a331ee8 a02d1518
   88006a331f78 8026ddbb 00676f6c5f746265 80229e77
   0282 88006a331f58 88006f3c8000 
  Call Trace:
   [a02d1518] ebt_log_fini+0x10/0x1e [ebt_log]
   [8026ddbb] sys_delete_module+0x18b/0x240
   [80229e77] ? do_page_fault+0x187/0x2a0
   [8022f08b] 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  [8048490a] nf_log_unregister+0x2a/0x90
   RSP 88006a331ec8
  CR2: 0008
  
  If this is not related or you don't have a clue I can bisect it.
  
 
 Thanks a lot for this report Mariusz
 
 Not related to my RCU patch IMHO (as it only touches conntrack)
 
 Check commit ca735b3aaa945626ba65a3e51145bfe4ecd9e222
 
 netfilter: use a linked list of loggers
 
 This patch modifies nf_log to use a linked list of loggers for each
 protocol. This list of loggers is read and write protected with a
 mutex.
 
 This patch separates registration and binding. To be used as
 logging module, a module has to register calling nf_log_register()
 and to bind to a protocol it has to call nf_log_bind_pf().
 This patch also converts the logging modules to the new API. For 
 nfnetlink_log,
 it simply switchs call to register functions to call to bind function and
 adds a call to nf_log_register() during init. For other modules, it just
 remove a const flag from the logger structure and replace it 

Re: panic on rmmod of nf_conntrack_irc

2009-04-14 Thread Eric Leblond
Hi,

Le mardi 14 avril 2009 à 22:14 +0200, Eric Dumazet a écrit :
 Mariusz Kozlowski a écrit :
  On Tue, 14 Apr 2009 14:16:37 +0200
  Patrick McHardy ka...@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 :)
 
...

 
 Check commit ca735b3aaa945626ba65a3e51145bfe4ecd9e222
 
 netfilter: use a linked list of loggers

...

 Signed-off-by: Eric Leblond e...@inl.fr
 Signed-off-by: Patrick McHardy ka...@trash.net
 
 It seems struct list_headlist[NFPROTO_NUMPROTO]; is not initialized 
 in struct nf_logger ?
 
 Please try following patch ?

I've just tested your patch. Without it, I was able to trigger the bug
(modprobe ebt_ulog ; rmmod ebt_ulog). All run cleanly with it.

All my apologies for having forgot to do this test.

BR,
-- 
Eric Leblond e...@inl.fr
INL: http://www.inl.fr/
NuFW: http://www.nufw.org/


signature.asc
Description: Ceci est une partie de message numériquement signée