[2.6.16] saa7134 disable_ir oops

[2.6.16] saa7134 disable_ir oops

[David Liontooth]

On 2.6.16 mainline, I get an oops when inserting the saa7134 module with 
the option "disable_ir=1", whether for a single card or several.

Gigabyte K8NS Ultra-939, LifeView FlyVideo-3000FM NTSC, athlon64 x2 
3800+, Debian sid. All four cards work great otherwise. The new 
saa7134-alsa does not appear to be involved in the oops.

Issued:

sudo modprobe saa7134 card=2,2,2,2 tuner=43,43,43,43 video_nr=1,2,3,4
vbi_nr=1,2,3,4 radio_nr=1,2,3,4 alsa=1,1,1,1 disable_ir=1,1,1,1
sudo modprobe saa7134-alsa index=1,2,3,4

Got in dmesg:

Unable to handle kernel NULL pointer dereference at 0000000000000260 RIP:
<ffffffff880d5a95>{:saa7134:saa7134_initdev+1452}
PGD 3e607067 PUD 3b0b6067 PMD 0
Oops: 0000 [1] SMP
CPU 0
Modules linked in: saa7134 video_buf compat_ioctl32 v4l2_common
v4l1_compat ir_kbd_i2c ir_common videodev xfs exportfs snd_intel8x0
snd_ac97_codec snd_ac97_bus snd_pcm snd_timer snd soundcore sk98lin
snd_page_alloc psmouse pcspkr evdev parport_pc parport ehci_hcd
i2c_nforce2 ohci_hcd i2c_core unix
Pid: 7134, comm: modprobe Not tainted 2.6.16 #1
RIP: 0010:[<ffffffff880d5a95>]
<ffffffff880d5a95>{:saa7134:saa7134_initdev+1452}
RSP: 0018:ffff810036fd3de8  EFLAGS: 00010246
RAX: 0000000000000000 RBX: ffff81003cb84000 RCX: 000000000000083f
RDX: 0000000000000001 RSI: 0000000000000000 RDI: ffff81003cb84000
RBP: 0000000000000000 R08: ffff81003cb84aa8 R09: ffffffff880ee496
R10: ffff810008030000 R11: ffff810008030000 R12: ffff81003cb84158
R13: ffff81003ff12000 R14: 0000000000000000 R15: 0000000000000003
FS:  00002b96383df6d0(0000) GS:ffffffff803ee000(0000) knlGS:0000000000000000
CS:  0010 DS: 0000 ES: 0000 CR0: 000000008005003b
CR2: 0000000000000260 CR3: 000000003d86d000 CR4: 00000000000006e0
Process modprobe (pid: 7134, threadinfo ffff810036fd2000, task
ffff810001f149f0)
Stack: ffff81003ff12070 0000000100000001 ffff81003ff12070 ffffffff880e8460
       ffff81003ff12000 ffffffff880ee7c0 0000000000000000 ffffffff801d860c
       ffffffff8036c6c8 ffff81003ff12070
Call Trace: <ffffffff801d860c>{pci_device_probe+221}
       <ffffffff8022db0d>{driver_probe_device+82}
<ffffffff8022dbc2>{__driver_attach+0}
       <ffffffff8022dc18>{__driver_attach+86}
<ffffffff8022d142>{bus_for_each_dev+67}
       <ffffffff8022d442>{bus_add_driver+116}
<ffffffff801d8182>{__pci_register_driver+128}
       <ffffffff80141655>{sys_init_module+240}
<ffffffff8010a82a>{system_call+126}

Code: 8b 96 60 02 00 00 89 d0 25 00 00 01 00 85 c0 75 20 81 e2 00
RIP <ffffffff880d5a95>{:saa7134:saa7134_initdev+1452} RSP <ffff810036fd3de8>
CR2: 0000000000000260

Dave

--
video4linux-list mailing list
Unsubscribe mailto:video4linux-list-request@redhat.com?subject=unsubscribe
https://www.redhat.com/mailman/listinfo/video4linux-list

Re: [2.6.16] saa7134 disable_ir oops

[Sergey Vlasov]

[-- Attachment #1: Type: text/plain, Size: 2886 bytes --]

On Fri, 24 Mar 2006 14:00:46 -0800 David Liontooth wrote:

> On 2.6.16 mainline, I get an oops when inserting the saa7134 module with 
> the option "disable_ir=1", whether for a single card or several.
> 
> Gigabyte K8NS Ultra-939, LifeView FlyVideo-3000FM NTSC, athlon64 x2 
> 3800+, Debian sid. All four cards work great otherwise. The new 
> saa7134-alsa does not appear to be involved in the oops.
> 
> Issued:
> 
> sudo modprobe saa7134 card=2,2,2,2 tuner=43,43,43,43 video_nr=1,2,3,4
> vbi_nr=1,2,3,4 radio_nr=1,2,3,4 alsa=1,1,1,1 disable_ir=1,1,1,1
> sudo modprobe saa7134-alsa index=1,2,3,4
> 
> Got in dmesg:
> 
> Unable to handle kernel NULL pointer dereference at 0000000000000260 RIP:
> <ffffffff880d5a95>{:saa7134:saa7134_initdev+1452}
> PGD 3e607067 PUD 3b0b6067 PMD 0
> Oops: 0000 [1] SMP
> CPU 0
> Modules linked in: saa7134 video_buf compat_ioctl32 v4l2_common
> v4l1_compat ir_kbd_i2c ir_common videodev xfs exportfs snd_intel8x0
> snd_ac97_codec snd_ac97_bus snd_pcm snd_timer snd soundcore sk98lin
> snd_page_alloc psmouse pcspkr evdev parport_pc parport ehci_hcd
> i2c_nforce2 ohci_hcd i2c_core unix
> Pid: 7134, comm: modprobe Not tainted 2.6.16 #1
> RIP: 0010:[<ffffffff880d5a95>]
> <ffffffff880d5a95>{:saa7134:saa7134_initdev+1452}
[skip]

Other people algo get oops with disable_ir=1:

http://lkml.org/lkml/2006/2/20/122

Does the following patch fix things?

---

saa7134: Fix oops with disable_ir=1

When disable_ir=1 parameter is used, or when saa7134_input_init1()
fails for any other reason, dev->remote will remain NULL, and the
driver will oops in saa7134_hwinit2().  Therefore dev->remote must be
checked before dereferencing.

Signed-off-by: Sergey Vlasov <vsu@altlinux.ru>

--- linux-2.6.16.orig/drivers/media/video/saa7134/saa7134-core.c	2006-03-20 08:53:29 +0300
+++ linux-2.6.16/drivers/media/video/saa7134/saa7134-core.c	2006-04-06 20:00:56 +0400
@@ -543,6 +543,8 @@ static irqreturn_t saa7134_irq(int irq, 
 		if (report & SAA7134_IRQ_REPORT_GPIO16) {
 			switch (dev->has_remote) {
 				case SAA7134_REMOTE_GPIO:
+					if (!dev->remote)
+						break;
 					if  (dev->remote->mask_keydown & 0x10000) {
 						saa7134_input_irq(dev);
 					}
@@ -559,6 +561,8 @@ static irqreturn_t saa7134_irq(int irq, 
 		if (report & SAA7134_IRQ_REPORT_GPIO18) {
 			switch (dev->has_remote) {
 				case SAA7134_REMOTE_GPIO:
+					if (!dev->remote)
+						break;
 					if ((dev->remote->mask_keydown & 0x40000) ||
 					    (dev->remote->mask_keyup & 0x40000)) {
 						saa7134_input_irq(dev);
@@ -671,7 +675,7 @@ static int saa7134_hwinit2(struct saa713
 		SAA7134_IRQ2_INTE_PE      |
 		SAA7134_IRQ2_INTE_AR;
 
-	if (dev->has_remote == SAA7134_REMOTE_GPIO) {
+	if (dev->has_remote == SAA7134_REMOTE_GPIO && dev->remote) {
 		if (dev->remote->mask_keydown & 0x10000)
 			irq2_mask |= SAA7134_IRQ2_INTE_GPIO16;
 		else if (dev->remote->mask_keydown & 0x40000)



[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

Re: [2.6.16] saa7134 disable_ir oops

[David Liontooth]

Sergey Vlasov wrote:
> On Fri, 24 Mar 2006 14:00:46 -0800 David Liontooth wrote:
>
>   
>> On 2.6.16 mainline, I get an oops when inserting the saa7134 module with 
>> the option "disable_ir=1", whether for a single card or several.
>>
>> Gigabyte K8NS Ultra-939, LifeView FlyVideo-3000FM NTSC, athlon64 x2 
>> 3800+, Debian sid. All four cards work great otherwise. The new 
>> saa7134-alsa does not appear to be involved in the oops.
>>
>> Issued:
>>
>> sudo modprobe saa7134 card=2,2,2,2 tuner=43,43,43,43 video_nr=1,2,3,4
>> vbi_nr=1,2,3,4 radio_nr=1,2,3,4 alsa=1,1,1,1 disable_ir=1,1,1,1
>> sudo modprobe saa7134-alsa index=1,2,3,4
>>
>> Got in dmesg:
>>
>> Unable to handle kernel NULL pointer dereference at 0000000000000260 RIP:
>> <ffffffff880d5a95>{:saa7134:saa7134_initdev+1452}
>> PGD 3e607067 PUD 3b0b6067 PMD 0
>> Oops: 0000 [1] SMP
>> CPU 0
>> Modules linked in: saa7134 video_buf compat_ioctl32 v4l2_common
>> v4l1_compat ir_kbd_i2c ir_common videodev xfs exportfs snd_intel8x0
>> snd_ac97_codec snd_ac97_bus snd_pcm snd_timer snd soundcore sk98lin
>> snd_page_alloc psmouse pcspkr evdev parport_pc parport ehci_hcd
>> i2c_nforce2 ohci_hcd i2c_core unix
>> Pid: 7134, comm: modprobe Not tainted 2.6.16 #1
>> RIP: 0010:[<ffffffff880d5a95>]
>> <ffffffff880d5a95>{:saa7134:saa7134_initdev+1452}
>>     
> [skip]
>
> Other people algo get oops with disable_ir=1:
>
> http://lkml.org/lkml/2006/2/20/122
>
> Does the following patch fix things?
>   
The patch fixes the oops -- thank you! Yet the ir modules get loaded in 
spite of the "disable_ir=1" parameter:

# sudo modprobe -vvv saa7134 card=2 tuner=43 disable_ir=1
insmod /lib/modules/2.6.16/kernel/drivers/media/common/ir-common.ko
insmod /lib/modules/2.6.16/kernel/drivers/media/video/ir-kbd-i2c.ko
insmod /lib/modules/2.6.16/kernel/drivers/media/video/video-buf.ko
insmod /lib/modules/2.6.16/kernel/drivers/media/video/saa7134/saa7134.ko 
card=2 tuner=43 disable_ir=1

David
> ---
>
> saa7134: Fix oops with disable_ir=1
>
> When disable_ir=1 parameter is used, or when saa7134_input_init1()
> fails for any other reason, dev->remote will remain NULL, and the
> driver will oops in saa7134_hwinit2().  Therefore dev->remote must be
> checked before dereferencing.
>
> Signed-off-by: Sergey Vlasov <vsu@altlinux.ru>
>
> --- linux-2.6.16.orig/drivers/media/video/saa7134/saa7134-core.c	2006-03-20 08:53:29 +0300
> +++ linux-2.6.16/drivers/media/video/saa7134/saa7134-core.c	2006-04-06 20:00:56 +0400
> @@ -543,6 +543,8 @@ static irqreturn_t saa7134_irq(int irq, 
>  		if (report & SAA7134_IRQ_REPORT_GPIO16) {
>  			switch (dev->has_remote) {
>  				case SAA7134_REMOTE_GPIO:
> +					if (!dev->remote)
> +						break;
>  					if  (dev->remote->mask_keydown & 0x10000) {
>  						saa7134_input_irq(dev);
>  					}
> @@ -559,6 +561,8 @@ static irqreturn_t saa7134_irq(int irq, 
>  		if (report & SAA7134_IRQ_REPORT_GPIO18) {
>  			switch (dev->has_remote) {
>  				case SAA7134_REMOTE_GPIO:
> +					if (!dev->remote)
> +						break;
>  					if ((dev->remote->mask_keydown & 0x40000) ||
>  					    (dev->remote->mask_keyup & 0x40000)) {
>  						saa7134_input_irq(dev);
> @@ -671,7 +675,7 @@ static int saa7134_hwinit2(struct saa713
>  		SAA7134_IRQ2_INTE_PE      |
>  		SAA7134_IRQ2_INTE_AR;
>  
> -	if (dev->has_remote == SAA7134_REMOTE_GPIO) {
> +	if (dev->has_remote == SAA7134_REMOTE_GPIO && dev->remote) {
>  		if (dev->remote->mask_keydown & 0x10000)
>  			irq2_mask |= SAA7134_IRQ2_INTE_GPIO16;
>  		else if (dev->remote->mask_keydown & 0x40000)
>
>
>   

Re: [2.6.16] saa7134 disable_ir oops

[Mauro Carvalho Chehab]

Em Qui, 2006-04-06 às 20:20 +0400, Sergey Vlasov escreveu:
> On Fri, 24 Mar 2006 14:00:46 -0800 David Liontooth wrote:

> Does the following patch fix things?
> 
Applied at v4l-dvb tree. Thanks.

Cheers, 
Mauro.

Re: [2.6.16] saa7134 disable_ir oops

[Sergey Vlasov]

[-- Attachment #1: Type: text/plain, Size: 2026 bytes --]

On Fri, Apr 07, 2006 at 10:16:10AM -0300, Mauro Carvalho Chehab wrote:
> Em Qui, 2006-04-06 ?s 20:20 +0400, Sergey Vlasov escreveu:
> > On Fri, 24 Mar 2006 14:00:46 -0800 David Liontooth wrote:
> 
> > Does the following patch fix things?
> > 
> Applied at v4l-dvb tree. Thanks.

IMHO this patch should also be added to 2.6.16-stable - it fixes oops in
configurations which worked fine with older kernels.

-----------------------------------------------------------------------

saa7134: Fix oops with disable_ir=1

When disable_ir=1 parameter is used, or when saa7134_input_init1()
fails for any other reason, dev->remote will remain NULL, and the
driver will oops in saa7134_hwinit2().  Therefore dev->remote must be
checked before dereferencing.

Signed-off-by: Sergey Vlasov <vsu@altlinux.ru>

--- linux-2.6.16.orig/drivers/media/video/saa7134/saa7134-core.c	2006-03-20 08:53:29 +0300
+++ linux-2.6.16/drivers/media/video/saa7134/saa7134-core.c	2006-04-06 20:00:56 +0400
@@ -543,6 +543,8 @@ static irqreturn_t saa7134_irq(int irq, 
 		if (report & SAA7134_IRQ_REPORT_GPIO16) {
 			switch (dev->has_remote) {
 				case SAA7134_REMOTE_GPIO:
+					if (!dev->remote)
+						break;
 					if  (dev->remote->mask_keydown & 0x10000) {
 						saa7134_input_irq(dev);
 					}
@@ -559,6 +561,8 @@ static irqreturn_t saa7134_irq(int irq, 
 		if (report & SAA7134_IRQ_REPORT_GPIO18) {
 			switch (dev->has_remote) {
 				case SAA7134_REMOTE_GPIO:
+					if (!dev->remote)
+						break;
 					if ((dev->remote->mask_keydown & 0x40000) ||
 					    (dev->remote->mask_keyup & 0x40000)) {
 						saa7134_input_irq(dev);
@@ -671,7 +675,7 @@ static int saa7134_hwinit2(struct saa713
 		SAA7134_IRQ2_INTE_PE      |
 		SAA7134_IRQ2_INTE_AR;
 
-	if (dev->has_remote == SAA7134_REMOTE_GPIO) {
+	if (dev->has_remote == SAA7134_REMOTE_GPIO && dev->remote) {
 		if (dev->remote->mask_keydown & 0x10000)
 			irq2_mask |= SAA7134_IRQ2_INTE_GPIO16;
 		else if (dev->remote->mask_keydown & 0x40000)


[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

Re: [2.6.16] saa7134 disable_ir oops

[Sergey Vlasov]

[-- Attachment #1: Type: text/plain, Size: 907 bytes --]

On Thu, Apr 06, 2006 at 06:03:20PM -0700, David Liontooth wrote:
> >Does the following patch fix things?
> >  
> The patch fixes the oops -- thank you! Yet the ir modules get loaded in 
> spite of the "disable_ir=1" parameter:
> 
> # sudo modprobe -vvv saa7134 card=2 tuner=43 disable_ir=1
> insmod /lib/modules/2.6.16/kernel/drivers/media/common/ir-common.ko
> insmod /lib/modules/2.6.16/kernel/drivers/media/video/ir-kbd-i2c.ko
> insmod /lib/modules/2.6.16/kernel/drivers/media/video/video-buf.ko
> insmod /lib/modules/2.6.16/kernel/drivers/media/video/saa7134/saa7134.ko 
> card=2 tuner=43 disable_ir=1

This is normal - saa7134 uses some symbols from these modules, and
such dependencies cannot be dynamic.  The disable_ir=1 option just
prevents saa7134 from enabling the IR input and registering the
corresponding input device with the kernel, therefore the IR code will
stay unused.

[-- Attachment #2: Type: application/pgp-signature, Size: 191 bytes --]

Re: [2.6.16] saa7134 disable_ir oops

[Hartmut Hackmann]

Hi,


Sergey Vlasov wrote:
> On Fri, Apr 07, 2006 at 10:16:10AM -0300, Mauro Carvalho Chehab wrote:
> 
>>Em Qui, 2006-04-06 ?s 20:20 +0400, Sergey Vlasov escreveu:
>>
>>>On Fri, 24 Mar 2006 14:00:46 -0800 David Liontooth wrote:
>>
>>>Does the following patch fix things?
>>>
>>
>>Applied at v4l-dvb tree. Thanks.
> 
> 
> IMHO this patch should also be added to 2.6.16-stable - it fixes oops in
> configurations which worked fine with older kernels.
> 
> -----------------------------------------------------------------------
> 
> saa7134: Fix oops with disable_ir=1
> 
> When disable_ir=1 parameter is used, or when saa7134_input_init1()
> fails for any other reason, dev->remote will remain NULL, and the
> driver will oops in saa7134_hwinit2().  Therefore dev->remote must be
> checked before dereferencing.
> 
> Signed-off-by: Sergey Vlasov <vsu@altlinux.ru>
> 
> --- linux-2.6.16.orig/drivers/media/video/saa7134/saa7134-core.c	2006-03-20 08:53:29 +0300
> +++ linux-2.6.16/drivers/media/video/saa7134/saa7134-core.c	2006-04-06 20:00:56 +0400
> @@ -543,6 +543,8 @@ static irqreturn_t saa7134_irq(int irq, 
>  		if (report & SAA7134_IRQ_REPORT_GPIO16) {
>  			switch (dev->has_remote) {
>  				case SAA7134_REMOTE_GPIO:
> +					if (!dev->remote)
> +						break;
>  					if  (dev->remote->mask_keydown & 0x10000) {
>  						saa7134_input_irq(dev);
>  					}
> @@ -559,6 +561,8 @@ static irqreturn_t saa7134_irq(int irq, 
>  		if (report & SAA7134_IRQ_REPORT_GPIO18) {
>  			switch (dev->has_remote) {
>  				case SAA7134_REMOTE_GPIO:
> +					if (!dev->remote)
> +						break;
>  					if ((dev->remote->mask_keydown & 0x40000) ||
>  					    (dev->remote->mask_keyup & 0x40000)) {
>  						saa7134_input_irq(dev);
> @@ -671,7 +675,7 @@ static int saa7134_hwinit2(struct saa713
>  		SAA7134_IRQ2_INTE_PE      |
>  		SAA7134_IRQ2_INTE_AR;
>  
> -	if (dev->has_remote == SAA7134_REMOTE_GPIO) {
> +	if (dev->has_remote == SAA7134_REMOTE_GPIO && dev->remote) {
>  		if (dev->remote->mask_keydown & 0x10000)
>  			irq2_mask |= SAA7134_IRQ2_INTE_GPIO16;
>  		else if (dev->remote->mask_keydown & 0x40000)
> 

Let's think this over, please.
1) The problem originally was that a card type with remote control was
    forced for a card that doesn't have one.
2) On the other hand you are right, this situation should not cause an oops.
The code should be that GPIO irqs should be completely ignored unless a handler
has been installed, so the
  if (dev->has_remote)
is completely wrong and should be repaced by something explictly initialized,
maybe the dev->remote pointer does the trick better.
I never worked seriously on this fraction of the driver, so there might be a
better solution.

Comments?
    Hartmut