Markus Rechberger wrote:
> 2008/11/20 Németh Márton <[EMAIL PROTECTED]>:
>> Hi,
>>
>> I can trigger a NULL pointer reference with the em28xx-aad module.
>>
>> Steps to reproduce:
>> 1. Boot computer
>> 2. modprobe -k em28xx-aad
>> 3. Plug Pinnacle Hybrid Pro Stick (320e)
>> 4. rmmod em28xx-aad
>>
>> Current result: segmentation fault of rmmod and some error message in dmesg.
>> I
>> attached a patch which adds BUG_ON to the critical point at em28xx-aad.c
>>
>
> I know about that one, I had to write that driver quickly and modify
> tvtime to support it since there was a request for it.
> The correct way would be to do it like in the em28xx-audio driver, to
> initialize a waitqueue, and wait till the last user closes
> the aad node.
> Do you want to write a patch for it? (em28xx-audio, em28xx-video
> basically use the same system).
I am afraid the problem is something else here. I checked the value of
aad_users (see the attached modified debug patch). I found that when the
em28xx_aad_unregister() is called, the aad_users is already 0.
What the following log also shows is that the em28xx_aad_register() was
not called at all:
[ 483.484524] Linux video capture interface: v2.00
[ 483.712568] em28xx v4l2 driver version 0.0.1 loaded
[ 483.714391] usbcore: registered new interface driver em28xx
[ 483.809697] initializing Empia Audio Driver
[ 483.813074] Copyright (C) 2008 Empia Technology Inc
[ 483.813568] Copyright (C) 2008 Sundtek Ltd.
[ 483.813952] /usr/src/mcentral.de/em28xx-new/em28xx-aad.c:415:
em28xx_aad_init()
[ 488.158068] usb 3-3: new high speed USB device using ehci_hcd and address 3
[ 488.287246] usb 3-3: configuration #1 chosen from 1 choice
[ 488.289373] em28xx: new video device (eb1a:2881): interface 0, class 255
[ 488.289873] em28xx: device is attached to a USB 2.0 bus
[ 488.290587] em28xx #0: Alternate settings: 8
[ 488.290968] em28xx #0: Alternate setting 0, max size= 0
[ 488.291390] em28xx #0: Alternate setting 1, max size= 0
[ 488.291762] em28xx #0: Alternate setting 2, max size= 1448
[ 488.292174] em28xx #0: Alternate setting 3, max size= 2048
[ 488.292549] em28xx #0: Alternate setting 4, max size= 2304
[ 488.292920] em28xx #0: Alternate setting 5, max size= 2580
[ 488.293352] em28xx #0: Alternate setting 6, max size= 2892
[ 488.293726] em28xx #0: Alternate setting 7, max size= 3072
[ 488.538531] em28xx #0 at em28xx_gpio_control: <3>register disabled:
command=0x6, gpio_value=0x0
[ 488.774584] em28xx #0 at em28xx_gpio_control: <3>register disabled:
command=0xF, gpio_value=0x0
[ 489.519939] attach_inform: tvp5150 detected.
[ 489.566465] tvp5150 1-005c: tvp5150am1 detected.
[ 491.673027] successfully attached tuner
[ 491.690555] em28xx #0: V4L2 VBI device registered as /dev/vbi0
[ 491.711258] em28xx #0: V4L2 device registered as /dev/video0
[ 491.715275] input: em2880/em2870 remote control as /class/input/input12
[ 491.727701] em28xx-input.c: remote control handler attached
[ 491.728333] em28xx #0: Found Pinnacle Hybrid Pro
[ 491.729935] audio device (eb1a:2881): interface 1, class 1
[ 491.730730] audio device (eb1a:2881): interface 2, class 1
[ 492.351815] usbcore: registered new interface driver snd-usb-audio
[ 492.573043] em2880-dvb.c: DVB Init
[ 492.575202] em28xx #0 at em28xx_gpio_control: <3>register disabled:
command=0x6, gpio_value=0x0
[ 492.940138] DVB: registering new adapter (em2880 DVB-T)
[ 492.940844] DVB: registering frontend 0 (Zarlink ZL10353 DVB-T)...
[ 492.951119] Em28xx: Initialized (Em2880 DVB Extension) extension
[ 504.831317] releasing Empia Audio Driver
[ 504.831897] /usr/src/mcentral.de/em28xx-new/em28xx-aad.c:423:
em28xx_aad_exit()
[ 504.832398] /usr/src/mcentral.de/em28xx-new/em28xx-aad.c:378: aad_users=0
[ 504.832827] ------------[ cut here ]------------
[ 504.832837] kernel BUG at /usr/src/mcentral.de/em28xx-new/em28xx-aad.c:379!
[ 504.832845] invalid opcode: 0000 [#1] PREEMPT
[ 504.832855] Modules linked in: em28xx_dvb drx3973d s921 snd_usb_audio mt2060
lgdt3304 zl10353 lgdt330x snd_usb_lib dvb_core snd_hwdep qt1010 tuner_xc3028
tvp5150 em28xx_aad(-) em28xx videodev v4l1_compat ppdev lp cpufreq_ondemand
cpufreq_conservative ipv6 xt_tcpudp iptable_filter ip_tables x_tables
leds_clevo_mail led_class via via_agp drm agpgart eeprom snd_pcm_oss
snd_mixer_oss cpufreq_userspace cpufreq_powersave powernow_k8 fan usbhid
snd_via82xx
snd_mpu401_uart pcmcia firmware_class snd_via82xx_modem snd_seq_midi
snd_ac97_codec k8temp snd_seq_midi_event ac97_bus 8139too snd_rawmidi snd_pcm
yenta_socket
mii mousedev snd_seq 8250_pnp snd_timer rsrc_nonstatic hwmon snd_seq_device
ide_cd_mod 8250 i2c_viapro bitrev crc32 video cdrom snd ehci_hcd pcmcia_core
psmouse
backlight uhci_hcd serio_raw serial_core pcspkr soundcore snd_page_alloc
i2c_core usbcore parport_pc output battery parport ac thermal button processor
evdev
[ 504.833020]
[ 504.833020] Pid: 4608, comm: rmmod Not tainted (2.6.27.5 #2)
[ 504.833020] EIP: 0060:[<f8cdd0cb>] EFLAGS: 00210246 CPU: 0
[ 504.833020] EIP is at em28xx_aad_fini+0xab/0xb0 [em28xx_aad]
[ 504.833020] EAX: 00000050 EBX: 00000000 ECX: f69e8000 EDX: 00000000
[ 504.833020] ESI: f68ca000 EDI: 00000000 EBP: f69e9f20 ESP: f69e9f08
[ 504.833020] DS: 007b ES: 007b FS: 0000 GS: 0033 SS: 0068
[ 504.833020] Process rmmod (pid: 4608, ti=f69e8000 task=f5c10160
task.ti=f69e8000)
[ 504.833020] Stack: f8cddaa1 f8cdd928 0000017a 00000000 f68ca030 f8cde3a0
f69e9f38 f8eab2aa
[ 504.833020] f69e9f38 c0329a16 00000000 f8cde480 f69e9f4c f8cdd8d8
f8cdda84 f8cdd928
[ 504.833020] 000001a7 f69e9fb0 c014ca68 f8cde48c 38326d65 615f7878
f5006461 f69e9f9c
[ 504.833020] Call Trace:
[ 504.833020] [<f8eab2aa>] ? em28xx_unregister_extension+0x3a/0x90 [em28xx]
[ 504.833020] [<c0329a16>] ? printk+0x18/0x1a
[ 504.833020] [<f8cdd8d8>] ? em28xx_aad_exit+0x38/0x3d [em28xx_aad]
[ 504.833020] [<c014ca68>] ? sys_delete_module+0x158/0x220
[ 504.833020] [<c0175651>] ? do_munmap+0x1e1/0x240
[ 504.833020] [<c0233838>] ? trace_hardirqs_on_thunk+0xc/0x10
[ 504.833020] [<c0103309>] ? sysenter_do_call+0x12/0x31
[ 504.833020] =======================
[ 504.833020] Code: 04 89 10 89 d8 c7 43 44 00 01 10 00 c7 43 48 00 02 20 00
e8 28 5a 4a c7 31 c0 c7 86 b8 0d 00 00 00 00 00 00 83 c4 10 5b 5e 5d c3 <0f> 0b
eb
fe 90 55 89 e5 53 89 c3 8b 40 6c e8 d2 e2 c2 ff 8b 43
[ 504.833020] EIP: [<f8cdd0cb>] em28xx_aad_fini+0xab/0xb0 [em28xx_aad] SS:ESP
0068:f69e9f08
[ 504.833488] ---[ end trace cdcc012a82b9a0c9 ]---
So I guess that a waiting on aad_users==0 condition in em28xx_aad_unregister()
won't help here.
Regards,
Márton Németh
diff -r 3fe18e8981e5 em28xx-aad.c
--- a/em28xx-aad.c Mon Nov 17 15:35:18 2008 +0100
+++ b/em28xx-aad.c Thu Nov 20 01:52:38 2008 +0100
@@ -281,6 +281,7 @@
if (aad_users != 0)
return -EBUSY;
aad_users++;
+ printk(KERN_DEBUG "%s:%u: aad_open(): aad_users=%u\n", __FILE__, __LINE__, aad_users);
/* TODO: support multiple devices here */
list_for_each(list, &em28xx_aad_devlist) {
aad = list_entry(list, struct em28xx_aad_info, __aad_devlist);
@@ -314,6 +315,7 @@
struct em28xx_aad_info *aad;
aad = file->private_data;
aad_users--;
+ printk(KERN_DEBUG "%s:%u: aad_release(): aad_users=%u\n", __FILE__, __LINE__, aad_users);
aad_audio_isoc_deinit(aad);
return 0;
}
@@ -329,7 +331,10 @@
{
struct em28xx_aad_info *aad;
int id;
+
+ printk(KERN_DEBUG "%s:%u: em28xx_aad_register() enter\n", __FILE__, __LINE__);
aad = kzalloc(sizeof(struct em28xx_aad_info), GFP_KERNEL);
+ printk(KERN_DEBUG "%s:%u: aad=%p\n", __FILE__, __LINE__, aad);
id = find_first_zero_bit(&em28xx_aad_devices, sizeof(u32));
em28xx_aad_devices |= (1<<id);
aad->__id = id;
@@ -369,6 +374,9 @@
void em28xx_aad_unregister(struct em28xx_aad_info **aad_int)
{
struct em28xx_aad_info *aad = (*aad_int);
+
+ printk(KERN_DEBUG "%s:%u: aad_users=%u\n", __FILE__, __LINE__, aad_users);
+ BUG_ON(!aad);
em28xx_aad_devices &= ~(1<<aad->__id);
#if LINUX_VERSION_CODE < KERNEL_VERSION(2, 6, 26)
class_device_destroy(aad->__aad_class, MKDEV(aad->__aad_major, 0));
@@ -404,6 +412,7 @@
printk(KERN_INFO "Copyright (C) 2008 Empia Technology Inc\n");
printk(KERN_INFO "Copyright (C) 2008 Sundtek Ltd.\n");
+ printk(KERN_DEBUG "%s:%u: em28xx_aad_init()\n", __FILE__, __LINE__);
em28xx_maxdevs = sizeof(int);
return em28xx_register_extension(&audio_ops);
}
@@ -411,6 +420,7 @@
static void __exit em28xx_aad_exit(void)
{
printk(KERN_INFO "releasing Empia Audio Driver\n");
+ printk(KERN_DEBUG "%s:%u: em28xx_aad_exit()\n", __FILE__, __LINE__);
em28xx_unregister_extension(&audio_ops);
}
_______________________________________________
Em28xx mailing list
[email protected]
http://mcentral.de/mailman/listinfo/em28xx
