2006-01-24 06:01:17

by Dave Jones

[permalink] [raw]
Subject: stradis oopses on modprobe.

We got this oops from a Fedora user..
Unable to handle kernel NULL pointer dereference at virtual address 000000fc
printing eip: *pde = 1ef5b067
Oops: 0002 [#1]
last sysfs file: /devices/pci0000:00/0000:00:11.5/modalias
Modules linked in: parport_pc parport floppy nvram orinoco_cs orinoco hermes hos
tap_cs hostap ieee80211_crypt stradis compat_ioctl32 ehci_hcd ohci1394 ieee1394
uhci_hcd snd_via82xx gameport snd_ac97_codec snd_ac97_bus snd_seq_dummy snd_seq_
oss snd_seq_midi_event snd_seq snd_pcm_oss snd_mixer_oss dvb_ttpci l64781 saa714
6_vv snd_pcm video_buf saa7146 v4l1_compat snd_timer v4l2_common snd_page_alloc
videodev ves1820 snd_mpu401_uart snd_rawmidi i2c_viapro stv0299 snd_seq_device v
ia_ircc dvb_core tda8083 snd irda stv0297 sp8870 ves1x93 ttpci_eeprom i2c_core v
ia_rhine mii soundcore crc_ccitt ext3 jbd
CPU:0
EIP:0060:[<e09ed182>]Not tainted VLI
EFLAGS: 00010246 (2.6.15-1.1863_FC5)
EIP is at stradis_probe+0x5ba/0xa81 [stradis]
eax: 00000000 ebx: e09f20e0 ecx: 00000000 edx: df23ccf0
esi: e09f1b2c edi: e09f2540 ebp: e09f20e0 esp: df074d70
ds: 007b es: 007b ss: 0068
Process modprobe (pid: 899, threadinfo=df074000 task=df23ccf0)
Stack: dfcf607c 00000000 00000000 00000001 dead4ead ffffffff ffffffff 00000001
dead4ead ffffffff ffffffff 00000001 dead4ead ffffffff ffffffff 00000001
dead4ead ffffffff ffffffff 00000001 dead4ead ffffffff ffffffff e09f190c
Call Trace:
[<c02284b0>] __driver_attach+0x0/0x8b [<c57
[<c02283fd>] driver_probe_device+0x42/0x8b [<c0228513>] __driver_attach+0x63/0x8b
[<c0227ef9>] bus_for_each_dev+0x33/0x55 [<c0228361>] driver_attach+0x11/0x13
[<c02284b0>] __driver_attach+0x0/0x8b [<c0227c1a>] bus_add_driver+0x64/0xfd
[<c01cd709>] __pci_register_driver+0x82/0xa4 [<e09c201a>] stradis_init+0x1a/0x2f [stradis]
[<c0131c95>] sys_init_module+0x137c/0x150e [<e09e1000>] ieee80211_crypt_null_init+0x0/0x6 [ieee80211_crypt
[<c01547df>] do_sync_read+0xb8/0xf3 [<c012a963 x0/0x2d
[<c02df797>] __mutex_unlock_slowpath+0x1c3/0x1c8 [<c0154727>] do_sync_read+0x0/0xf3
[<c01550a9>] vfs_read+0x9f/0x13e [<c0155410>] sys_read+0x3c/0x63
[<c0102ba9>] syscall_call+0x7/0xb <0>Code: c4 10 e9 df 04 00 00 8b 04 24 8b
98 74 01 00 00 31 c0 b9 18 01 00 00 89 df f3 ab c7 83 7c 02 00 00 00 00 00 00 8b
83 0c 03 00 00 <c7> 80 fc 00 00 00 00 00 ff ff b8 00 f0 ff ff 21 e0 f7 40 14 00


The backtrace is a bit of a mess, but what seems to happen is that we're oopsing
on the saawrite(0xffff0000, SAA7146_MC1) in init_saa7146
saawrite does a writel((dat), saa->saa7146_mem+(adr)), and as we've already
dereferenced 'saa' a few lines above, the oops is due to saa7146_mem being NULL
however that should be set up by configure_saa7146, which gets called in stradis_probe()
prior to calling init_saa7146.

*puzzled*.

Dave


2006-01-24 06:17:48

by Dave Jones

[permalink] [raw]
Subject: Re: stradis oopses on modprobe.

On Tue, Jan 24, 2006 at 01:01:03AM -0500, Dave Jones wrote:

> The backtrace is a bit of a mess, but what seems to happen is that we're oopsing
> on the saawrite(0xffff0000, SAA7146_MC1) in init_saa7146
> saawrite does a writel((dat), saa->saa7146_mem+(adr)), and as we've already
> dereferenced 'saa' a few lines above, the oops is due to saa7146_mem being NULL
> however that should be set up by configure_saa7146, which gets called in stradis_probe()
> prior to calling init_saa7146.
>
> *puzzled*.

This came from Jiri's PCI probing cleanups in 9ae82293ff3a3057d939b4f56d57eeea2f91bbec

There's a memset in init_saa7146 now, which wipes out the whole struct,
so we lose track of saa7146_mem

Jiri ?

Dave

2006-01-24 11:55:22

by Jiri Slaby

[permalink] [raw]
Subject: [PATCH] media video stradis memory fix

media video stradis memory fix

memset clears once set structure, there is actually no need for memset,
because configure function do it for us. Next, vfree(NULL) is legal, so
avoid useless labels.

Thanks Dave Jones for reporting this.

Signed-off-by: Jiri Slaby <[email protected]>

---
commit 6bf69f737175c593ae5dbccf1c836e86e60e80d4
tree 06984cc9874957c93276ad546f9e0756ebc2a11f
parent 34f0900e27681dd7bea568f6c593b9b216e7ce78
author <ku@bellona.(none)> Tue, 24 Jan 2006 12:18:56 +0100
committer <ku@bellona.(none)> Tue, 24 Jan 2006 12:18:56 +0100

drivers/media/video/stradis.c | 15 +++++----------
1 files changed, 5 insertions(+), 10 deletions(-)

diff --git a/drivers/media/video/stradis.c b/drivers/media/video/stradis.c
index 54fc330..9d76926 100644
--- a/drivers/media/video/stradis.c
+++ b/drivers/media/video/stradis.c
@@ -2012,7 +2012,6 @@ static int __devinit init_saa7146(struct
{
struct saa7146 *saa = pci_get_drvdata(pdev);

- memset(saa, 0, sizeof(*saa));
saa->user = 0;
/* reset the saa7146 */
saawrite(0xffff0000, SAA7146_MC1);
@@ -2062,16 +2061,16 @@ static int __devinit init_saa7146(struct
}
if (saa->audbuf == NULL && (saa->audbuf = vmalloc(65536)) == NULL) {
dev_err(&pdev->dev, "%d: malloc failed\n", saa->nr);
- goto errvid;
+ goto errfree;
}
if (saa->osdbuf == NULL && (saa->osdbuf = vmalloc(131072)) == NULL) {
dev_err(&pdev->dev, "%d: malloc failed\n", saa->nr);
- goto erraud;
+ goto errfree;
}
/* allocate 81920 byte buffer for clipping */
if ((saa->dmavid2 = kzalloc(VIDEO_CLIPMAP_SIZE, GFP_KERNEL)) == NULL) {
dev_err(&pdev->dev, "%d: clip kmalloc failed\n", saa->nr);
- goto errosd;
+ goto errfree;
}
/* setup clipping registers */
saawrite(virt_to_bus(saa->dmavid2), SAA7146_BASE_EVEN2);
@@ -2085,15 +2084,11 @@ static int __devinit init_saa7146(struct
I2CBusScan(saa);

return 0;
-errosd:
+errfree:
vfree(saa->osdbuf);
- saa->osdbuf = NULL;
-erraud:
vfree(saa->audbuf);
- saa->audbuf = NULL;
-errvid:
vfree(saa->vidbuf);
- saa->vidbuf = NULL;
+ saa->audbuf = saa->osdbuf = saa->vidbuf = NULL;
err:
return -ENOMEM;
}

2006-01-26 06:53:38

by Mauro Carvalho Chehab

[permalink] [raw]
Subject: Re: [PATCH] media video stradis memory fix

Em Ter, 2006-01-24 ?s 12:25 +0100, Jiri Slaby escreveu:
> media video stradis memory fix
>
> memset clears once set structure, there is actually no need for memset,
> because configure function do it for us. Next, vfree(NULL) is legal, so
> avoid useless labels.
>
> Thanks Dave Jones for reporting this.
>
> Signed-off-by: Jiri Slaby <[email protected]>
Applied to v4l-dvb.git. Thanks.

Cheers,
Mauro.