diff mbox

[media] gspca-ov534: don't call sd_start() from sd_init()

Message ID 1376562572-10772-1-git-send-email-ospite@studenti.unina.it (mailing list archive)
State Accepted, archived
Delegated to: Hans de Goede
Headers

Commit Message

Antonio Ospite Aug. 15, 2013, 10:29 a.m. UTC 
  sd_start() operates on device controls but after the conversion to the
v4l2 control framework in commits 62bba5d and 1bd7d6a controls are
initialized in sd_init_controls() which is called _after_ sd_init():

The change fixes a NULL pointer dereference for Hercules Blog Webcam;
the problem is observable since 3.6:

  gspca_main: v2.14.0 registered
  gspca_main: ov534-2.14.0 probing 06f8:3002
  BUG: unable to handle kernel NULL pointer dereference at 0000000000000050
  IP: [<ffffffffa03c1b01>] v4l2_ctrl_g_ctrl+0x11/0x60 [videodev]
  PGD 0
  Oops: 0000 [#1] SMP
  Modules linked in: gspca_ov534(+) gspca_main videodev rfcomm bnep ppdev bluetooth binfmt_misc snd_hda_codec_hdmi snd_hda_codec_realtek stir4200 irda crc_ccitt usblp snd_hda_intel snd_hda_codec snd_hwdep snd_pcm hid_generic snd_page_alloc snd_seq_midi snd_seq_midi_event usbhid snd_rawmidi snd_seq snd_seq_device snd_timer hid i915 snd psmouse drm_kms_helper serio_raw mei_me drm mei soundcore video i2c_algo_bit lpc_ich mac_hid coretemp lp parport firewire_ohci firewire_core crc_itu_t ahci libahci alx mdio r8169 mii [last unloaded: parport_pc]
  CPU: 3 PID: 4352 Comm: modprobe Not tainted 3.11.0-031100rc2-generic #201307211535
  Hardware name: Gigabyte Technology Co., Ltd. To be filled by O.E.M./Z77-DS3H, BIOS F9 09/19/2012
  task: ffff8801c20f9770 ti: ffff8801ceaa0000 task.ti: ffff8801ceaa0000
  RIP: 0010:[<ffffffffa03c1b01>]  [<ffffffffa03c1b01>] v4l2_ctrl_g_ctrl+0x11/0x60 [videodev]
  RSP: 0018:ffff8801ceaa1af8  EFLAGS: 00010292
  RAX: 0000000000000001 RBX: 0000000000000000 RCX: 000000000001988b
  RDX: 000000000001988a RSI: ffffffffa032745a RDI: 0000000000000000
  RBP: ffff8801ceaa1b28 R08: 0000000000017380 R09: ffffea0008419d80
  R10: ffffffff81538f5a R11: 0000000000000002 R12: ffffffffa03273dc
  R13: ffffffffa03273dc R14: 0000000000000000 R15: ffffffffa03270a0
  FS:  00007f72d564a740(0000) GS:ffff88021f380000(0000) knlGS:0000000000000000
  CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
  CR2: 0000000000000050 CR3: 00000001bd1f0000 CR4: 00000000001407e0
  Stack:
   ffff8801ceaa1b28 ffffffffa0325cff ffff8801000001f4 ffff8801ceb44000
   ffffffffa03273dc ffff8801ceb44000 ffff8801ceaa1b58 ffffffffa032688e
   ffff8801ceb44000 ffffffffa03274f0 ffffffffa03274f0 ffff8801ceb44380
  Call Trace:
   [<ffffffffa0325cff>] ? sccb_w_array+0x3f/0x80 [gspca_ov534]
   [<ffffffffa032688e>] sd_start+0xce/0x2b0 [gspca_ov534]
   [<ffffffffa0326bf9>] sd_init+0x189/0x1e8 [gspca_ov534]
   [<ffffffffa02a0c95>] gspca_dev_probe2+0x285/0x410 [gspca_main]
   [<ffffffffa02a0e58>] gspca_dev_probe+0x38/0x60 [gspca_main]
   [<ffffffffa0325081>] sd_probe+0x21/0x30 [gspca_ov534]
   [<ffffffff8153c960>] usb_probe_interface+0x1c0/0x2f0
   [<ffffffff8148758c>] really_probe+0x6c/0x330
   [<ffffffff814879d7>] driver_probe_device+0x47/0xa0
   [<ffffffff81487adb>] __driver_attach+0xab/0xb0
   [<ffffffff81487a30>] ? driver_probe_device+0xa0/0xa0
   [<ffffffff814857be>] bus_for_each_dev+0x5e/0x90
   [<ffffffff8148714e>] driver_attach+0x1e/0x20
   [<ffffffff81486bdc>] bus_add_driver+0x10c/0x290
   [<ffffffff8148805d>] driver_register+0x7d/0x160
   [<ffffffff8153b590>] usb_register_driver+0xa0/0x160
   [<ffffffffa0067000>] ? 0xffffffffa0066fff
   [<ffffffffa006701e>] sd_driver_init+0x1e/0x1000 [gspca_ov534]
   [<ffffffff8100212a>] do_one_initcall+0xfa/0x1b0
   [<ffffffff810578c3>] ? set_memory_nx+0x43/0x50
   [<ffffffff81712e8d>] do_init_module+0x80/0x1d1
   [<ffffffff810d2079>] load_module+0x4c9/0x5f0
   [<ffffffff810cf7b0>] ? add_kallsyms+0x210/0x210
   [<ffffffff810d2254>] SyS_init_module+0xb4/0x100
   [<ffffffff817333ef>] tracesys+0xe1/0xe6
  Code: a0 09 00 00 48 c7 c7 30 c3 3c a0 e8 7a 38 ca e0 eb cf 0f 1f 84 00 00 00 00 00 0f 1f 44 00 00 55 48 89 e5 53 48 89 fb 48 83 ec 28 <8b> 47 50 83 e8 05 83 f8 02 77 09 80 b8 20 8c 3c a0 00 74 1d 48
  RIP  [<ffffffffa03c1b01>] v4l2_ctrl_g_ctrl+0x11/0x60 [videodev]
   RSP <ffff8801ceaa1af8>
  CR2: 0000000000000050
  ---[ end trace 6786f15abfd2ac90 ]---

Original bug report from:
https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1173723/

Signed-off-by: Antonio Ospite <ospite@studenti.unina.it>
Tested-by: Yaroslav Zakharuk <slavikz@gmail.com>
Cc: <stable@vger.kernel.org> # 3.6+
---
 drivers/media/usb/gspca/ov534.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

Comments

Hans de Goede Aug. 20, 2013, 12:21 p.m. UTC | #1 
Hi,

Thanks for the patch I've added this to my "gspca" tree, and this
will be included in my next pull-request to Mauro for 3.12

Regards,

Hans



On 08/15/2013 12:29 PM, Antonio Ospite wrote:
> sd_start() operates on device controls but after the conversion to the
> v4l2 control framework in commits 62bba5d and 1bd7d6a controls are
> initialized in sd_init_controls() which is called _after_ sd_init():
>
> The change fixes a NULL pointer dereference for Hercules Blog Webcam;
> the problem is observable since 3.6:
>
>    gspca_main: v2.14.0 registered
>    gspca_main: ov534-2.14.0 probing 06f8:3002
>    BUG: unable to handle kernel NULL pointer dereference at 0000000000000050
>    IP: [<ffffffffa03c1b01>] v4l2_ctrl_g_ctrl+0x11/0x60 [videodev]
>    PGD 0
>    Oops: 0000 [#1] SMP
>    Modules linked in: gspca_ov534(+) gspca_main videodev rfcomm bnep ppdev bluetooth binfmt_misc snd_hda_codec_hdmi snd_hda_codec_realtek stir4200 irda crc_ccitt usblp snd_hda_intel snd_hda_codec snd_hwdep snd_pcm hid_generic snd_page_alloc snd_seq_midi snd_seq_midi_event usbhid snd_rawmidi snd_seq snd_seq_device snd_timer hid i915 snd psmouse drm_kms_helper serio_raw mei_me drm mei soundcore video i2c_algo_bit lpc_ich mac_hid coretemp lp parport firewire_ohci firewire_core crc_itu_t ahci libahci alx mdio r8169 mii [last unloaded: parport_pc]
>    CPU: 3 PID: 4352 Comm: modprobe Not tainted 3.11.0-031100rc2-generic #201307211535
>    Hardware name: Gigabyte Technology Co., Ltd. To be filled by O.E.M./Z77-DS3H, BIOS F9 09/19/2012
>    task: ffff8801c20f9770 ti: ffff8801ceaa0000 task.ti: ffff8801ceaa0000
>    RIP: 0010:[<ffffffffa03c1b01>]  [<ffffffffa03c1b01>] v4l2_ctrl_g_ctrl+0x11/0x60 [videodev]
>    RSP: 0018:ffff8801ceaa1af8  EFLAGS: 00010292
>    RAX: 0000000000000001 RBX: 0000000000000000 RCX: 000000000001988b
>    RDX: 000000000001988a RSI: ffffffffa032745a RDI: 0000000000000000
>    RBP: ffff8801ceaa1b28 R08: 0000000000017380 R09: ffffea0008419d80
>    R10: ffffffff81538f5a R11: 0000000000000002 R12: ffffffffa03273dc
>    R13: ffffffffa03273dc R14: 0000000000000000 R15: ffffffffa03270a0
>    FS:  00007f72d564a740(0000) GS:ffff88021f380000(0000) knlGS:0000000000000000
>    CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
>    CR2: 0000000000000050 CR3: 00000001bd1f0000 CR4: 00000000001407e0
>    Stack:
>     ffff8801ceaa1b28 ffffffffa0325cff ffff8801000001f4 ffff8801ceb44000
>     ffffffffa03273dc ffff8801ceb44000 ffff8801ceaa1b58 ffffffffa032688e
>     ffff8801ceb44000 ffffffffa03274f0 ffffffffa03274f0 ffff8801ceb44380
>    Call Trace:
>     [<ffffffffa0325cff>] ? sccb_w_array+0x3f/0x80 [gspca_ov534]
>     [<ffffffffa032688e>] sd_start+0xce/0x2b0 [gspca_ov534]
>     [<ffffffffa0326bf9>] sd_init+0x189/0x1e8 [gspca_ov534]
>     [<ffffffffa02a0c95>] gspca_dev_probe2+0x285/0x410 [gspca_main]
>     [<ffffffffa02a0e58>] gspca_dev_probe+0x38/0x60 [gspca_main]
>     [<ffffffffa0325081>] sd_probe+0x21/0x30 [gspca_ov534]
>     [<ffffffff8153c960>] usb_probe_interface+0x1c0/0x2f0
>     [<ffffffff8148758c>] really_probe+0x6c/0x330
>     [<ffffffff814879d7>] driver_probe_device+0x47/0xa0
>     [<ffffffff81487adb>] __driver_attach+0xab/0xb0
>     [<ffffffff81487a30>] ? driver_probe_device+0xa0/0xa0
>     [<ffffffff814857be>] bus_for_each_dev+0x5e/0x90
>     [<ffffffff8148714e>] driver_attach+0x1e/0x20
>     [<ffffffff81486bdc>] bus_add_driver+0x10c/0x290
>     [<ffffffff8148805d>] driver_register+0x7d/0x160
>     [<ffffffff8153b590>] usb_register_driver+0xa0/0x160
>     [<ffffffffa0067000>] ? 0xffffffffa0066fff
>     [<ffffffffa006701e>] sd_driver_init+0x1e/0x1000 [gspca_ov534]
>     [<ffffffff8100212a>] do_one_initcall+0xfa/0x1b0
>     [<ffffffff810578c3>] ? set_memory_nx+0x43/0x50
>     [<ffffffff81712e8d>] do_init_module+0x80/0x1d1
>     [<ffffffff810d2079>] load_module+0x4c9/0x5f0
>     [<ffffffff810cf7b0>] ? add_kallsyms+0x210/0x210
>     [<ffffffff810d2254>] SyS_init_module+0xb4/0x100
>     [<ffffffff817333ef>] tracesys+0xe1/0xe6
>    Code: a0 09 00 00 48 c7 c7 30 c3 3c a0 e8 7a 38 ca e0 eb cf 0f 1f 84 00 00 00 00 00 0f 1f 44 00 00 55 48 89 e5 53 48 89 fb 48 83 ec 28 <8b> 47 50 83 e8 05 83 f8 02 77 09 80 b8 20 8c 3c a0 00 74 1d 48
>    RIP  [<ffffffffa03c1b01>] v4l2_ctrl_g_ctrl+0x11/0x60 [videodev]
>     RSP <ffff8801ceaa1af8>
>    CR2: 0000000000000050
>    ---[ end trace 6786f15abfd2ac90 ]---
>
> Original bug report from:
> https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1173723/
>
> Signed-off-by: Antonio Ospite <ospite@studenti.unina.it>
> Tested-by: Yaroslav Zakharuk <slavikz@gmail.com>
> Cc: <stable@vger.kernel.org> # 3.6+
> ---
>   drivers/media/usb/gspca/ov534.c | 3 +--
>   1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/drivers/media/usb/gspca/ov534.c b/drivers/media/usb/gspca/ov534.c
> index 2e28c81..03a33c4 100644
> --- a/drivers/media/usb/gspca/ov534.c
> +++ b/drivers/media/usb/gspca/ov534.c
> @@ -1305,8 +1305,7 @@ static int sd_init(struct gspca_dev *gspca_dev)
>   	ov534_set_led(gspca_dev, 1);
>   	sccb_w_array(gspca_dev, sensor_init[sd->sensor].val,
>   			sensor_init[sd->sensor].len);
> -	if (sd->sensor == SENSOR_OV767x)
> -		sd_start(gspca_dev);
> +
>   	sd_stopN(gspca_dev);
>   /*	set_frame_rate(gspca_dev);	*/
>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Antonio Ospite Aug. 20, 2013, 1:13 p.m. UTC | #2 
On Tue, 20 Aug 2013 14:21:22 +0200
Hans de Goede <hdegoede@redhat.com> wrote:

> Hi,
> 
> Thanks for the patch I've added this to my "gspca" tree, and this
> will be included in my next pull-request to Mauro for 3.12
> 

Thanks HdG.

It's fine with me to have the patch in 3.12 and then have it picked up
for inclusion in stable releases, I was just wondering why you didn't
consider it as a fix for 3.11, the patch fixes an actual crash
experienced by a user.

Regards,
   Antonio
Hans de Goede Aug. 20, 2013, 1:38 p.m. UTC | #3 
Hi,

On 08/20/2013 03:13 PM, Antonio Ospite wrote:
> On Tue, 20 Aug 2013 14:21:22 +0200
> Hans de Goede <hdegoede@redhat.com> wrote:
>
>> Hi,
>>
>> Thanks for the patch I've added this to my "gspca" tree, and this
>> will be included in my next pull-request to Mauro for 3.12
>>
>
> Thanks HdG.
>
> It's fine with me to have the patch in 3.12 and then have it picked up
> for inclusion in stable releases, I was just wondering why you didn't
> consider it as a fix for 3.11

I did not have time to do v4l work before now, and atm it is simply too
late for 3.11

Regards,

Hans
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/drivers/media/usb/gspca/ov534.c b/drivers/media/usb/gspca/ov534.c
index 2e28c81..03a33c4 100644
--- a/drivers/media/usb/gspca/ov534.c
+++ b/drivers/media/usb/gspca/ov534.c
@@ -1305,8 +1305,7 @@  static int sd_init(struct gspca_dev *gspca_dev)
 	ov534_set_led(gspca_dev, 1);
 	sccb_w_array(gspca_dev, sensor_init[sd->sensor].val,
 			sensor_init[sd->sensor].len);
-	if (sd->sensor == SENSOR_OV767x)
-		sd_start(gspca_dev);
+
 	sd_stopN(gspca_dev);
 /*	set_frame_rate(gspca_dev);	*/