* Re: [PATCH]videobuf-core.c: add pointer check [not found] <1244337379.3355.1.camel@myhost> @ 2009-06-07 1:21 ` Figo.zhang 2009-06-15 16:04 ` Figo.zhang 0 siblings, 1 reply; 4+ messages in thread From: Figo.zhang @ 2009-06-07 1:21 UTC (permalink / raw) To: Mauro Carvalho Chehab, Hans Verkuil; +Cc: Linux Media Mailing List On Sun, 2009-06-07 at 09:16 +0800, Figo.zhang wrote: > On Wed, 2009-06-03 at 10:01 +0800, Figo.zhang wrote: > > add poiter check for videobuf_queue_core_init(). > > > > any guys who write a v4l driver, pass a NULL pointer or a non-inintial > > pointer to the first parameter such as videobuf_queue_sg_init() , it > > would be crashed. > > > > Signed-off-by: Figo.zhang <figo1802@xxxxxxxxx> > > --- > > drivers/media/video/videobuf-core.c | 1 + > > 1 files changed, 1 insertions(+), 0 deletions(-) > > > > diff --git a/drivers/media/video/videobuf-core.c b/drivers/media/video/videobuf-core.c > > index b7b0584..5f41fd9 100644 > > --- a/drivers/media/video/videobuf-core.c > > +++ b/drivers/media/video/videobuf-core.c > > @@ -118,6 +118,7 @@ void videobuf_queue_core_init(struct videobuf_queue *q, > > void *priv, > > struct videobuf_qtype_ops *int_ops) > > { > > + BUG_ON(!q); > > memset(q, 0, sizeof(*q)); > > q->irqlock = irqlock; > > q->dev = dev; > > > > i do a test driver for it, the main code like this. > > struct mydev_dev{ > struct video_device *video_dev; > ... > struct videobuf_queue *cap; > }; > > > > static int video_open(struct inode *inode, struct file *file) > { > ... > videobuf_queue_dma_contig_init(dev->cap, &video_qops, > &dev->pci->dev, &dev->slock, > V4L2_BUF_TYPE_VIDEO_CAPTURE, > V4L2_FIELD_ALTERNATE, > sizeof(struct mydev_buf), > dev); > > ... > } > > i pass a non-initial pointer for the first argment, so it crashed. > > > > BUG: unable to handle kernel NULL pointer dereference at 00000000 > IP: [<f8d6bd67>] :videobuf_core:videobuf_queue_core_init+0x1b/0xbf > *pde = 7ed5a067 > Oops: 0002 [#1] SMP > Modules linked in: mydev_drv tvp5160_vd videobuf_dma_contig videobuf_dma_sg > videobuf_core videocodec videodev v4l2_int_device v4l2_common v4l1_compat > compat_ioctl32 fuse i915 drm sco bridge stp bnep l2cap bluetooth sunrpc ipv6 > cpufreq_ondemand acpi_cpufreq dm_multipath uinput snd_hda_intel snd_seq_dummy > snd_seq_oss snd_seq_midi_event snd_seq snd_seq_device ppdev snd_pcm_oss parport_pc > snd_mixer_oss snd_pcm snd_timer snd_page_alloc snd_hwdep parport snd i2c_i801 > i2c_core pcspkr soundcore iTCO_wdt iTCO_vendor_support r8169 mii ftdi_sio usbserial > ata_generic pata_acpi [last unloaded: microcode] > > Pid: 4053, comm: capture Not tainted (2.6.27.5-117.fc10.i686 #1) > EIP: 0060:[<f8d6bd67>] EFLAGS: 00210246 CPU: 1 > EIP is at videobuf_queue_core_init+0x1b/0xbf [videobuf_core] > EAX: 00000000 EBX: 00000000 ECX: 00000036 EDX: 00000036 > ESI: f8e1e158 EDI: 00000000 EBP: f15f3e28 ESP: f15f3e18 > DS: 007b ES: 007b FS: 00d8 GS: 0033 SS: 0068 > Process capture (pid: 4053, ti=f15f3000 task=f1550000 task.ti=f15f3000) > Stack: f790905c f5fd5224 f15d8840 f5811560 f15f3e48 f8e01163 f5fd5224 00000001 > 00000007 0000006c f5fd5200 f8e0202c f15f3e70 f8e1b5a0 f5fd5224 00000001 > 00000007 0000006c f5fd5200 f8e0faa4 00000000 f15d8840 f15f3e88 f8e0c195 > Call Trace: > [<f8e01163>] ? videobuf_queue_dma_contig_init+0x1c/0x21 [videobuf_dma_contig] > [<f8e1b5a0>] ? video_open+0x8b/0xb1 [kt2741drv] > [<f8e0c195>] ? video_open+0xc7/0x125 [videodev] > [<c0492767>] ? chrdev_open+0x12b/0x142 > [<c048edd2>] ? __dentry_open+0x10e/0x1fc > [<c048ef47>] ? nameidata_to_filp+0x1f/0x33 > [<c049263c>] ? chrdev_open+0x0/0x142 > [<c0498a3c>] ? do_filp_open+0x31c/0x611 > [<c048a971>] ? virt_to_head_page+0x22/0x2e > [<c041d8ba>] ? need_resched+0x18/0x22 > [<c048ebf0>] ? do_sys_open+0x42/0xb7 > [<c048eca7>] ? sys_open+0x1e/0x26 > [<c0403c76>] ? syscall_call+0x7/0xb > [<c06a007b>] ? init_intel_cacheinfo+0x0/0x421 > ======================= > Code: d8 e8 69 ff ff ff 89 d8 e8 6d a9 93 c7 5b 5d c3 55 89 e5 57 89 c7 56 89 > d6 53 ba 36 00 00 00 83 ec 04 89 c3 89 4d f0 31 c0 89 d1 <f3> ab 8b 45 08 8b > 4d f0 89 b3 b8 00 00 00 89 43 10 8b 45 0c 89 > EIP: [<f8d6bd67>] videobuf_queue_core_init+0x1b/0xbf [videobuf_core] SS:ESP 0068:f15f3e18 > ---[ end trace 4bfe52d17b82af8c ]--- > > so i think it need to add pointer checking for the first argment of videobuf_queue_core_init(), > the videobuf code would be more stronger and reliable. > hi, Mauro & Hans, at this point, would you agree with me or would you like to give me some advice? Best Regards, Figo.zhang ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH]videobuf-core.c: add pointer check 2009-06-07 1:21 ` [PATCH]videobuf-core.c: add pointer check Figo.zhang @ 2009-06-15 16:04 ` Figo.zhang 0 siblings, 0 replies; 4+ messages in thread From: Figo.zhang @ 2009-06-15 16:04 UTC (permalink / raw) To: Mauro Carvalho Chehab; +Cc: Hans Verkuil, Linux Media Mailing List On Sun, 2009-06-07 at 09:21 +0800, Figo.zhang wrote: > On Sun, 2009-06-07 at 09:16 +0800, Figo.zhang wrote: > > On Wed, 2009-06-03 at 10:01 +0800, Figo.zhang wrote: > > > add poiter check for videobuf_queue_core_init(). > > > > > > any guys who write a v4l driver, pass a NULL pointer or a non-inintial > > > pointer to the first parameter such as videobuf_queue_sg_init() , it > > > would be crashed. > > > > > > Signed-off-by: Figo.zhang <figo1802@xxxxxxxxx> > > > --- > > > drivers/media/video/videobuf-core.c | 1 + > > > 1 files changed, 1 insertions(+), 0 deletions(-) > > > > > > diff --git a/drivers/media/video/videobuf-core.c b/drivers/media/video/videobuf-core.c > > > index b7b0584..5f41fd9 100644 > > > --- a/drivers/media/video/videobuf-core.c > > > +++ b/drivers/media/video/videobuf-core.c > > > @@ -118,6 +118,7 @@ void videobuf_queue_core_init(struct videobuf_queue *q, > > > void *priv, > > > struct videobuf_qtype_ops *int_ops) > > > { > > > + BUG_ON(!q); > > > memset(q, 0, sizeof(*q)); > > > q->irqlock = irqlock; > > > q->dev = dev; > > > > > > > i do a test driver for it, the main code like this. > > > > struct mydev_dev{ > > struct video_device *video_dev; > > ... > > struct videobuf_queue *cap; > > }; > > > > > > > > static int video_open(struct inode *inode, struct file *file) > > { > > ... > > videobuf_queue_dma_contig_init(dev->cap, &video_qops, > > &dev->pci->dev, &dev->slock, > > V4L2_BUF_TYPE_VIDEO_CAPTURE, > > V4L2_FIELD_ALTERNATE, > > sizeof(struct mydev_buf), > > dev); > > > > ... > > } > > > > i pass a non-initial pointer for the first argment, so it crashed. > > > > > > > > BUG: unable to handle kernel NULL pointer dereference at 00000000 > > IP: [<f8d6bd67>] :videobuf_core:videobuf_queue_core_init+0x1b/0xbf > > *pde = 7ed5a067 > > Oops: 0002 [#1] SMP > > Modules linked in: mydev_drv tvp5160_vd videobuf_dma_contig videobuf_dma_sg > > videobuf_core videocodec videodev v4l2_int_device v4l2_common v4l1_compat > > compat_ioctl32 fuse i915 drm sco bridge stp bnep l2cap bluetooth sunrpc ipv6 > > cpufreq_ondemand acpi_cpufreq dm_multipath uinput snd_hda_intel snd_seq_dummy > > snd_seq_oss snd_seq_midi_event snd_seq snd_seq_device ppdev snd_pcm_oss parport_pc > > snd_mixer_oss snd_pcm snd_timer snd_page_alloc snd_hwdep parport snd i2c_i801 > > i2c_core pcspkr soundcore iTCO_wdt iTCO_vendor_support r8169 mii ftdi_sio usbserial > > ata_generic pata_acpi [last unloaded: microcode] > > > > Pid: 4053, comm: capture Not tainted (2.6.27.5-117.fc10.i686 #1) > > EIP: 0060:[<f8d6bd67>] EFLAGS: 00210246 CPU: 1 > > EIP is at videobuf_queue_core_init+0x1b/0xbf [videobuf_core] > > EAX: 00000000 EBX: 00000000 ECX: 00000036 EDX: 00000036 > > ESI: f8e1e158 EDI: 00000000 EBP: f15f3e28 ESP: f15f3e18 > > DS: 007b ES: 007b FS: 00d8 GS: 0033 SS: 0068 > > Process capture (pid: 4053, ti=f15f3000 task=f1550000 task.ti=f15f3000) > > Stack: f790905c f5fd5224 f15d8840 f5811560 f15f3e48 f8e01163 f5fd5224 00000001 > > 00000007 0000006c f5fd5200 f8e0202c f15f3e70 f8e1b5a0 f5fd5224 00000001 > > 00000007 0000006c f5fd5200 f8e0faa4 00000000 f15d8840 f15f3e88 f8e0c195 > > Call Trace: > > [<f8e01163>] ? videobuf_queue_dma_contig_init+0x1c/0x21 [videobuf_dma_contig] > > [<f8e1b5a0>] ? video_open+0x8b/0xb1 [kt2741drv] > > [<f8e0c195>] ? video_open+0xc7/0x125 [videodev] > > [<c0492767>] ? chrdev_open+0x12b/0x142 > > [<c048edd2>] ? __dentry_open+0x10e/0x1fc > > [<c048ef47>] ? nameidata_to_filp+0x1f/0x33 > > [<c049263c>] ? chrdev_open+0x0/0x142 > > [<c0498a3c>] ? do_filp_open+0x31c/0x611 > > [<c048a971>] ? virt_to_head_page+0x22/0x2e > > [<c041d8ba>] ? need_resched+0x18/0x22 > > [<c048ebf0>] ? do_sys_open+0x42/0xb7 > > [<c048eca7>] ? sys_open+0x1e/0x26 > > [<c0403c76>] ? syscall_call+0x7/0xb > > [<c06a007b>] ? init_intel_cacheinfo+0x0/0x421 > > ======================= > > Code: d8 e8 69 ff ff ff 89 d8 e8 6d a9 93 c7 5b 5d c3 55 89 e5 57 89 c7 56 89 > > d6 53 ba 36 00 00 00 83 ec 04 89 c3 89 4d f0 31 c0 89 d1 <f3> ab 8b 45 08 8b > > 4d f0 89 b3 b8 00 00 00 89 43 10 8b 45 0c 89 > > EIP: [<f8d6bd67>] videobuf_queue_core_init+0x1b/0xbf [videobuf_core] SS:ESP 0068:f15f3e18 > > ---[ end trace 4bfe52d17b82af8c ]--- > > > > so i think it need to add pointer checking for the first argment of videobuf_queue_core_init(), > > the videobuf code would be more stronger and reliable. > > > > hi, Mauro & Hans, > > at this point, would you agree with me or would you like to give me some > advice? > > Best Regards, > > Figo.zhang hi Mauro, are you agree with this patch? Best Regards, Figo.zhang ^ permalink raw reply [flat|nested] 4+ messages in thread
* [PATCH]videobuf-core.c: add pointer check @ 2009-06-03 2:01 Figo.zhang 2009-06-05 2:13 ` figo.zhang 0 siblings, 1 reply; 4+ messages in thread From: Figo.zhang @ 2009-06-03 2:01 UTC (permalink / raw) To: Mauro Carvalho Chehab; +Cc: linux-media, Hans Verkuil, Trent Piepho add poiter check for videobuf_queue_core_init(). any guys who write a v4l driver, pass a NULL pointer or a non-inintial pointer to the first parameter such as videobuf_queue_sg_init() , it would be crashed. Signed-off-by: Figo.zhang <figo1802@gmail.com> --- drivers/media/video/videobuf-core.c | 1 + 1 files changed, 1 insertions(+), 0 deletions(-) diff --git a/drivers/media/video/videobuf-core.c b/drivers/media/video/videobuf-core.c index b7b0584..5f41fd9 100644 --- a/drivers/media/video/videobuf-core.c +++ b/drivers/media/video/videobuf-core.c @@ -118,6 +118,7 @@ void videobuf_queue_core_init(struct videobuf_queue *q, void *priv, struct videobuf_qtype_ops *int_ops) { + BUG_ON(!q); memset(q, 0, sizeof(*q)); q->irqlock = irqlock; q->dev = dev; ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH]videobuf-core.c: add pointer check 2009-06-03 2:01 Figo.zhang @ 2009-06-05 2:13 ` figo.zhang 0 siblings, 0 replies; 4+ messages in thread From: figo.zhang @ 2009-06-05 2:13 UTC (permalink / raw) To: Mauro Carvalho Chehab Cc: linux-media, Hans Verkuil, Trent Piepho, Laurent Pinchart On Wed, 2009-06-03 at 10:01 +0800, Figo.zhang wrote: > add poiter check for videobuf_queue_core_init(). > > any guys who write a v4l driver, pass a NULL pointer or a non-inintial > pointer to the first parameter such as videobuf_queue_sg_init() , it > would be crashed. > > Signed-off-by: Figo.zhang <figo1802@gmail.com> > --- > drivers/media/video/videobuf-core.c | 1 + > 1 files changed, 1 insertions(+), 0 deletions(-) > > diff --git a/drivers/media/video/videobuf-core.c b/drivers/media/video/videobuf-core.c > index b7b0584..5f41fd9 100644 > --- a/drivers/media/video/videobuf-core.c > +++ b/drivers/media/video/videobuf-core.c > @@ -118,6 +118,7 @@ void videobuf_queue_core_init(struct videobuf_queue *q, > void *priv, > struct videobuf_qtype_ops *int_ops) > { > + BUG_ON(!q); > memset(q, 0, sizeof(*q)); > q->irqlock = irqlock; > q->dev = dev; > i do a test driver for it, the main code like this. struct mydev_dev{ struct video_device *video_dev; ... struct videobuf_queue *cap; }; static int video_open(struct inode *inode, struct file *file) { ... videobuf_queue_dma_contig_init(dev->cap, &video_qops, &dev->pci->dev, &dev->slock, V4L2_BUF_TYPE_VIDEO_CAPTURE, V4L2_FIELD_ALTERNATE, sizeof(struct mydev_buf), dev); ... } i pass a non-initial pointer for the first argment, so it crashed. BUG: unable to handle kernel NULL pointer dereference at 00000000 IP: [<f8d6bd67>] :videobuf_core:videobuf_queue_core_init+0x1b/0xbf *pde = 7ed5a067 Oops: 0002 [#1] SMP Modules linked in: mydev_drv tvp5160_vd videobuf_dma_contig videobuf_dma_sg videobuf_core videocodec videodev v4l2_int_device v4l2_common v4l1_compat compat_ioctl32 fuse i915 drm sco bridge stp bnep l2cap bluetooth sunrpc ipv6 cpufreq_ondemand acpi_cpufreq dm_multipath uinput snd_hda_intel snd_seq_dummy snd_seq_oss snd_seq_midi_event snd_seq snd_seq_device ppdev snd_pcm_oss parport_pc snd_mixer_oss snd_pcm snd_timer snd_page_alloc snd_hwdep parport snd i2c_i801 i2c_core pcspkr soundcore iTCO_wdt iTCO_vendor_support r8169 mii ftdi_sio usbserial ata_generic pata_acpi [last unloaded: microcode] Pid: 4053, comm: capture Not tainted (2.6.27.5-117.fc10.i686 #1) EIP: 0060:[<f8d6bd67>] EFLAGS: 00210246 CPU: 1 EIP is at videobuf_queue_core_init+0x1b/0xbf [videobuf_core] EAX: 00000000 EBX: 00000000 ECX: 00000036 EDX: 00000036 ESI: f8e1e158 EDI: 00000000 EBP: f15f3e28 ESP: f15f3e18 DS: 007b ES: 007b FS: 00d8 GS: 0033 SS: 0068 Process capture (pid: 4053, ti=f15f3000 task=f1550000 task.ti=f15f3000) Stack: f790905c f5fd5224 f15d8840 f5811560 f15f3e48 f8e01163 f5fd5224 00000001 00000007 0000006c f5fd5200 f8e0202c f15f3e70 f8e1b5a0 f5fd5224 00000001 00000007 0000006c f5fd5200 f8e0faa4 00000000 f15d8840 f15f3e88 f8e0c195 Call Trace: [<f8e01163>] ? videobuf_queue_dma_contig_init+0x1c/0x21 [videobuf_dma_contig] [<f8e1b5a0>] ? video_open+0x8b/0xb1 [kt2741drv] [<f8e0c195>] ? video_open+0xc7/0x125 [videodev] [<c0492767>] ? chrdev_open+0x12b/0x142 [<c048edd2>] ? __dentry_open+0x10e/0x1fc [<c048ef47>] ? nameidata_to_filp+0x1f/0x33 [<c049263c>] ? chrdev_open+0x0/0x142 [<c0498a3c>] ? do_filp_open+0x31c/0x611 [<c048a971>] ? virt_to_head_page+0x22/0x2e [<c041d8ba>] ? need_resched+0x18/0x22 [<c048ebf0>] ? do_sys_open+0x42/0xb7 [<c048eca7>] ? sys_open+0x1e/0x26 [<c0403c76>] ? syscall_call+0x7/0xb [<c06a007b>] ? init_intel_cacheinfo+0x0/0x421 ======================= Code: d8 e8 69 ff ff ff 89 d8 e8 6d a9 93 c7 5b 5d c3 55 89 e5 57 89 c7 56 89 d6 53 ba 36 00 00 00 83 ec 04 89 c3 89 4d f0 31 c0 89 d1 <f3> ab 8b 45 08 8b 4d f0 89 b3 b8 00 00 00 89 43 10 8b 45 0c 89 EIP: [<f8d6bd67>] videobuf_queue_core_init+0x1b/0xbf [videobuf_core] SS:ESP 0068:f15f3e18 ---[ end trace 4bfe52d17b82af8c ]--- so i think it need to add pointer checking for the first argment of videobuf_queue_core_init(), the videobuf code would be more stronger and reliable. ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2009-06-15 16:04 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <1244337379.3355.1.camel@myhost>
2009-06-07 1:21 ` [PATCH]videobuf-core.c: add pointer check Figo.zhang
2009-06-15 16:04 ` Figo.zhang
2009-06-03 2:01 Figo.zhang
2009-06-05 2:13 ` figo.zhang
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox