* [v4.9-rc4] dvb-usb/cinergyT2 NULL pointer dereference @ 2016-11-06 15:40 Jörg Otte 2016-11-08 18:42 ` Linus Torvalds 0 siblings, 1 reply; 14+ messages in thread From: Jörg Otte @ 2016-11-06 15:40 UTC (permalink / raw) To: Mauro Carvalho Chehab Cc: Linus Torvalds, Linux Kernel Mailing List, Linux Media Mailing List Since v4.9-rc4 I get following crash in dvb-usb-cinergyT2 module. dvb-usb: found a 'TerraTec/qanu USB2.0 Highspeed DVB-T Receiver' in warm state. BUG: unable to handle kernel NULL pointer dereference at (null) IP: [<ffffffff846617af>] __mutex_lock_slowpath+0x6f/0x100 PGD 0 Oops: 0002 [#1] SMP Modules linked in: dvb_usb_cinergyT2(+) dvb_usb CPU: 0 PID: 2029 Comm: modprobe Not tainted 4.9.0-rc4-dvbmod #24 Hardware name: FUJITSU LIFEBOOK A544/FJNBB35 , BIOS Version 1.17 05/09/2014 task: ffff88020e943840 task.stack: ffff8801f36ec000 RIP: 0010:[<ffffffff846617af>] [<ffffffff846617af>] __mutex_lock_slowpath+0x6f/0x100 RSP: 0018:ffff8801f36efb10 EFLAGS: 00010282 RAX: 0000000000000000 RBX: ffff88021509bdc8 RCX: 00000000c0000100 RDX: 0000000000000001 RSI: 0000000000000000 RDI: ffff88021509bdcc RBP: ffff8801f36efb58 R08: ffff88021f216320 R09: 0000000000100000 R10: ffff88021f216320 R11: 00000023fee6c5a1 R12: ffff88020e943840 R13: ffff88021509bdcc R14: 00000000ffffffff R15: ffff88021509bdd0 FS: 00007f21adb86740(0000) GS:ffff88021f200000(0000) knlGS:0000000000000000 CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 CR2: 0000000000000000 CR3: 0000000215bce000 CR4: 00000000001406f0 Stack: ffff88021509bdd0 0000000000000000 0000000000000000 ffffffffc0137c80 ffff88021509bdc8 ffff8801f5944000 0000000000000001 ffffffffc0136b00 ffff880213e52000 ffff88021509bdc8 ffffffff84661856 ffff88021509bd80 Call Trace: [<ffffffff84661856>] ? mutex_lock+0x16/0x25 [<ffffffffc013616f>] ? cinergyt2_power_ctrl+0x1f/0x60 [dvb_usb_cinergyT2] [<ffffffffc012e67e>] ? dvb_usb_device_init+0x21e/0x5d0 [dvb_usb] [<ffffffffc0136021>] ? cinergyt2_usb_probe+0x21/0x50 [dvb_usb_cinergyT2] [<ffffffff844326f3>] ? usb_probe_interface+0xf3/0x2a0 [<ffffffff8438e348>] ? driver_probe_device+0x208/0x2b0 [<ffffffff8438e477>] ? __driver_attach+0x87/0x90 [<ffffffff8438e3f0>] ? driver_probe_device+0x2b0/0x2b0 [<ffffffff8438c612>] ? bus_for_each_dev+0x52/0x80 [<ffffffff8438d983>] ? bus_add_driver+0x1a3/0x220 [<ffffffff8438ec06>] ? driver_register+0x56/0xd0 [<ffffffff84431527>] ? usb_register_driver+0x77/0x130 [<ffffffffc013a000>] ? 0xffffffffc013a000 [<ffffffff84000426>] ? do_one_initcall+0x46/0x180 [<ffffffff840eb2c8>] ? free_vmap_area_noflush+0x38/0x70 [<ffffffff840f3844>] ? kmem_cache_alloc+0x84/0xc0 [<ffffffff840b802c>] ? do_init_module+0x50/0x1be [<ffffffff84095adb>] ? load_module+0x1d8b/0x2100 [<ffffffff84093020>] ? find_symbol_in_section+0xa0/0xa0 [<ffffffff84095fe9>] ? SyS_finit_module+0x89/0x90 [<ffffffff846637a0>] ? entry_SYSCALL_64_fastpath+0x13/0x94 Code: e8 a7 1d 00 00 8b 03 83 f8 01 0f 84 97 00 00 00 48 8b 43 10 4c 8d 7b 08 48 89 63 10 4c 89 3c 24 41 be ff ff ff ff 48 89 44 24 08 <48> 89 20 4c 89 64 24 10 eb 1a 49 c7 44 24 08 02 00 00 00 c6 43 RIP [<ffffffff846617af>] __mutex_lock_slowpath+0x6f/0x100 RSP <ffff8801f36efb10> CR2: 0000000000000000 ---[ end trace 648d79474da94e34 ]--- Thanks, Jörg ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [v4.9-rc4] dvb-usb/cinergyT2 NULL pointer dereference 2016-11-06 15:40 [v4.9-rc4] dvb-usb/cinergyT2 NULL pointer dereference Jörg Otte @ 2016-11-08 18:42 ` Linus Torvalds 2016-11-08 20:22 ` Mauro Carvalho Chehab 0 siblings, 1 reply; 14+ messages in thread From: Linus Torvalds @ 2016-11-08 18:42 UTC (permalink / raw) To: Jörg Otte, Patrick Boettcher, Mauro Carvalho Chehab Cc: Mauro Carvalho Chehab, Linux Kernel Mailing List, Linux Media Mailing List On Sun, Nov 6, 2016 at 7:40 AM, Jörg Otte <jrg.otte@gmail.com> wrote: > Since v4.9-rc4 I get following crash in dvb-usb-cinergyT2 module. Looks like it's commit 5ef8ed0e5608f ("[media] cinergyT2-core: don't do DMA on stack"), which movced the DMA data array from the stack to the "private" pointer. In the process it also added serialization in the form of "data_mutex", but and now it oopses on that mutex because the private pointer is NULL. It looks like the "->private" pointer is allocated in dvb_usb_adapter_init() cinergyt2_usb_probe -> dvb_usb_device_init -> dvb_usb_init() -> dvb_usb_adapter_init() but the dvb_usb_init() function calls dvb_usb_device_power_ctrl() (which calls the "power_ctrl" function, which is cinergyt2_power_ctrl() for that drive) *before* it initializes the private field. Mauro, Patrick, could dvb_usb_adapter_init() be called earlier, perhaps? Linus ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [v4.9-rc4] dvb-usb/cinergyT2 NULL pointer dereference 2016-11-08 18:42 ` Linus Torvalds @ 2016-11-08 20:22 ` Mauro Carvalho Chehab 2016-11-08 21:15 ` Benjamin Larsson 2016-11-09 11:09 ` Jörg Otte 0 siblings, 2 replies; 14+ messages in thread From: Mauro Carvalho Chehab @ 2016-11-08 20:22 UTC (permalink / raw) To: Linus Torvalds Cc: Jörg Otte, Patrick Boettcher, Mauro Carvalho Chehab, Linux Kernel Mailing List, Linux Media Mailing List Em Tue, 8 Nov 2016 10:42:03 -0800 Linus Torvalds <torvalds@linux-foundation.org> escreveu: > On Sun, Nov 6, 2016 at 7:40 AM, Jörg Otte <jrg.otte@gmail.com> wrote: > > Since v4.9-rc4 I get following crash in dvb-usb-cinergyT2 module. > > Looks like it's commit 5ef8ed0e5608f ("[media] cinergyT2-core: don't > do DMA on stack"), which movced the DMA data array from the stack to > the "private" pointer. In the process it also added serialization in > the form of "data_mutex", but and now it oopses on that mutex because > the private pointer is NULL. > > It looks like the "->private" pointer is allocated in dvb_usb_adapter_init() > > cinergyt2_usb_probe -> > dvb_usb_device_init -> > dvb_usb_init() -> > dvb_usb_adapter_init() > > but the dvb_usb_init() function calls dvb_usb_device_power_ctrl() > (which calls the "power_ctrl" function, which is > cinergyt2_power_ctrl() for that drive) *before* it initializes the > private field. > > Mauro, Patrick, could dvb_usb_adapter_init() be called earlier, perhaps? Calling it earlier won't work, as we need to load the firmware before sending the power control commands on some devices. Probably the best here is to pass an extra optional function parameter that will initialize the mutex before calling any functions. Btw, if it broke here, the DMA fixes will likely break on other drivers. So, after Jörg tests this patch, I'll work on a patch series addressing this issue on the other drivers I touched. Regards, Mauro - [PATCH RFC] cinergyT2-core: initialize the mutex early NOTE: don't merge this patch as-is... I actually folded two patches together here, in order to make easier to test, but the best is to place the changes at the core first, and then the changes at the drivers that would need an early init. The mutex used to protect the URB data buffer needs to be inialized early, as otherwise it will cause an OOPS: dvb-usb: found a 'TerraTec/qanu USB2.0 Highspeed DVB-T Receiver' in warm state. BUG: unable to handle kernel NULL pointer dereference at (null) IP: [<ffffffff846617af>] __mutex_lock_slowpath+0x6f/0x100 PGD 0 Oops: 0002 [#1] SMP Modules linked in: dvb_usb_cinergyT2(+) dvb_usb CPU: 0 PID: 2029 Comm: modprobe Not tainted 4.9.0-rc4-dvbmod #24 Hardware name: FUJITSU LIFEBOOK A544/FJNBB35 , BIOS Version 1.17 05/09/2014 task: ffff88020e943840 task.stack: ffff8801f36ec000 RIP: 0010:[<ffffffff846617af>] [<ffffffff846617af>] __mutex_lock_slowpath+0x6f/0x100 RSP: 0018:ffff8801f36efb10 EFLAGS: 00010282 RAX: 0000000000000000 RBX: ffff88021509bdc8 RCX: 00000000c0000100 RDX: 0000000000000001 RSI: 0000000000000000 RDI: ffff88021509bdcc RBP: ffff8801f36efb58 R08: ffff88021f216320 R09: 0000000000100000 R10: ffff88021f216320 R11: 00000023fee6c5a1 R12: ffff88020e943840 R13: ffff88021509bdcc R14: 00000000ffffffff R15: ffff88021509bdd0 FS: 00007f21adb86740(0000) GS:ffff88021f200000(0000) knlGS:0000000000000000 CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 CR2: 0000000000000000 CR3: 0000000215bce000 CR4: 00000000001406f0 Stack: ffff88021509bdd0 0000000000000000 0000000000000000 ffffffffc0137c80 ffff88021509bdc8 ffff8801f5944000 0000000000000001 ffffffffc0136b00 ffff880213e52000 ffff88021509bdc8 ffffffff84661856 ffff88021509bd80 Call Trace: [<ffffffff84661856>] ? mutex_lock+0x16/0x25 [<ffffffffc013616f>] ? cinergyt2_power_ctrl+0x1f/0x60 [dvb_usb_cinergyT2] [<ffffffffc012e67e>] ? dvb_usb_device_init+0x21e/0x5d0 [dvb_usb] [<ffffffffc0136021>] ? cinergyt2_usb_probe+0x21/0x50 [dvb_usb_cinergyT2] [<ffffffff844326f3>] ? usb_probe_interface+0xf3/0x2a0 [<ffffffff8438e348>] ? driver_probe_device+0x208/0x2b0 [<ffffffff8438e477>] ? __driver_attach+0x87/0x90 [<ffffffff8438e3f0>] ? driver_probe_device+0x2b0/0x2b0 [<ffffffff8438c612>] ? bus_for_each_dev+0x52/0x80 [<ffffffff8438d983>] ? bus_add_driver+0x1a3/0x220 [<ffffffff8438ec06>] ? driver_register+0x56/0xd0 [<ffffffff84431527>] ? usb_register_driver+0x77/0x130 [<ffffffffc013a000>] ? 0xffffffffc013a000 [<ffffffff84000426>] ? do_one_initcall+0x46/0x180 [<ffffffff840eb2c8>] ? free_vmap_area_noflush+0x38/0x70 [<ffffffff840f3844>] ? kmem_cache_alloc+0x84/0xc0 [<ffffffff840b802c>] ? do_init_module+0x50/0x1be [<ffffffff84095adb>] ? load_module+0x1d8b/0x2100 [<ffffffff84093020>] ? find_symbol_in_section+0xa0/0xa0 [<ffffffff84095fe9>] ? SyS_finit_module+0x89/0x90 [<ffffffff846637a0>] ? entry_SYSCALL_64_fastpath+0x13/0x94 Code: e8 a7 1d 00 00 8b 03 83 f8 01 0f 84 97 00 00 00 48 8b 43 10 4c 8d 7b 08 48 89 63 10 4c 89 3c 24 41 be ff ff ff ff 48 89 44 24 08 <48> 89 20 4c 89 64 24 10 eb 1a 49 c7 44 24 08 02 00 00 00 c6 43 RIP [<ffffffff846617af>] __mutex_lock_slowpath+0x6f/0x100 RSP <ffff8801f36efb10> CR2: 0000000000000000 Reported-by: Jörg Otte <jrg.otte@gmail.com> Fixes: 6679a901c380 ("[media] cinergyT2-core: don't do DMA on stack") Signed-off-by: Mauro Carvalho Chehab <mchehab@s-opensource.com> >From cbc7e48a86e8ffd16e49b10061120dfc417397f8 Mon Sep 17 00:00:00 2001 From: Mauro Carvalho Chehab <mchehab@s-opensource.com> Date: Tue, 8 Nov 2016 18:04:24 -0200 Subject: [PATCH] [media] dvb-usb: allow early initialization of usb device priv data On some drivers, we need to initialize a mutex before calling power control or firmware download routines. Add an extra parameter to allow it. Signed-off-by: Mauro Carvalho Chehab <mchehab@s-opensource.com> diff --git a/drivers/media/usb/dvb-usb/a800.c b/drivers/media/usb/dvb-usb/a800.c index 7ba975bea96a..1e14f79aa57a 100644 --- a/drivers/media/usb/dvb-usb/a800.c +++ b/drivers/media/usb/dvb-usb/a800.c @@ -107,7 +107,7 @@ static int a800_probe(struct usb_interface *intf, const struct usb_device_id *id) { return dvb_usb_device_init(intf, &a800_properties, - THIS_MODULE, NULL, adapter_nr); + THIS_MODULE, NULL, adapter_nr, NULL); } /* do not change the order of the ID table */ diff --git a/drivers/media/usb/dvb-usb/af9005.c b/drivers/media/usb/dvb-usb/af9005.c index b257780fb380..1625b4714d83 100644 --- a/drivers/media/usb/dvb-usb/af9005.c +++ b/drivers/media/usb/dvb-usb/af9005.c @@ -1009,7 +1009,7 @@ static int af9005_usb_probe(struct usb_interface *intf, int ret; ret = dvb_usb_device_init(intf, &af9005_properties, - THIS_MODULE, &d, adapter_nr); + THIS_MODULE, &d, adapter_nr, NULL); if (ret < 0) return ret; diff --git a/drivers/media/usb/dvb-usb/az6027.c b/drivers/media/usb/dvb-usb/az6027.c index 2e711362847e..e8f73a96efd9 100644 --- a/drivers/media/usb/dvb-usb/az6027.c +++ b/drivers/media/usb/dvb-usb/az6027.c @@ -946,7 +946,7 @@ static int az6027_usb_probe(struct usb_interface *intf, &az6027_properties, THIS_MODULE, NULL, - adapter_nr); + adapter_nr, NULL); } /* I2C */ diff --git a/drivers/media/usb/dvb-usb/cinergyT2-core.c b/drivers/media/usb/dvb-usb/cinergyT2-core.c index 8ac825413d5a..2aa99c52e39d 100644 --- a/drivers/media/usb/dvb-usb/cinergyT2-core.c +++ b/drivers/media/usb/dvb-usb/cinergyT2-core.c @@ -206,24 +206,21 @@ static int cinergyt2_rc_query(struct dvb_usb_device *d, u32 *event, int *state) return ret; } -static int cinergyt2_usb_probe(struct usb_interface *intf, - const struct usb_device_id *id) +static int cinergyT2_init_mutex(struct dvb_usb_device *d) { - struct dvb_usb_device *d; - struct cinergyt2_state *st; - int ret; - - ret = dvb_usb_device_init(intf, &cinergyt2_properties, - THIS_MODULE, &d, adapter_nr); - if (ret < 0) - return ret; + struct cinergyt2_state *st = d->priv; - st = d->priv; mutex_init(&st->data_mutex); - return 0; } +static int cinergyt2_usb_probe(struct usb_interface *intf, + const struct usb_device_id *id) +{ + return dvb_usb_device_init(intf, &cinergyt2_properties, + THIS_MODULE, NULL, adapter_nr, + cinergyT2_init_mutex); +} static struct usb_device_id cinergyt2_usb_table[] = { { USB_DEVICE(USB_VID_TERRATEC, 0x0038) }, diff --git a/drivers/media/usb/dvb-usb/cxusb.c b/drivers/media/usb/dvb-usb/cxusb.c index 39772812269d..73c1a8568b55 100644 --- a/drivers/media/usb/dvb-usb/cxusb.c +++ b/drivers/media/usb/dvb-usb/cxusb.c @@ -1465,33 +1465,33 @@ static int cxusb_probe(struct usb_interface *intf, struct cxusb_state *st; if (0 == dvb_usb_device_init(intf, &cxusb_medion_properties, - THIS_MODULE, &d, adapter_nr) || + THIS_MODULE, &d, adapter_nr, NULL) || 0 == dvb_usb_device_init(intf, &cxusb_bluebird_lgh064f_properties, - THIS_MODULE, &d, adapter_nr) || + THIS_MODULE, &d, adapter_nr, NULL) || 0 == dvb_usb_device_init(intf, &cxusb_bluebird_dee1601_properties, - THIS_MODULE, &d, adapter_nr) || + THIS_MODULE, &d, adapter_nr, NULL) || 0 == dvb_usb_device_init(intf, &cxusb_bluebird_lgz201_properties, - THIS_MODULE, &d, adapter_nr) || + THIS_MODULE, &d, adapter_nr, NULL) || 0 == dvb_usb_device_init(intf, &cxusb_bluebird_dtt7579_properties, - THIS_MODULE, &d, adapter_nr) || + THIS_MODULE, &d, adapter_nr, NULL) || 0 == dvb_usb_device_init(intf, &cxusb_bluebird_dualdig4_properties, - THIS_MODULE, &d, adapter_nr) || + THIS_MODULE, &d, adapter_nr, NULL) || 0 == dvb_usb_device_init(intf, &cxusb_bluebird_nano2_properties, - THIS_MODULE, &d, adapter_nr) || + THIS_MODULE, &d, adapter_nr, NULL) || 0 == dvb_usb_device_init(intf, &cxusb_bluebird_nano2_needsfirmware_properties, - THIS_MODULE, &d, adapter_nr) || + THIS_MODULE, &d, adapter_nr, NULL) || 0 == dvb_usb_device_init(intf, &cxusb_aver_a868r_properties, - THIS_MODULE, &d, adapter_nr) || + THIS_MODULE, &d, adapter_nr, NULL) || 0 == dvb_usb_device_init(intf, &cxusb_bluebird_dualdig4_rev2_properties, - THIS_MODULE, &d, adapter_nr) || + THIS_MODULE, &d, adapter_nr, NULL) || 0 == dvb_usb_device_init(intf, &cxusb_d680_dmb_properties, - THIS_MODULE, &d, adapter_nr) || + THIS_MODULE, &d, adapter_nr, NULL) || 0 == dvb_usb_device_init(intf, &cxusb_mygica_d689_properties, - THIS_MODULE, &d, adapter_nr) || + THIS_MODULE, &d, adapter_nr, NULL) || 0 == dvb_usb_device_init(intf, &cxusb_mygica_t230_properties, - THIS_MODULE, &d, adapter_nr) || + THIS_MODULE, &d, adapter_nr, NULL) || 0) { st = d->priv; mutex_init(&st->data_mutex); diff --git a/drivers/media/usb/dvb-usb/dib0700_core.c b/drivers/media/usb/dvb-usb/dib0700_core.c index 92d5408684ac..3423b56c92b3 100644 --- a/drivers/media/usb/dvb-usb/dib0700_core.c +++ b/drivers/media/usb/dvb-usb/dib0700_core.c @@ -871,7 +871,7 @@ static int dib0700_probe(struct usb_interface *intf, for (i = 0; i < dib0700_device_count; i++) if (dvb_usb_device_init(intf, &dib0700_devices[i], THIS_MODULE, - &dev, adapter_nr) == 0) { + &dev, adapter_nr, NULL) == 0) { struct dib0700_state *st = dev->priv; u32 hwversion, romversion, fw_version, fwtype; diff --git a/drivers/media/usb/dvb-usb/dibusb-mb.c b/drivers/media/usb/dvb-usb/dibusb-mb.c index a0057641cc86..de4ffe81e8d7 100644 --- a/drivers/media/usb/dvb-usb/dibusb-mb.c +++ b/drivers/media/usb/dvb-usb/dibusb-mb.c @@ -111,13 +111,13 @@ static int dibusb_probe(struct usb_interface *intf, const struct usb_device_id *id) { if (0 == dvb_usb_device_init(intf, &dibusb1_1_properties, - THIS_MODULE, NULL, adapter_nr) || + THIS_MODULE, NULL, adapter_nr, NULL) || 0 == dvb_usb_device_init(intf, &dibusb1_1_an2235_properties, - THIS_MODULE, NULL, adapter_nr) || + THIS_MODULE, NULL, adapter_nr, NULL) || 0 == dvb_usb_device_init(intf, &dibusb2_0b_properties, - THIS_MODULE, NULL, adapter_nr) || + THIS_MODULE, NULL, adapter_nr, NULL) || 0 == dvb_usb_device_init(intf, &artec_t1_usb2_properties, - THIS_MODULE, NULL, adapter_nr)) + THIS_MODULE, NULL, adapter_nr, NULL)) return 0; return -EINVAL; diff --git a/drivers/media/usb/dvb-usb/dibusb-mc.c b/drivers/media/usb/dvb-usb/dibusb-mc.c index 08fb8a3f6e0c..d50731bb5372 100644 --- a/drivers/media/usb/dvb-usb/dibusb-mc.c +++ b/drivers/media/usb/dvb-usb/dibusb-mc.c @@ -23,7 +23,7 @@ static int dibusb_mc_probe(struct usb_interface *intf, const struct usb_device_id *id) { return dvb_usb_device_init(intf, &dibusb_mc_properties, THIS_MODULE, - NULL, adapter_nr); + NULL, adapter_nr, NULL); } /* do not change the order of the ID table */ diff --git a/drivers/media/usb/dvb-usb/digitv.c b/drivers/media/usb/dvb-usb/digitv.c index 4284f6984dc1..3dad2293e598 100644 --- a/drivers/media/usb/dvb-usb/digitv.c +++ b/drivers/media/usb/dvb-usb/digitv.c @@ -269,7 +269,7 @@ static int digitv_probe(struct usb_interface *intf, { struct dvb_usb_device *d; int ret = dvb_usb_device_init(intf, &digitv_properties, THIS_MODULE, &d, - adapter_nr); + adapter_nr, NULL); if (ret == 0) { u8 b[4] = { 0 }; diff --git a/drivers/media/usb/dvb-usb/dtt200u.c b/drivers/media/usb/dvb-usb/dtt200u.c index f88572c7ae7c..3255fee31433 100644 --- a/drivers/media/usb/dvb-usb/dtt200u.c +++ b/drivers/media/usb/dvb-usb/dtt200u.c @@ -149,15 +149,15 @@ static int dtt200u_usb_probe(struct usb_interface *intf, struct dtt200u_state *st; if (0 == dvb_usb_device_init(intf, &dtt200u_properties, - THIS_MODULE, &d, adapter_nr) || + THIS_MODULE, &d, adapter_nr, NULL) || 0 == dvb_usb_device_init(intf, &wt220u_properties, - THIS_MODULE, &d, adapter_nr) || + THIS_MODULE, &d, adapter_nr, NULL) || 0 == dvb_usb_device_init(intf, &wt220u_fc_properties, - THIS_MODULE, &d, adapter_nr) || + THIS_MODULE, &d, adapter_nr, NULL) || 0 == dvb_usb_device_init(intf, &wt220u_zl0353_properties, - THIS_MODULE, &d, adapter_nr) || + THIS_MODULE, &d, adapter_nr, NULL) || 0 == dvb_usb_device_init(intf, &wt220u_miglia_properties, - THIS_MODULE, &d, adapter_nr)) { + THIS_MODULE, &d, adapter_nr, NULL)) { st = d->priv; mutex_init(&st->data_mutex); diff --git a/drivers/media/usb/dvb-usb/dtv5100.c b/drivers/media/usb/dvb-usb/dtv5100.c index c60fb54f445f..36b553cb3133 100644 --- a/drivers/media/usb/dvb-usb/dtv5100.c +++ b/drivers/media/usb/dvb-usb/dtv5100.c @@ -165,7 +165,7 @@ static int dtv5100_probe(struct usb_interface *intf, } ret = dvb_usb_device_init(intf, &dtv5100_properties, - THIS_MODULE, NULL, adapter_nr); + THIS_MODULE, NULL, adapter_nr, NULL); if (ret) return ret; diff --git a/drivers/media/usb/dvb-usb/dvb-usb-init.c b/drivers/media/usb/dvb-usb/dvb-usb-init.c index 3896ba9a4179..66493fb25645 100644 --- a/drivers/media/usb/dvb-usb/dvb-usb-init.c +++ b/drivers/media/usb/dvb-usb/dvb-usb-init.c @@ -138,23 +138,14 @@ static int dvb_usb_exit(struct dvb_usb_device *d) return 0; } -static int dvb_usb_init(struct dvb_usb_device *d, short *adapter_nums) +static int dvb_usb_init(struct dvb_usb_device *d, short *adapter_nums, + int (init_device)(struct dvb_usb_device *d)) { int ret = 0; mutex_init(&d->usb_mutex); mutex_init(&d->i2c_mutex); - d->state = DVB_USB_STATE_INIT; - - if (d->props.size_of_priv > 0) { - d->priv = kzalloc(d->props.size_of_priv, GFP_KERNEL); - if (d->priv == NULL) { - err("no memory for priv in 'struct dvb_usb_device'"); - return -ENOMEM; - } - } - /* check the capabilities and set appropriate variables */ dvb_usb_device_power_ctrl(d, 1); @@ -233,7 +224,8 @@ int dvb_usb_device_power_ctrl(struct dvb_usb_device *d, int onoff) int dvb_usb_device_init(struct usb_interface *intf, struct dvb_usb_device_properties *props, struct module *owner, struct dvb_usb_device **du, - short *adapter_nums) + short *adapter_nums, + int (init_device)(struct dvb_usb_device *d)) { struct usb_device *udev = interface_to_usbdev(intf); struct dvb_usb_device *d = NULL; @@ -249,19 +241,42 @@ int dvb_usb_device_init(struct usb_interface *intf, return -ENODEV; } - if (cold) { - info("found a '%s' in cold state, will try to load a firmware", desc->name); - ret = dvb_usb_download_firmware(udev, props); - if (!props->no_reconnect || ret != 0) - return ret; - } - - info("found a '%s' in warm state.", desc->name); d = kzalloc(sizeof(struct dvb_usb_device), GFP_KERNEL); if (d == NULL) { err("no memory for 'struct dvb_usb_device'"); return -ENOMEM; } + d->state = DVB_USB_STATE_INIT; + + if (d->props.size_of_priv > 0) { + d->priv = kzalloc(d->props.size_of_priv, GFP_KERNEL); + if (d->priv == NULL) { + err("no memory for priv in 'struct dvb_usb_device'"); + ret = -ENOMEM; + goto err; + } + } + + /* + * Some drivers may need to early initialize the device private data, + * for example, when a mutex is serializing URB reads/writes, + * in order for dvb_usb_device_power_ctrl() or firmware load to work. + */ + if (init_device) { + ret = init_device(d); + if (ret < 0) + goto err; + } + + if (cold) { + info("found a '%s' in cold state, will try to load a firmware", + desc->name); + ret = dvb_usb_download_firmware(udev, props); + if (!props->no_reconnect || ret != 0) + goto err; + } else { + info("found a '%s' in warm state.", desc->name); + } d->udev = udev; memcpy(&d->props, props, sizeof(struct dvb_usb_device_properties)); @@ -273,12 +288,17 @@ int dvb_usb_device_init(struct usb_interface *intf, if (du != NULL) *du = d; - ret = dvb_usb_init(d, adapter_nums); + ret = dvb_usb_init(d, adapter_nums, init_device); - if (ret == 0) + if (!ret) { info("%s successfully initialized and connected.", desc->name); - else - info("%s error while loading driver (%d)", desc->name, ret); + return 0; + } +err: + info("%s error while loading driver (%d)", desc->name, ret); + + kfree(d->priv); + kfree(d); return ret; } EXPORT_SYMBOL(dvb_usb_device_init); diff --git a/drivers/media/usb/dvb-usb/dvb-usb.h b/drivers/media/usb/dvb-usb/dvb-usb.h index 1448c3d27ea2..02c4dd3c206a 100644 --- a/drivers/media/usb/dvb-usb/dvb-usb.h +++ b/drivers/media/usb/dvb-usb/dvb-usb.h @@ -458,7 +458,9 @@ struct dvb_usb_device { extern int dvb_usb_device_init(struct usb_interface *, struct dvb_usb_device_properties *, struct module *, struct dvb_usb_device **, - short *adapter_nums); + short *adapter_nums, + int (init_device)(struct dvb_usb_device *d)); + extern void dvb_usb_device_exit(struct usb_interface *); /* the generic read/write method for device control */ diff --git a/drivers/media/usb/dvb-usb/dw2102.c b/drivers/media/usb/dvb-usb/dw2102.c index 2c720cb2fb00..1cb8d96f485e 100644 --- a/drivers/media/usb/dvb-usb/dw2102.c +++ b/drivers/media/usb/dvb-usb/dw2102.c @@ -2284,27 +2284,27 @@ static int dw2102_probe(struct usb_interface *intf, s421->adapter->fe[0].frontend_attach = m88rs2000_frontend_attach; if (0 == dvb_usb_device_init(intf, &dw2102_properties, - THIS_MODULE, NULL, adapter_nr) || + THIS_MODULE, NULL, adapter_nr, NULL) || 0 == dvb_usb_device_init(intf, &dw2104_properties, - THIS_MODULE, NULL, adapter_nr) || + THIS_MODULE, NULL, adapter_nr, NULL) || 0 == dvb_usb_device_init(intf, &dw3101_properties, - THIS_MODULE, NULL, adapter_nr) || + THIS_MODULE, NULL, adapter_nr, NULL) || 0 == dvb_usb_device_init(intf, &s6x0_properties, - THIS_MODULE, NULL, adapter_nr) || + THIS_MODULE, NULL, adapter_nr, NULL) || 0 == dvb_usb_device_init(intf, p1100, - THIS_MODULE, NULL, adapter_nr) || + THIS_MODULE, NULL, adapter_nr, NULL) || 0 == dvb_usb_device_init(intf, s660, - THIS_MODULE, NULL, adapter_nr) || + THIS_MODULE, NULL, adapter_nr, NULL) || 0 == dvb_usb_device_init(intf, p7500, - THIS_MODULE, NULL, adapter_nr) || + THIS_MODULE, NULL, adapter_nr, NULL) || 0 == dvb_usb_device_init(intf, s421, - THIS_MODULE, NULL, adapter_nr) || + THIS_MODULE, NULL, adapter_nr, NULL) || 0 == dvb_usb_device_init(intf, &su3000_properties, - THIS_MODULE, NULL, adapter_nr) || + THIS_MODULE, NULL, adapter_nr, NULL) || 0 == dvb_usb_device_init(intf, &t220_properties, - THIS_MODULE, NULL, adapter_nr) || + THIS_MODULE, NULL, adapter_nr, NULL) || 0 == dvb_usb_device_init(intf, &tt_s2_4600_properties, - THIS_MODULE, NULL, adapter_nr)) + THIS_MODULE, NULL, adapter_nr, NULL)) return 0; return -ENODEV; diff --git a/drivers/media/usb/dvb-usb/friio.c b/drivers/media/usb/dvb-usb/friio.c index 474a17e4db0c..3854ac7434ad 100644 --- a/drivers/media/usb/dvb-usb/friio.c +++ b/drivers/media/usb/dvb-usb/friio.c @@ -437,7 +437,7 @@ static int friio_probe(struct usb_interface *intf, } ret = dvb_usb_device_init(intf, &friio_properties, - THIS_MODULE, &d, adapter_nr); + THIS_MODULE, &d, adapter_nr, NULL); if (ret == 0) friio_streaming_ctrl(&d->adapter[0], 1); diff --git a/drivers/media/usb/dvb-usb/gp8psk.c b/drivers/media/usb/dvb-usb/gp8psk.c index 2829e3082d15..cede0d8b0f8a 100644 --- a/drivers/media/usb/dvb-usb/gp8psk.c +++ b/drivers/media/usb/dvb-usb/gp8psk.c @@ -262,7 +262,7 @@ static int gp8psk_usb_probe(struct usb_interface *intf, int ret; struct usb_device *udev = interface_to_usbdev(intf); ret = dvb_usb_device_init(intf, &gp8psk_properties, - THIS_MODULE, NULL, adapter_nr); + THIS_MODULE, NULL, adapter_nr, NULL); if (ret == 0) { info("found Genpix USB device pID = %x (hex)", le16_to_cpu(udev->descriptor.idProduct)); diff --git a/drivers/media/usb/dvb-usb/m920x.c b/drivers/media/usb/dvb-usb/m920x.c index eafc5c82467f..b4c83f36abee 100644 --- a/drivers/media/usb/dvb-usb/m920x.c +++ b/drivers/media/usb/dvb-usb/m920x.c @@ -835,14 +835,14 @@ static int m920x_probe(struct usb_interface *intf, */ ret = dvb_usb_device_init(intf, &megasky_properties, - THIS_MODULE, &d, adapter_nr); + THIS_MODULE, &d, adapter_nr, NULL); if (ret == 0) { rc_init_seq = megasky_rc_init; goto found; } ret = dvb_usb_device_init(intf, &digivox_mini_ii_properties, - THIS_MODULE, &d, adapter_nr); + THIS_MODULE, &d, adapter_nr, NULL); if (ret == 0) { /* No remote control, so no rc_init_seq */ goto found; @@ -850,28 +850,28 @@ static int m920x_probe(struct usb_interface *intf, /* This configures both tuners on the TV Walker Twin */ ret = dvb_usb_device_init(intf, &tvwalkertwin_properties, - THIS_MODULE, &d, adapter_nr); + THIS_MODULE, &d, adapter_nr, NULL); if (ret == 0) { rc_init_seq = tvwalkertwin_rc_init; goto found; } ret = dvb_usb_device_init(intf, &dposh_properties, - THIS_MODULE, &d, adapter_nr); + THIS_MODULE, &d, adapter_nr, NULL); if (ret == 0) { /* Remote controller not supported yet. */ goto found; } ret = dvb_usb_device_init(intf, &pinnacle_pctv310e_properties, - THIS_MODULE, &d, adapter_nr); + THIS_MODULE, &d, adapter_nr, NULL); if (ret == 0) { rc_init_seq = pinnacle310e_init; goto found; } ret = dvb_usb_device_init(intf, &vp7049_properties, - THIS_MODULE, &d, adapter_nr); + THIS_MODULE, &d, adapter_nr, NULL); if (ret == 0) { rc_init_seq = vp7049_rc_init; goto found; diff --git a/drivers/media/usb/dvb-usb/nova-t-usb2.c b/drivers/media/usb/dvb-usb/nova-t-usb2.c index 1babd3341910..0d8f06430ca5 100644 --- a/drivers/media/usb/dvb-usb/nova-t-usb2.c +++ b/drivers/media/usb/dvb-usb/nova-t-usb2.c @@ -157,7 +157,7 @@ static int nova_t_probe(struct usb_interface *intf, const struct usb_device_id *id) { return dvb_usb_device_init(intf, &nova_t_properties, - THIS_MODULE, NULL, adapter_nr); + THIS_MODULE, NULL, adapter_nr, NULL); } /* do not change the order of the ID table */ diff --git a/drivers/media/usb/dvb-usb/opera1.c b/drivers/media/usb/dvb-usb/opera1.c index 2566d2f1c2ad..9f2f156e2939 100644 --- a/drivers/media/usb/dvb-usb/opera1.c +++ b/drivers/media/usb/dvb-usb/opera1.c @@ -563,7 +563,7 @@ static int opera1_probe(struct usb_interface *intf, } if (0 != dvb_usb_device_init(intf, &opera1_properties, - THIS_MODULE, NULL, adapter_nr)) + THIS_MODULE, NULL, adapter_nr, NULL)) return -EINVAL; return 0; } diff --git a/drivers/media/usb/dvb-usb/pctv452e.c b/drivers/media/usb/dvb-usb/pctv452e.c index 07fa08be9e99..2f844ff39840 100644 --- a/drivers/media/usb/dvb-usb/pctv452e.c +++ b/drivers/media/usb/dvb-usb/pctv452e.c @@ -1059,9 +1059,9 @@ static int pctv452e_usb_probe(struct usb_interface *intf, const struct usb_device_id *id) { if (0 == dvb_usb_device_init(intf, &pctv452e_properties, - THIS_MODULE, NULL, adapter_nr) || + THIS_MODULE, NULL, adapter_nr, NULL) || 0 == dvb_usb_device_init(intf, &tt_connect_s2_3600_properties, - THIS_MODULE, NULL, adapter_nr)) + THIS_MODULE, NULL, adapter_nr, NULL)) return 0; return -ENODEV; diff --git a/drivers/media/usb/dvb-usb/technisat-usb2.c b/drivers/media/usb/dvb-usb/technisat-usb2.c index 4706628a3ed5..80fc5043e4cf 100644 --- a/drivers/media/usb/dvb-usb/technisat-usb2.c +++ b/drivers/media/usb/dvb-usb/technisat-usb2.c @@ -765,7 +765,7 @@ static int technisat_usb2_probe(struct usb_interface *intf, struct dvb_usb_device *dev; if (dvb_usb_device_init(intf, &technisat_usb2_devices, THIS_MODULE, - &dev, adapter_nr) != 0) + &dev, adapter_nr, NULL) != 0) return -ENODEV; if (dev) { diff --git a/drivers/media/usb/dvb-usb/ttusb2.c b/drivers/media/usb/dvb-usb/ttusb2.c index ecc207fbaf3c..192057ea58c0 100644 --- a/drivers/media/usb/dvb-usb/ttusb2.c +++ b/drivers/media/usb/dvb-usb/ttusb2.c @@ -604,11 +604,11 @@ static int ttusb2_probe(struct usb_interface *intf, const struct usb_device_id *id) { if (0 == dvb_usb_device_init(intf, &ttusb2_properties, - THIS_MODULE, NULL, adapter_nr) || + THIS_MODULE, NULL, adapter_nr, NULL) || 0 == dvb_usb_device_init(intf, &ttusb2_properties_s2400, - THIS_MODULE, NULL, adapter_nr) || + THIS_MODULE, NULL, adapter_nr, NULL) || 0 == dvb_usb_device_init(intf, &ttusb2_properties_ct3650, - THIS_MODULE, NULL, adapter_nr)) + THIS_MODULE, NULL, adapter_nr, NULL)) return 0; return -ENODEV; } diff --git a/drivers/media/usb/dvb-usb/umt-010.c b/drivers/media/usb/dvb-usb/umt-010.c index 58ad5b4f856c..9af8cca5f2d2 100644 --- a/drivers/media/usb/dvb-usb/umt-010.c +++ b/drivers/media/usb/dvb-usb/umt-010.c @@ -78,7 +78,7 @@ static int umt_probe(struct usb_interface *intf, const struct usb_device_id *id) { if (0 == dvb_usb_device_init(intf, &umt_properties, - THIS_MODULE, NULL, adapter_nr)) + THIS_MODULE, NULL, adapter_nr, NULL)) return 0; return -EINVAL; } diff --git a/drivers/media/usb/dvb-usb/vp702x.c b/drivers/media/usb/dvb-usb/vp702x.c index 40de33de90a7..e95a84e3f9de 100644 --- a/drivers/media/usb/dvb-usb/vp702x.c +++ b/drivers/media/usb/dvb-usb/vp702x.c @@ -337,7 +337,7 @@ static int vp702x_usb_probe(struct usb_interface *intf, int ret; ret = dvb_usb_device_init(intf, &vp702x_properties, - THIS_MODULE, &d, adapter_nr); + THIS_MODULE, &d, adapter_nr, NULL); if (ret) goto out; diff --git a/drivers/media/usb/dvb-usb/vp7045.c b/drivers/media/usb/dvb-usb/vp7045.c index 13340af0d39c..350603bbe3da 100644 --- a/drivers/media/usb/dvb-usb/vp7045.c +++ b/drivers/media/usb/dvb-usb/vp7045.c @@ -225,7 +225,7 @@ static int vp7045_usb_probe(struct usb_interface *intf, const struct usb_device_id *id) { return dvb_usb_device_init(intf, &vp7045_properties, - THIS_MODULE, NULL, adapter_nr); + THIS_MODULE, NULL, adapter_nr, NULL); } static struct usb_device_id vp7045_usb_table [] = { ^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [v4.9-rc4] dvb-usb/cinergyT2 NULL pointer dereference 2016-11-08 20:22 ` Mauro Carvalho Chehab @ 2016-11-08 21:15 ` Benjamin Larsson 2016-11-08 21:38 ` Mauro Carvalho Chehab 2016-11-09 11:09 ` Jörg Otte 1 sibling, 1 reply; 14+ messages in thread From: Benjamin Larsson @ 2016-11-08 21:15 UTC (permalink / raw) To: Mauro Carvalho Chehab, Linus Torvalds Cc: Jörg Otte, Patrick Boettcher, Mauro Carvalho Chehab, Linux Kernel Mailing List, Linux Media Mailing List On 11/08/2016 09:22 PM, Mauro Carvalho Chehab wrote: > Em Tue, 8 Nov 2016 10:42:03 -0800 > Linus Torvalds <torvalds@linux-foundation.org> escreveu: > >> On Sun, Nov 6, 2016 at 7:40 AM, Jörg Otte <jrg.otte@gmail.com> wrote: >>> Since v4.9-rc4 I get following crash in dvb-usb-cinergyT2 module. >> >> Looks like it's commit 5ef8ed0e5608f ("[media] cinergyT2-core: don't >> do DMA on stack"), which movced the DMA data array from the stack to >> the "private" pointer. In the process it also added serialization in >> the form of "data_mutex", but and now it oopses on that mutex because >> the private pointer is NULL. >> >> It looks like the "->private" pointer is allocated in dvb_usb_adapter_init() >> >> cinergyt2_usb_probe -> >> dvb_usb_device_init -> >> dvb_usb_init() -> >> dvb_usb_adapter_init() >> >> but the dvb_usb_init() function calls dvb_usb_device_power_ctrl() >> (which calls the "power_ctrl" function, which is >> cinergyt2_power_ctrl() for that drive) *before* it initializes the >> private field. >> >> Mauro, Patrick, could dvb_usb_adapter_init() be called earlier, perhaps? > > Calling it earlier won't work, as we need to load the firmware before > sending the power control commands on some devices. > > Probably the best here is to pass an extra optional function parameter > that will initialize the mutex before calling any functions. > > Btw, if it broke here, the DMA fixes will likely break on other drivers. > So, after Jörg tests this patch, I'll work on a patch series addressing > this issue on the other drivers I touched. > > Regards, > Mauro Just for reference I got the following call trace a week ago. I looks like this confirms that other drivers are affected also. MvH Benjamin Larsson dvb-usb: found a 'Mygica T230 DVB-T/T2/C' in warm state. BUG: unable to handle kernel NULL pointer dereference at (null) IP: [<ffffffff8182f688>] __mutex_lock_slowpath+0x98/0x130 PGD 0 Oops: 0002 [#1] SMP Modules linked in: dvb_usb_cxusb(OE+) dib0070(OE) dvb_usb_rtl28xxu(OE+) dvb_usb_dw2102(OE+) dvb_usb(OE) dvb_usb_v2(OE) dvb_core(OE) rc_core(OE) ipmi_ssif media(OE) lpc_sch joydev input_leds i2c_ismt ipmi_si 8250_fintek ipmi_msghandler shpchp mac_hid kvm_intel kvm irqbypass ib_iser rdma_cm iw_cm ib_cm ib_sa ib_mad ib_core ib_addr iscsi_tcp libiscsi_tcp libiscsi scsi_transport_iscsi autofs4 btrfs raid10 multipath linear raid456 async_raid6_recov async_memcpy async_pq async_xor async_tx xor raid6_pq libcrc32c raid0 raid1 hid_generic igb dca usbhid ptp pps_core i2c_algo_bit hid ahci libahci fjes CPU: 3 PID: 571 Comm: systemd-udevd Tainted: G OE 4.4.0-45-generic #66-Ubuntu Hardware name: Supermicro X9SBAA/X9SBAA, BIOS 1.00 04/29/2014 task: ffff880231678000 ti: ffff880232048000 task.ti: ffff880232048000 RIP: 0010:[<ffffffff8182f688>] [<ffffffff8182f688>] __mutex_lock_slowpath+0x98/0x130 RSP: 0018:ffff88023204b920 EFLAGS: 00010282 RAX: 0000000000000000 RBX: ffff88023152fea8 RCX: 0000000000000000 RDX: 0000000000000001 RSI: ffff880231678000 RDI: ffff88023152feac RBP: ffff88023204b970 R08: ffff880232048000 R09: 0000000000000000 R10: 0000000000000325 R11: 0000000000000000 R12: ffff88023152feac R13: ffff880231678000 R14: 00000000ffffffff R15: ffff88023152feb0 FS: 00007fc5390d68c0(0000) GS:ffff88023fd80000(0000) knlGS:0000000000000000 CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 CR2: 0000000000000000 CR3: 000000023262f000 CR4: 00000000000006e0 Stack: ffff88023152feb0 0000000000000000 ffff88023204b978 0000000027226a0c 01ff880200000000 ffff88023152fea8 ffff88023152fe40 ffff88023152fea8 ffff8800ba9e4000 0000000000000000 ffff88023204b988 ffffffff8182f73f Call Trace: [<ffffffff8182f73f>] mutex_lock+0x1f/0x30 [<ffffffffc04793da>] cxusb_ctrl_msg+0x5a/0xe0 [dvb_usb_cxusb] [<ffffffffc0479ac8>] cxusb_power_ctrl+0x58/0x60 [dvb_usb_cxusb] [<ffffffffc047b0db>] cxusb_d680_dmb_power_ctrl+0x2b/0x90 [dvb_usb_cxusb] [<ffffffffc050e5a1>] dvb_usb_device_power_ctrl.part.4+0x31/0x50 [dvb_usb] [<ffffffffc050e5ea>] dvb_usb_device_power_ctrl+0x2a/0x40 [dvb_usb] [<ffffffffc050e844>] dvb_usb_device_init+0x244/0x750 [dvb_usb] [<ffffffff8128baca>] ? kernfs_activate+0x7a/0xe0 [<ffffffffc0479360>] cxusb_probe+0x260/0x280 [dvb_usb_cxusb] [<ffffffff8161e858>] usb_probe_interface+0x118/0x2f0 [<ffffffff815550d2>] driver_probe_device+0x222/0x4a0 [<ffffffff815553d4>] __driver_attach+0x84/0x90 [<ffffffff81555350>] ? driver_probe_device+0x4a0/0x4a0 [<ffffffff81552cfc>] bus_for_each_dev+0x6c/0xc0 [<ffffffff8155488e>] driver_attach+0x1e/0x20 [<ffffffff815543cb>] bus_add_driver+0x1eb/0x280 [<ffffffff81555ce0>] driver_register+0x60/0xe0 [<ffffffff8161d224>] usb_register_driver+0x84/0x140 [<ffffffffc03d8000>] ? 0xffffffffc03d8000 [<ffffffffc03d801e>] cxusb_driver_init+0x1e/0x1000 [dvb_usb_cxusb] [<ffffffff81002123>] do_one_initcall+0xb3/0x200 [<ffffffff811cfa51>] ? __vunmap+0x91/0xe0 [<ffffffff811ebc03>] ? kmem_cache_alloc_trace+0x183/0x1f0 [<ffffffff811ec9fa>] ? kfree+0x13a/0x150 [<ffffffff8118caa3>] do_init_module+0x5f/0x1cf [<ffffffff8110a2ff>] load_module+0x166f/0x1c10 [<ffffffff811068a0>] ? __symbol_put+0x60/0x60 [<ffffffff81213760>] ? kernel_read+0x50/0x80 [<ffffffff8110aae4>] SYSC_finit_module+0xb4/0xe0 [<ffffffff8110ab2e>] SyS_finit_module+0xe/0x10 [<ffffffff818318b2>] entry_SYSCALL_64_fastpath+0x16/0x71 Code: e8 ae 1f 00 00 8b 03 83 f8 01 0f 84 94 00 00 00 48 8b 43 10 4c 8d 7b 08 48 89 63 10 41 be ff ff ff ff 4c 89 3c 24 48 89 44 24 08 <48> 89 20 4c 89 6c 24 10 eb 1f 49 c7 45 00 02 00 00 00 4c 89 e7 RIP [<ffffffff8182f688>] __mutex_lock_slowpath+0x98/0x130 RSP <ffff88023204b920> CR2: 0000000000000000 ---[ end trace b28b9190ee8e4917 ]--- ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [v4.9-rc4] dvb-usb/cinergyT2 NULL pointer dereference 2016-11-08 21:15 ` Benjamin Larsson @ 2016-11-08 21:38 ` Mauro Carvalho Chehab 2016-11-09 19:57 ` Malcolm Priestley 0 siblings, 1 reply; 14+ messages in thread From: Mauro Carvalho Chehab @ 2016-11-08 21:38 UTC (permalink / raw) To: Benjamin Larsson Cc: Linus Torvalds, Jörg Otte, Patrick Boettcher, Mauro Carvalho Chehab, Linux Kernel Mailing List, Linux Media Mailing List Em Tue, 8 Nov 2016 22:15:24 +0100 Benjamin Larsson <benjamin@southpole.se> escreveu: > On 11/08/2016 09:22 PM, Mauro Carvalho Chehab wrote: > > Em Tue, 8 Nov 2016 10:42:03 -0800 > > Linus Torvalds <torvalds@linux-foundation.org> escreveu: > > > >> On Sun, Nov 6, 2016 at 7:40 AM, Jörg Otte <jrg.otte@gmail.com> wrote: > >>> Since v4.9-rc4 I get following crash in dvb-usb-cinergyT2 module. > >> > >> Looks like it's commit 5ef8ed0e5608f ("[media] cinergyT2-core: don't > >> do DMA on stack"), which movced the DMA data array from the stack to > >> the "private" pointer. In the process it also added serialization in > >> the form of "data_mutex", but and now it oopses on that mutex because > >> the private pointer is NULL. > >> > >> It looks like the "->private" pointer is allocated in dvb_usb_adapter_init() > >> > >> cinergyt2_usb_probe -> > >> dvb_usb_device_init -> > >> dvb_usb_init() -> > >> dvb_usb_adapter_init() > >> > >> but the dvb_usb_init() function calls dvb_usb_device_power_ctrl() > >> (which calls the "power_ctrl" function, which is > >> cinergyt2_power_ctrl() for that drive) *before* it initializes the > >> private field. > >> > >> Mauro, Patrick, could dvb_usb_adapter_init() be called earlier, perhaps? > > > > Calling it earlier won't work, as we need to load the firmware before > > sending the power control commands on some devices. > > > > Probably the best here is to pass an extra optional function parameter > > that will initialize the mutex before calling any functions. > > > > Btw, if it broke here, the DMA fixes will likely break on other drivers. > > So, after Jörg tests this patch, I'll work on a patch series addressing > > this issue on the other drivers I touched. > > > > Regards, > > Mauro > > Just for reference I got the following call trace a week ago. I looks > like this confirms that other drivers are affected also. Yeah, I avoided serializing the logic that detects if the firmware is loaded, but forgot that the power control had the same issue. The newer dvb usb drivers use the dvb-usb-v2, so I didn't touch this code for a while. I'll need to review all touched drivers to be sure that they'll all do the right thing. The good news is that it will likely simplify the drivers' logic a little bit. Thanks, Mauro ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [v4.9-rc4] dvb-usb/cinergyT2 NULL pointer dereference 2016-11-08 21:38 ` Mauro Carvalho Chehab @ 2016-11-09 19:57 ` Malcolm Priestley 2016-11-09 20:25 ` Mauro Carvalho Chehab 0 siblings, 1 reply; 14+ messages in thread From: Malcolm Priestley @ 2016-11-09 19:57 UTC (permalink / raw) To: Mauro Carvalho Chehab, Benjamin Larsson Cc: Linus Torvalds, Jörg Otte, Patrick Boettcher, Mauro Carvalho Chehab, Linux Kernel Mailing List, Linux Media Mailing List On 08/11/16 21:38, Mauro Carvalho Chehab wrote: > Em Tue, 8 Nov 2016 22:15:24 +0100 > Benjamin Larsson <benjamin@southpole.se> escreveu: > >> On 11/08/2016 09:22 PM, Mauro Carvalho Chehab wrote: >>> Em Tue, 8 Nov 2016 10:42:03 -0800 >>> Linus Torvalds <torvalds@linux-foundation.org> escreveu: >>> >>>> On Sun, Nov 6, 2016 at 7:40 AM, Jörg Otte <jrg.otte@gmail.com> wrote: >>>>> Since v4.9-rc4 I get following crash in dvb-usb-cinergyT2 module. >>>> >>>> Looks like it's commit 5ef8ed0e5608f ("[media] cinergyT2-core: don't >>>> do DMA on stack"), which movced the DMA data array from the stack to >>>> the "private" pointer. In the process it also added serialization in >>>> the form of "data_mutex", but and now it oopses on that mutex because >>>> the private pointer is NULL. >>>> >>>> It looks like the "->private" pointer is allocated in dvb_usb_adapter_init() >>>> >>>> cinergyt2_usb_probe -> >>>> dvb_usb_device_init -> >>>> dvb_usb_init() -> >>>> dvb_usb_adapter_init() >>>> >>>> but the dvb_usb_init() function calls dvb_usb_device_power_ctrl() >>>> (which calls the "power_ctrl" function, which is >>>> cinergyt2_power_ctrl() for that drive) *before* it initializes the >>>> private field. >>>> >>>> Mauro, Patrick, could dvb_usb_adapter_init() be called earlier, perhaps? >>> >>> Calling it earlier won't work, as we need to load the firmware before >>> sending the power control commands on some devices. >>> >>> Probably the best here is to pass an extra optional function parameter >>> that will initialize the mutex before calling any functions. >>> >>> Btw, if it broke here, the DMA fixes will likely break on other drivers. >>> So, after Jörg tests this patch, I'll work on a patch series addressing >>> this issue on the other drivers I touched. >>> >>> Regards, >>> Mauro >> >> Just for reference I got the following call trace a week ago. I looks >> like this confirms that other drivers are affected also. > > Yeah, I avoided serializing the logic that detects if the firmware is > loaded, but forgot that the power control had the same issue. The > newer dvb usb drivers use the dvb-usb-v2, so I didn't touch this > code for a while. I think the problem is that the usb buffer has been put in struct cinergyt2_state private area which has not been initialized for initial usb probing. That was one of the main reasons for porting drivers to dvb-usb-v2. Regards Malcolm ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [v4.9-rc4] dvb-usb/cinergyT2 NULL pointer dereference 2016-11-09 19:57 ` Malcolm Priestley @ 2016-11-09 20:25 ` Mauro Carvalho Chehab 0 siblings, 0 replies; 14+ messages in thread From: Mauro Carvalho Chehab @ 2016-11-09 20:25 UTC (permalink / raw) To: Malcolm Priestley Cc: Benjamin Larsson, Linus Torvalds, Jörg Otte, Patrick Boettcher, Mauro Carvalho Chehab, Linux Kernel Mailing List, Linux Media Mailing List Em Wed, 9 Nov 2016 19:57:58 +0000 Malcolm Priestley <tvboxspy@gmail.com> escreveu: > > Yeah, I avoided serializing the logic that detects if the firmware is > > loaded, but forgot that the power control had the same issue. The > > newer dvb usb drivers use the dvb-usb-v2, so I didn't touch this > > code for a while. > > I think the problem is that the usb buffer has been put in struct > cinergyt2_state private area which has not been initialized for initial > usb probing. > > That was one of the main reasons for porting drivers to dvb-usb-v2. True, but converting to dvb-usb-v2, is more complex. In particular, dib0700 and dib3000 drivers rely on some things that may not be ported to dvb-usb-v2. So, I don't think we should do such change during the -rc cycle. Also, such change requires testing. So, one with those hardware should help with it, or the developer willing to do the conversion would need to get those old hardware in hands. Thanks, Mauro ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [v4.9-rc4] dvb-usb/cinergyT2 NULL pointer dereference 2016-11-08 20:22 ` Mauro Carvalho Chehab 2016-11-08 21:15 ` Benjamin Larsson @ 2016-11-09 11:09 ` Jörg Otte 2016-11-09 19:07 ` Linus Torvalds 1 sibling, 1 reply; 14+ messages in thread From: Jörg Otte @ 2016-11-09 11:09 UTC (permalink / raw) To: Mauro Carvalho Chehab Cc: Linus Torvalds, Patrick Boettcher, Mauro Carvalho Chehab, Linux Kernel Mailing List, Linux Media Mailing List 2016-11-08 21:22 GMT+01:00 Mauro Carvalho Chehab <mchehab@s-opensource.com>: > Em Tue, 8 Nov 2016 10:42:03 -0800 > Linus Torvalds <torvalds@linux-foundation.org> escreveu: > >> On Sun, Nov 6, 2016 at 7:40 AM, Jörg Otte <jrg.otte@gmail.com> wrote: >> > Since v4.9-rc4 I get following crash in dvb-usb-cinergyT2 module. >> >> Looks like it's commit 5ef8ed0e5608f ("[media] cinergyT2-core: don't >> do DMA on stack"), which movced the DMA data array from the stack to >> the "private" pointer. In the process it also added serialization in >> the form of "data_mutex", but and now it oopses on that mutex because >> the private pointer is NULL. >> >> It looks like the "->private" pointer is allocated in dvb_usb_adapter_init() >> >> cinergyt2_usb_probe -> >> dvb_usb_device_init -> >> dvb_usb_init() -> >> dvb_usb_adapter_init() >> >> but the dvb_usb_init() function calls dvb_usb_device_power_ctrl() >> (which calls the "power_ctrl" function, which is >> cinergyt2_power_ctrl() for that drive) *before* it initializes the >> private field. >> >> Mauro, Patrick, could dvb_usb_adapter_init() be called earlier, perhaps? > > Calling it earlier won't work, as we need to load the firmware before > sending the power control commands on some devices. > > Probably the best here is to pass an extra optional function parameter > that will initialize the mutex before calling any functions. > > Btw, if it broke here, the DMA fixes will likely break on other drivers. > So, after Jörg tests this patch, I'll work on a patch series addressing > this issue on the other drivers I touched. > > Regards, > Mauro > > - > > [PATCH RFC] cinergyT2-core: initialize the mutex early > > NOTE: don't merge this patch as-is... I actually folded two patches > together here, in order to make easier to test, but the best is to > place the changes at the core first, and then the changes at the > drivers that would need an early init. > > The mutex used to protect the URB data buffer needs to be > inialized early, as otherwise it will cause an OOPS: > > dvb-usb: found a 'TerraTec/qanu USB2.0 Highspeed DVB-T Receiver' in warm state. > BUG: unable to handle kernel NULL pointer dereference at (null) > IP: [<ffffffff846617af>] __mutex_lock_slowpath+0x6f/0x100 PGD 0 > Oops: 0002 [#1] SMP > Modules linked in: dvb_usb_cinergyT2(+) dvb_usb > CPU: 0 PID: 2029 Comm: modprobe Not tainted 4.9.0-rc4-dvbmod #24 > Hardware name: FUJITSU LIFEBOOK A544/FJNBB35 , BIOS Version 1.17 05/09/2014 > task: ffff88020e943840 task.stack: ffff8801f36ec000 > RIP: 0010:[<ffffffff846617af>] [<ffffffff846617af>] __mutex_lock_slowpath+0x6f/0x100 > RSP: 0018:ffff8801f36efb10 EFLAGS: 00010282 > RAX: 0000000000000000 RBX: ffff88021509bdc8 RCX: 00000000c0000100 > RDX: 0000000000000001 RSI: 0000000000000000 RDI: ffff88021509bdcc > RBP: ffff8801f36efb58 R08: ffff88021f216320 R09: 0000000000100000 > R10: ffff88021f216320 R11: 00000023fee6c5a1 R12: ffff88020e943840 > R13: ffff88021509bdcc R14: 00000000ffffffff R15: ffff88021509bdd0 > FS: 00007f21adb86740(0000) GS:ffff88021f200000(0000) knlGS:0000000000000000 > CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 > CR2: 0000000000000000 CR3: 0000000215bce000 CR4: 00000000001406f0 > Stack: > ffff88021509bdd0 0000000000000000 0000000000000000 ffffffffc0137c80 > ffff88021509bdc8 ffff8801f5944000 0000000000000001 ffffffffc0136b00 > ffff880213e52000 ffff88021509bdc8 ffffffff84661856 ffff88021509bd80 > Call Trace: > [<ffffffff84661856>] ? mutex_lock+0x16/0x25 > [<ffffffffc013616f>] ? cinergyt2_power_ctrl+0x1f/0x60 [dvb_usb_cinergyT2] > [<ffffffffc012e67e>] ? dvb_usb_device_init+0x21e/0x5d0 [dvb_usb] > [<ffffffffc0136021>] ? cinergyt2_usb_probe+0x21/0x50 [dvb_usb_cinergyT2] > [<ffffffff844326f3>] ? usb_probe_interface+0xf3/0x2a0 > [<ffffffff8438e348>] ? driver_probe_device+0x208/0x2b0 > [<ffffffff8438e477>] ? __driver_attach+0x87/0x90 > [<ffffffff8438e3f0>] ? driver_probe_device+0x2b0/0x2b0 > [<ffffffff8438c612>] ? bus_for_each_dev+0x52/0x80 > [<ffffffff8438d983>] ? bus_add_driver+0x1a3/0x220 > [<ffffffff8438ec06>] ? driver_register+0x56/0xd0 > [<ffffffff84431527>] ? usb_register_driver+0x77/0x130 > [<ffffffffc013a000>] ? 0xffffffffc013a000 > [<ffffffff84000426>] ? do_one_initcall+0x46/0x180 > [<ffffffff840eb2c8>] ? free_vmap_area_noflush+0x38/0x70 > [<ffffffff840f3844>] ? kmem_cache_alloc+0x84/0xc0 > [<ffffffff840b802c>] ? do_init_module+0x50/0x1be > [<ffffffff84095adb>] ? load_module+0x1d8b/0x2100 > [<ffffffff84093020>] ? find_symbol_in_section+0xa0/0xa0 > [<ffffffff84095fe9>] ? SyS_finit_module+0x89/0x90 > [<ffffffff846637a0>] ? entry_SYSCALL_64_fastpath+0x13/0x94 > Code: e8 a7 1d 00 00 8b 03 83 f8 01 0f 84 97 00 00 00 48 8b 43 10 4c 8d 7b 08 48 89 63 10 4c 89 3c 24 41 be ff ff ff ff 48 89 44 24 08 <48> 89 20 4c 89 64 24 10 eb 1a 49 c7 44 24 08 02 00 00 00 c6 43 RIP [<ffffffff846617af>] __mutex_lock_slowpath+0x6f/0x100 RSP <ffff8801f36efb10> > CR2: 0000000000000000 > > Reported-by: Jörg Otte <jrg.otte@gmail.com> > Fixes: 6679a901c380 ("[media] cinergyT2-core: don't do DMA on stack") > Signed-off-by: Mauro Carvalho Chehab <mchehab@s-opensource.com> > > From cbc7e48a86e8ffd16e49b10061120dfc417397f8 Mon Sep 17 00:00:00 2001 > From: Mauro Carvalho Chehab <mchehab@s-opensource.com> > Date: Tue, 8 Nov 2016 18:04:24 -0200 > Subject: [PATCH] [media] dvb-usb: allow early initialization of usb device > priv data > > On some drivers, we need to initialize a mutex before calling > power control or firmware download routines. Add an extra > parameter to allow it. > > Signed-off-by: Mauro Carvalho Chehab <mchehab@s-opensource.com> > > diff --git a/drivers/media/usb/dvb-usb/a800.c b/drivers/media/usb/dvb-usb/a800.c > index 7ba975bea96a..1e14f79aa57a 100644 > --- a/drivers/media/usb/dvb-usb/a800.c > +++ b/drivers/media/usb/dvb-usb/a800.c > @@ -107,7 +107,7 @@ static int a800_probe(struct usb_interface *intf, > const struct usb_device_id *id) > { > return dvb_usb_device_init(intf, &a800_properties, > - THIS_MODULE, NULL, adapter_nr); > + THIS_MODULE, NULL, adapter_nr, NULL); > } > > /* do not change the order of the ID table */ > diff --git a/drivers/media/usb/dvb-usb/af9005.c b/drivers/media/usb/dvb-usb/af9005.c > index b257780fb380..1625b4714d83 100644 > --- a/drivers/media/usb/dvb-usb/af9005.c > +++ b/drivers/media/usb/dvb-usb/af9005.c > @@ -1009,7 +1009,7 @@ static int af9005_usb_probe(struct usb_interface *intf, > int ret; > > ret = dvb_usb_device_init(intf, &af9005_properties, > - THIS_MODULE, &d, adapter_nr); > + THIS_MODULE, &d, adapter_nr, NULL); > > if (ret < 0) > return ret; > diff --git a/drivers/media/usb/dvb-usb/az6027.c b/drivers/media/usb/dvb-usb/az6027.c > index 2e711362847e..e8f73a96efd9 100644 > --- a/drivers/media/usb/dvb-usb/az6027.c > +++ b/drivers/media/usb/dvb-usb/az6027.c > @@ -946,7 +946,7 @@ static int az6027_usb_probe(struct usb_interface *intf, > &az6027_properties, > THIS_MODULE, > NULL, > - adapter_nr); > + adapter_nr, NULL); > } > > /* I2C */ > diff --git a/drivers/media/usb/dvb-usb/cinergyT2-core.c b/drivers/media/usb/dvb-usb/cinergyT2-core.c > index 8ac825413d5a..2aa99c52e39d 100644 > --- a/drivers/media/usb/dvb-usb/cinergyT2-core.c > +++ b/drivers/media/usb/dvb-usb/cinergyT2-core.c > @@ -206,24 +206,21 @@ static int cinergyt2_rc_query(struct dvb_usb_device *d, u32 *event, int *state) > return ret; > } > > -static int cinergyt2_usb_probe(struct usb_interface *intf, > - const struct usb_device_id *id) > +static int cinergyT2_init_mutex(struct dvb_usb_device *d) > { > - struct dvb_usb_device *d; > - struct cinergyt2_state *st; > - int ret; > - > - ret = dvb_usb_device_init(intf, &cinergyt2_properties, > - THIS_MODULE, &d, adapter_nr); > - if (ret < 0) > - return ret; > + struct cinergyt2_state *st = d->priv; > > - st = d->priv; > mutex_init(&st->data_mutex); > - > return 0; > } > > +static int cinergyt2_usb_probe(struct usb_interface *intf, > + const struct usb_device_id *id) > +{ > + return dvb_usb_device_init(intf, &cinergyt2_properties, > + THIS_MODULE, NULL, adapter_nr, > + cinergyT2_init_mutex); > +} > > static struct usb_device_id cinergyt2_usb_table[] = { > { USB_DEVICE(USB_VID_TERRATEC, 0x0038) }, > diff --git a/drivers/media/usb/dvb-usb/cxusb.c b/drivers/media/usb/dvb-usb/cxusb.c > index 39772812269d..73c1a8568b55 100644 > --- a/drivers/media/usb/dvb-usb/cxusb.c > +++ b/drivers/media/usb/dvb-usb/cxusb.c > @@ -1465,33 +1465,33 @@ static int cxusb_probe(struct usb_interface *intf, > struct cxusb_state *st; > > if (0 == dvb_usb_device_init(intf, &cxusb_medion_properties, > - THIS_MODULE, &d, adapter_nr) || > + THIS_MODULE, &d, adapter_nr, NULL) || > 0 == dvb_usb_device_init(intf, &cxusb_bluebird_lgh064f_properties, > - THIS_MODULE, &d, adapter_nr) || > + THIS_MODULE, &d, adapter_nr, NULL) || > 0 == dvb_usb_device_init(intf, &cxusb_bluebird_dee1601_properties, > - THIS_MODULE, &d, adapter_nr) || > + THIS_MODULE, &d, adapter_nr, NULL) || > 0 == dvb_usb_device_init(intf, &cxusb_bluebird_lgz201_properties, > - THIS_MODULE, &d, adapter_nr) || > + THIS_MODULE, &d, adapter_nr, NULL) || > 0 == dvb_usb_device_init(intf, &cxusb_bluebird_dtt7579_properties, > - THIS_MODULE, &d, adapter_nr) || > + THIS_MODULE, &d, adapter_nr, NULL) || > 0 == dvb_usb_device_init(intf, &cxusb_bluebird_dualdig4_properties, > - THIS_MODULE, &d, adapter_nr) || > + THIS_MODULE, &d, adapter_nr, NULL) || > 0 == dvb_usb_device_init(intf, &cxusb_bluebird_nano2_properties, > - THIS_MODULE, &d, adapter_nr) || > + THIS_MODULE, &d, adapter_nr, NULL) || > 0 == dvb_usb_device_init(intf, > &cxusb_bluebird_nano2_needsfirmware_properties, > - THIS_MODULE, &d, adapter_nr) || > + THIS_MODULE, &d, adapter_nr, NULL) || > 0 == dvb_usb_device_init(intf, &cxusb_aver_a868r_properties, > - THIS_MODULE, &d, adapter_nr) || > + THIS_MODULE, &d, adapter_nr, NULL) || > 0 == dvb_usb_device_init(intf, > &cxusb_bluebird_dualdig4_rev2_properties, > - THIS_MODULE, &d, adapter_nr) || > + THIS_MODULE, &d, adapter_nr, NULL) || > 0 == dvb_usb_device_init(intf, &cxusb_d680_dmb_properties, > - THIS_MODULE, &d, adapter_nr) || > + THIS_MODULE, &d, adapter_nr, NULL) || > 0 == dvb_usb_device_init(intf, &cxusb_mygica_d689_properties, > - THIS_MODULE, &d, adapter_nr) || > + THIS_MODULE, &d, adapter_nr, NULL) || > 0 == dvb_usb_device_init(intf, &cxusb_mygica_t230_properties, > - THIS_MODULE, &d, adapter_nr) || > + THIS_MODULE, &d, adapter_nr, NULL) || > 0) { > st = d->priv; > mutex_init(&st->data_mutex); > diff --git a/drivers/media/usb/dvb-usb/dib0700_core.c b/drivers/media/usb/dvb-usb/dib0700_core.c > index 92d5408684ac..3423b56c92b3 100644 > --- a/drivers/media/usb/dvb-usb/dib0700_core.c > +++ b/drivers/media/usb/dvb-usb/dib0700_core.c > @@ -871,7 +871,7 @@ static int dib0700_probe(struct usb_interface *intf, > > for (i = 0; i < dib0700_device_count; i++) > if (dvb_usb_device_init(intf, &dib0700_devices[i], THIS_MODULE, > - &dev, adapter_nr) == 0) { > + &dev, adapter_nr, NULL) == 0) { > struct dib0700_state *st = dev->priv; > u32 hwversion, romversion, fw_version, fwtype; > > diff --git a/drivers/media/usb/dvb-usb/dibusb-mb.c b/drivers/media/usb/dvb-usb/dibusb-mb.c > index a0057641cc86..de4ffe81e8d7 100644 > --- a/drivers/media/usb/dvb-usb/dibusb-mb.c > +++ b/drivers/media/usb/dvb-usb/dibusb-mb.c > @@ -111,13 +111,13 @@ static int dibusb_probe(struct usb_interface *intf, > const struct usb_device_id *id) > { > if (0 == dvb_usb_device_init(intf, &dibusb1_1_properties, > - THIS_MODULE, NULL, adapter_nr) || > + THIS_MODULE, NULL, adapter_nr, NULL) || > 0 == dvb_usb_device_init(intf, &dibusb1_1_an2235_properties, > - THIS_MODULE, NULL, adapter_nr) || > + THIS_MODULE, NULL, adapter_nr, NULL) || > 0 == dvb_usb_device_init(intf, &dibusb2_0b_properties, > - THIS_MODULE, NULL, adapter_nr) || > + THIS_MODULE, NULL, adapter_nr, NULL) || > 0 == dvb_usb_device_init(intf, &artec_t1_usb2_properties, > - THIS_MODULE, NULL, adapter_nr)) > + THIS_MODULE, NULL, adapter_nr, NULL)) > return 0; > > return -EINVAL; > diff --git a/drivers/media/usb/dvb-usb/dibusb-mc.c b/drivers/media/usb/dvb-usb/dibusb-mc.c > index 08fb8a3f6e0c..d50731bb5372 100644 > --- a/drivers/media/usb/dvb-usb/dibusb-mc.c > +++ b/drivers/media/usb/dvb-usb/dibusb-mc.c > @@ -23,7 +23,7 @@ static int dibusb_mc_probe(struct usb_interface *intf, > const struct usb_device_id *id) > { > return dvb_usb_device_init(intf, &dibusb_mc_properties, THIS_MODULE, > - NULL, adapter_nr); > + NULL, adapter_nr, NULL); > } > > /* do not change the order of the ID table */ > diff --git a/drivers/media/usb/dvb-usb/digitv.c b/drivers/media/usb/dvb-usb/digitv.c > index 4284f6984dc1..3dad2293e598 100644 > --- a/drivers/media/usb/dvb-usb/digitv.c > +++ b/drivers/media/usb/dvb-usb/digitv.c > @@ -269,7 +269,7 @@ static int digitv_probe(struct usb_interface *intf, > { > struct dvb_usb_device *d; > int ret = dvb_usb_device_init(intf, &digitv_properties, THIS_MODULE, &d, > - adapter_nr); > + adapter_nr, NULL); > if (ret == 0) { > u8 b[4] = { 0 }; > > diff --git a/drivers/media/usb/dvb-usb/dtt200u.c b/drivers/media/usb/dvb-usb/dtt200u.c > index f88572c7ae7c..3255fee31433 100644 > --- a/drivers/media/usb/dvb-usb/dtt200u.c > +++ b/drivers/media/usb/dvb-usb/dtt200u.c > @@ -149,15 +149,15 @@ static int dtt200u_usb_probe(struct usb_interface *intf, > struct dtt200u_state *st; > > if (0 == dvb_usb_device_init(intf, &dtt200u_properties, > - THIS_MODULE, &d, adapter_nr) || > + THIS_MODULE, &d, adapter_nr, NULL) || > 0 == dvb_usb_device_init(intf, &wt220u_properties, > - THIS_MODULE, &d, adapter_nr) || > + THIS_MODULE, &d, adapter_nr, NULL) || > 0 == dvb_usb_device_init(intf, &wt220u_fc_properties, > - THIS_MODULE, &d, adapter_nr) || > + THIS_MODULE, &d, adapter_nr, NULL) || > 0 == dvb_usb_device_init(intf, &wt220u_zl0353_properties, > - THIS_MODULE, &d, adapter_nr) || > + THIS_MODULE, &d, adapter_nr, NULL) || > 0 == dvb_usb_device_init(intf, &wt220u_miglia_properties, > - THIS_MODULE, &d, adapter_nr)) { > + THIS_MODULE, &d, adapter_nr, NULL)) { > st = d->priv; > mutex_init(&st->data_mutex); > > diff --git a/drivers/media/usb/dvb-usb/dtv5100.c b/drivers/media/usb/dvb-usb/dtv5100.c > index c60fb54f445f..36b553cb3133 100644 > --- a/drivers/media/usb/dvb-usb/dtv5100.c > +++ b/drivers/media/usb/dvb-usb/dtv5100.c > @@ -165,7 +165,7 @@ static int dtv5100_probe(struct usb_interface *intf, > } > > ret = dvb_usb_device_init(intf, &dtv5100_properties, > - THIS_MODULE, NULL, adapter_nr); > + THIS_MODULE, NULL, adapter_nr, NULL); > if (ret) > return ret; > > diff --git a/drivers/media/usb/dvb-usb/dvb-usb-init.c b/drivers/media/usb/dvb-usb/dvb-usb-init.c > index 3896ba9a4179..66493fb25645 100644 > --- a/drivers/media/usb/dvb-usb/dvb-usb-init.c > +++ b/drivers/media/usb/dvb-usb/dvb-usb-init.c > @@ -138,23 +138,14 @@ static int dvb_usb_exit(struct dvb_usb_device *d) > return 0; > } > > -static int dvb_usb_init(struct dvb_usb_device *d, short *adapter_nums) > +static int dvb_usb_init(struct dvb_usb_device *d, short *adapter_nums, > + int (init_device)(struct dvb_usb_device *d)) > { > int ret = 0; > > mutex_init(&d->usb_mutex); > mutex_init(&d->i2c_mutex); > > - d->state = DVB_USB_STATE_INIT; > - > - if (d->props.size_of_priv > 0) { > - d->priv = kzalloc(d->props.size_of_priv, GFP_KERNEL); > - if (d->priv == NULL) { > - err("no memory for priv in 'struct dvb_usb_device'"); > - return -ENOMEM; > - } > - } > - > /* check the capabilities and set appropriate variables */ > dvb_usb_device_power_ctrl(d, 1); > > @@ -233,7 +224,8 @@ int dvb_usb_device_power_ctrl(struct dvb_usb_device *d, int onoff) > int dvb_usb_device_init(struct usb_interface *intf, > struct dvb_usb_device_properties *props, > struct module *owner, struct dvb_usb_device **du, > - short *adapter_nums) > + short *adapter_nums, > + int (init_device)(struct dvb_usb_device *d)) > { > struct usb_device *udev = interface_to_usbdev(intf); > struct dvb_usb_device *d = NULL; > @@ -249,19 +241,42 @@ int dvb_usb_device_init(struct usb_interface *intf, > return -ENODEV; > } > > - if (cold) { > - info("found a '%s' in cold state, will try to load a firmware", desc->name); > - ret = dvb_usb_download_firmware(udev, props); > - if (!props->no_reconnect || ret != 0) > - return ret; > - } > - > - info("found a '%s' in warm state.", desc->name); > d = kzalloc(sizeof(struct dvb_usb_device), GFP_KERNEL); > if (d == NULL) { > err("no memory for 'struct dvb_usb_device'"); > return -ENOMEM; > } > + d->state = DVB_USB_STATE_INIT; > + > + if (d->props.size_of_priv > 0) { > + d->priv = kzalloc(d->props.size_of_priv, GFP_KERNEL); > + if (d->priv == NULL) { > + err("no memory for priv in 'struct dvb_usb_device'"); > + ret = -ENOMEM; > + goto err; > + } > + } > + > + /* > + * Some drivers may need to early initialize the device private data, > + * for example, when a mutex is serializing URB reads/writes, > + * in order for dvb_usb_device_power_ctrl() or firmware load to work. > + */ > + if (init_device) { > + ret = init_device(d); > + if (ret < 0) > + goto err; > + } > + > + if (cold) { > + info("found a '%s' in cold state, will try to load a firmware", > + desc->name); > + ret = dvb_usb_download_firmware(udev, props); > + if (!props->no_reconnect || ret != 0) > + goto err; > + } else { > + info("found a '%s' in warm state.", desc->name); > + } > > d->udev = udev; > memcpy(&d->props, props, sizeof(struct dvb_usb_device_properties)); > @@ -273,12 +288,17 @@ int dvb_usb_device_init(struct usb_interface *intf, > if (du != NULL) > *du = d; > > - ret = dvb_usb_init(d, adapter_nums); > + ret = dvb_usb_init(d, adapter_nums, init_device); > > - if (ret == 0) > + if (!ret) { > info("%s successfully initialized and connected.", desc->name); > - else > - info("%s error while loading driver (%d)", desc->name, ret); > + return 0; > + } > +err: > + info("%s error while loading driver (%d)", desc->name, ret); > + > + kfree(d->priv); > + kfree(d); > return ret; > } > EXPORT_SYMBOL(dvb_usb_device_init); > diff --git a/drivers/media/usb/dvb-usb/dvb-usb.h b/drivers/media/usb/dvb-usb/dvb-usb.h > index 1448c3d27ea2..02c4dd3c206a 100644 > --- a/drivers/media/usb/dvb-usb/dvb-usb.h > +++ b/drivers/media/usb/dvb-usb/dvb-usb.h > @@ -458,7 +458,9 @@ struct dvb_usb_device { > extern int dvb_usb_device_init(struct usb_interface *, > struct dvb_usb_device_properties *, > struct module *, struct dvb_usb_device **, > - short *adapter_nums); > + short *adapter_nums, > + int (init_device)(struct dvb_usb_device *d)); > + > extern void dvb_usb_device_exit(struct usb_interface *); > > /* the generic read/write method for device control */ > diff --git a/drivers/media/usb/dvb-usb/dw2102.c b/drivers/media/usb/dvb-usb/dw2102.c > index 2c720cb2fb00..1cb8d96f485e 100644 > --- a/drivers/media/usb/dvb-usb/dw2102.c > +++ b/drivers/media/usb/dvb-usb/dw2102.c > @@ -2284,27 +2284,27 @@ static int dw2102_probe(struct usb_interface *intf, > s421->adapter->fe[0].frontend_attach = m88rs2000_frontend_attach; > > if (0 == dvb_usb_device_init(intf, &dw2102_properties, > - THIS_MODULE, NULL, adapter_nr) || > + THIS_MODULE, NULL, adapter_nr, NULL) || > 0 == dvb_usb_device_init(intf, &dw2104_properties, > - THIS_MODULE, NULL, adapter_nr) || > + THIS_MODULE, NULL, adapter_nr, NULL) || > 0 == dvb_usb_device_init(intf, &dw3101_properties, > - THIS_MODULE, NULL, adapter_nr) || > + THIS_MODULE, NULL, adapter_nr, NULL) || > 0 == dvb_usb_device_init(intf, &s6x0_properties, > - THIS_MODULE, NULL, adapter_nr) || > + THIS_MODULE, NULL, adapter_nr, NULL) || > 0 == dvb_usb_device_init(intf, p1100, > - THIS_MODULE, NULL, adapter_nr) || > + THIS_MODULE, NULL, adapter_nr, NULL) || > 0 == dvb_usb_device_init(intf, s660, > - THIS_MODULE, NULL, adapter_nr) || > + THIS_MODULE, NULL, adapter_nr, NULL) || > 0 == dvb_usb_device_init(intf, p7500, > - THIS_MODULE, NULL, adapter_nr) || > + THIS_MODULE, NULL, adapter_nr, NULL) || > 0 == dvb_usb_device_init(intf, s421, > - THIS_MODULE, NULL, adapter_nr) || > + THIS_MODULE, NULL, adapter_nr, NULL) || > 0 == dvb_usb_device_init(intf, &su3000_properties, > - THIS_MODULE, NULL, adapter_nr) || > + THIS_MODULE, NULL, adapter_nr, NULL) || > 0 == dvb_usb_device_init(intf, &t220_properties, > - THIS_MODULE, NULL, adapter_nr) || > + THIS_MODULE, NULL, adapter_nr, NULL) || > 0 == dvb_usb_device_init(intf, &tt_s2_4600_properties, > - THIS_MODULE, NULL, adapter_nr)) > + THIS_MODULE, NULL, adapter_nr, NULL)) > return 0; > > return -ENODEV; > diff --git a/drivers/media/usb/dvb-usb/friio.c b/drivers/media/usb/dvb-usb/friio.c > index 474a17e4db0c..3854ac7434ad 100644 > --- a/drivers/media/usb/dvb-usb/friio.c > +++ b/drivers/media/usb/dvb-usb/friio.c > @@ -437,7 +437,7 @@ static int friio_probe(struct usb_interface *intf, > } > > ret = dvb_usb_device_init(intf, &friio_properties, > - THIS_MODULE, &d, adapter_nr); > + THIS_MODULE, &d, adapter_nr, NULL); > if (ret == 0) > friio_streaming_ctrl(&d->adapter[0], 1); > > diff --git a/drivers/media/usb/dvb-usb/gp8psk.c b/drivers/media/usb/dvb-usb/gp8psk.c > index 2829e3082d15..cede0d8b0f8a 100644 > --- a/drivers/media/usb/dvb-usb/gp8psk.c > +++ b/drivers/media/usb/dvb-usb/gp8psk.c > @@ -262,7 +262,7 @@ static int gp8psk_usb_probe(struct usb_interface *intf, > int ret; > struct usb_device *udev = interface_to_usbdev(intf); > ret = dvb_usb_device_init(intf, &gp8psk_properties, > - THIS_MODULE, NULL, adapter_nr); > + THIS_MODULE, NULL, adapter_nr, NULL); > if (ret == 0) { > info("found Genpix USB device pID = %x (hex)", > le16_to_cpu(udev->descriptor.idProduct)); > diff --git a/drivers/media/usb/dvb-usb/m920x.c b/drivers/media/usb/dvb-usb/m920x.c > index eafc5c82467f..b4c83f36abee 100644 > --- a/drivers/media/usb/dvb-usb/m920x.c > +++ b/drivers/media/usb/dvb-usb/m920x.c > @@ -835,14 +835,14 @@ static int m920x_probe(struct usb_interface *intf, > */ > > ret = dvb_usb_device_init(intf, &megasky_properties, > - THIS_MODULE, &d, adapter_nr); > + THIS_MODULE, &d, adapter_nr, NULL); > if (ret == 0) { > rc_init_seq = megasky_rc_init; > goto found; > } > > ret = dvb_usb_device_init(intf, &digivox_mini_ii_properties, > - THIS_MODULE, &d, adapter_nr); > + THIS_MODULE, &d, adapter_nr, NULL); > if (ret == 0) { > /* No remote control, so no rc_init_seq */ > goto found; > @@ -850,28 +850,28 @@ static int m920x_probe(struct usb_interface *intf, > > /* This configures both tuners on the TV Walker Twin */ > ret = dvb_usb_device_init(intf, &tvwalkertwin_properties, > - THIS_MODULE, &d, adapter_nr); > + THIS_MODULE, &d, adapter_nr, NULL); > if (ret == 0) { > rc_init_seq = tvwalkertwin_rc_init; > goto found; > } > > ret = dvb_usb_device_init(intf, &dposh_properties, > - THIS_MODULE, &d, adapter_nr); > + THIS_MODULE, &d, adapter_nr, NULL); > if (ret == 0) { > /* Remote controller not supported yet. */ > goto found; > } > > ret = dvb_usb_device_init(intf, &pinnacle_pctv310e_properties, > - THIS_MODULE, &d, adapter_nr); > + THIS_MODULE, &d, adapter_nr, NULL); > if (ret == 0) { > rc_init_seq = pinnacle310e_init; > goto found; > } > > ret = dvb_usb_device_init(intf, &vp7049_properties, > - THIS_MODULE, &d, adapter_nr); > + THIS_MODULE, &d, adapter_nr, NULL); > if (ret == 0) { > rc_init_seq = vp7049_rc_init; > goto found; > diff --git a/drivers/media/usb/dvb-usb/nova-t-usb2.c b/drivers/media/usb/dvb-usb/nova-t-usb2.c > index 1babd3341910..0d8f06430ca5 100644 > --- a/drivers/media/usb/dvb-usb/nova-t-usb2.c > +++ b/drivers/media/usb/dvb-usb/nova-t-usb2.c > @@ -157,7 +157,7 @@ static int nova_t_probe(struct usb_interface *intf, > const struct usb_device_id *id) > { > return dvb_usb_device_init(intf, &nova_t_properties, > - THIS_MODULE, NULL, adapter_nr); > + THIS_MODULE, NULL, adapter_nr, NULL); > } > > /* do not change the order of the ID table */ > diff --git a/drivers/media/usb/dvb-usb/opera1.c b/drivers/media/usb/dvb-usb/opera1.c > index 2566d2f1c2ad..9f2f156e2939 100644 > --- a/drivers/media/usb/dvb-usb/opera1.c > +++ b/drivers/media/usb/dvb-usb/opera1.c > @@ -563,7 +563,7 @@ static int opera1_probe(struct usb_interface *intf, > } > > if (0 != dvb_usb_device_init(intf, &opera1_properties, > - THIS_MODULE, NULL, adapter_nr)) > + THIS_MODULE, NULL, adapter_nr, NULL)) > return -EINVAL; > return 0; > } > diff --git a/drivers/media/usb/dvb-usb/pctv452e.c b/drivers/media/usb/dvb-usb/pctv452e.c > index 07fa08be9e99..2f844ff39840 100644 > --- a/drivers/media/usb/dvb-usb/pctv452e.c > +++ b/drivers/media/usb/dvb-usb/pctv452e.c > @@ -1059,9 +1059,9 @@ static int pctv452e_usb_probe(struct usb_interface *intf, > const struct usb_device_id *id) > { > if (0 == dvb_usb_device_init(intf, &pctv452e_properties, > - THIS_MODULE, NULL, adapter_nr) || > + THIS_MODULE, NULL, adapter_nr, NULL) || > 0 == dvb_usb_device_init(intf, &tt_connect_s2_3600_properties, > - THIS_MODULE, NULL, adapter_nr)) > + THIS_MODULE, NULL, adapter_nr, NULL)) > return 0; > > return -ENODEV; > diff --git a/drivers/media/usb/dvb-usb/technisat-usb2.c b/drivers/media/usb/dvb-usb/technisat-usb2.c > index 4706628a3ed5..80fc5043e4cf 100644 > --- a/drivers/media/usb/dvb-usb/technisat-usb2.c > +++ b/drivers/media/usb/dvb-usb/technisat-usb2.c > @@ -765,7 +765,7 @@ static int technisat_usb2_probe(struct usb_interface *intf, > struct dvb_usb_device *dev; > > if (dvb_usb_device_init(intf, &technisat_usb2_devices, THIS_MODULE, > - &dev, adapter_nr) != 0) > + &dev, adapter_nr, NULL) != 0) > return -ENODEV; > > if (dev) { > diff --git a/drivers/media/usb/dvb-usb/ttusb2.c b/drivers/media/usb/dvb-usb/ttusb2.c > index ecc207fbaf3c..192057ea58c0 100644 > --- a/drivers/media/usb/dvb-usb/ttusb2.c > +++ b/drivers/media/usb/dvb-usb/ttusb2.c > @@ -604,11 +604,11 @@ static int ttusb2_probe(struct usb_interface *intf, > const struct usb_device_id *id) > { > if (0 == dvb_usb_device_init(intf, &ttusb2_properties, > - THIS_MODULE, NULL, adapter_nr) || > + THIS_MODULE, NULL, adapter_nr, NULL) || > 0 == dvb_usb_device_init(intf, &ttusb2_properties_s2400, > - THIS_MODULE, NULL, adapter_nr) || > + THIS_MODULE, NULL, adapter_nr, NULL) || > 0 == dvb_usb_device_init(intf, &ttusb2_properties_ct3650, > - THIS_MODULE, NULL, adapter_nr)) > + THIS_MODULE, NULL, adapter_nr, NULL)) > return 0; > return -ENODEV; > } > diff --git a/drivers/media/usb/dvb-usb/umt-010.c b/drivers/media/usb/dvb-usb/umt-010.c > index 58ad5b4f856c..9af8cca5f2d2 100644 > --- a/drivers/media/usb/dvb-usb/umt-010.c > +++ b/drivers/media/usb/dvb-usb/umt-010.c > @@ -78,7 +78,7 @@ static int umt_probe(struct usb_interface *intf, > const struct usb_device_id *id) > { > if (0 == dvb_usb_device_init(intf, &umt_properties, > - THIS_MODULE, NULL, adapter_nr)) > + THIS_MODULE, NULL, adapter_nr, NULL)) > return 0; > return -EINVAL; > } > diff --git a/drivers/media/usb/dvb-usb/vp702x.c b/drivers/media/usb/dvb-usb/vp702x.c > index 40de33de90a7..e95a84e3f9de 100644 > --- a/drivers/media/usb/dvb-usb/vp702x.c > +++ b/drivers/media/usb/dvb-usb/vp702x.c > @@ -337,7 +337,7 @@ static int vp702x_usb_probe(struct usb_interface *intf, > int ret; > > ret = dvb_usb_device_init(intf, &vp702x_properties, > - THIS_MODULE, &d, adapter_nr); > + THIS_MODULE, &d, adapter_nr, NULL); > if (ret) > goto out; > > diff --git a/drivers/media/usb/dvb-usb/vp7045.c b/drivers/media/usb/dvb-usb/vp7045.c > index 13340af0d39c..350603bbe3da 100644 > --- a/drivers/media/usb/dvb-usb/vp7045.c > +++ b/drivers/media/usb/dvb-usb/vp7045.c > @@ -225,7 +225,7 @@ static int vp7045_usb_probe(struct usb_interface *intf, > const struct usb_device_id *id) > { > return dvb_usb_device_init(intf, &vp7045_properties, > - THIS_MODULE, NULL, adapter_nr); > + THIS_MODULE, NULL, adapter_nr, NULL); > } > > static struct usb_device_id vp7045_usb_table [] = { Tried patch with no success. Again a NULL ptr dereferece. Thanks Jörg Hardware name: FUJITSU LIFEBOOK A544/FJNBB35 , BIOS Version 1.17 05/09/2014 task: ffff8802134ac380 task.stack: ffff8801ed878000 RIP: 0010:[<ffffffffa406b444>] [<ffffffffa406b444>] __mutex_init+0x4/0x30 RSP: 0018:ffff8801ed87bb98 EFLAGS: 00010206 RAX: 0000000000000050 RBX: ffff8801ef23c000 RCX: ffffea0007bc8f01 RDX: ffffffffc035ef84 RSI: ffffffffc035d772 RDI: 0000000000000048 RBP: ffffffffc035db00 R08: ffffffffffffffe1 R09: ffffffffa4c5fa90 R10: 0000000000000004 R11: 0000000000000000 R12: ffffffffc035d030 R13: ffff880214c93000 R14: 0000000000000000 R15: ffff88020d942400 FS: 00007fb1359ae880(0000) GS:ffff88021f380000(0000) knlGS:0000000000000000 CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 CR2: 0000000000000048 CR3: 00000001f53a2000 CR4: 00000000001406e0 Stack: ffffffffc035d04e ffffffffc03555f0 ffffffffc035e970 ffffffffc035ddd0 ffffffffc035ec80 00000000a4397a2f ffffffffc035ddd0 ffff880214c93098 ffff880214c93000 ffffffffc035d8e8 ffff88020d942400 ffffffffc035d980 Call Trace: [<ffffffffc035d04e>] ? cinergyT2_init_mutex+0x1e/0x30 [dvb_usb_cinergyT2] [<ffffffffc03555f0>] ? dvb_usb_device_init+0x190/0x640 [dvb_usb] [<ffffffffa44326f3>] ? usb_probe_interface+0xf3/0x2a0 [<ffffffffa438e348>] ? driver_probe_device+0x208/0x2b0 [<ffffffffa438e477>] ? __driver_attach+0x87/0x90 [<ffffffffa438e3f0>] ? driver_probe_device+0x2b0/0x2b0 [<ffffffffa438c612>] ? bus_for_each_dev+0x52/0x80 [<ffffffffa438d983>] ? bus_add_driver+0x1a3/0x220 [<ffffffffa438ec06>] ? driver_register+0x56/0xd0 [<ffffffffa4431527>] ? usb_register_driver+0x77/0x130 [<ffffffffc0361000>] ? 0xffffffffc0361000 [<ffffffffa4000426>] ? do_one_initcall+0x46/0x180 [<ffffffffa40eb2c8>] ? free_vmap_area_noflush+0x38/0x70 [<ffffffffa40f3844>] ? kmem_cache_alloc+0x84/0xc0 [<ffffffffa40b802c>] ? do_init_module+0x50/0x1be [<ffffffffa4095adb>] ? load_module+0x1d8b/0x2100 [<ffffffffa4093020>] ? find_symbol_in_section+0xa0/0xa0 [<ffffffffa4095fe9>] ? SyS_finit_module+0x89/0x90 [<ffffffffa46637a0>] ? entry_SYSCALL_64_fastpath+0x13/0x94 Code: 5b 48 89 ee 89 c2 5d e9 bb f9 ff ff 41 b8 00 04 00 00 eb b1 c6 83 a1 00 00 00 00 c7 43 38 ff ff ff ff e9 2f ff ff ff 48 8d 47 08 <c7> 07 01 00 00 00 c7 47 04 00 00 00 00 48 89 47 08 48 89 47 10 RIP [<ffffffffa406b444>] __mutex_init+0x4/0x30 RSP <ffff8801ed87bb98> CR2: 0000000000000048 ---[ end trace 03576741447cea2c ]--- ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [v4.9-rc4] dvb-usb/cinergyT2 NULL pointer dereference 2016-11-09 11:09 ` Jörg Otte @ 2016-11-09 19:07 ` Linus Torvalds 2016-11-09 20:21 ` Mauro Carvalho Chehab 2016-11-10 8:40 ` Mauro Carvalho Chehab 0 siblings, 2 replies; 14+ messages in thread From: Linus Torvalds @ 2016-11-09 19:07 UTC (permalink / raw) To: Jörg Otte Cc: Mauro Carvalho Chehab, Patrick Boettcher, Mauro Carvalho Chehab, Linux Kernel Mailing List, Linux Media Mailing List On Wed, Nov 9, 2016 at 3:09 AM, Jörg Otte <jrg.otte@gmail.com> wrote: > > Tried patch with no success. Again a NULL ptr dereferece. That patch was pure garbage, I think. Pretty much all the other drivers that use the same approach will have the same issue. Adding that init function just for the semaphore is crazy. I suspect a much simpler approach is to just miove the "data_mutex" away from the priv area and into "struct dvb_usb_device" and "dvb_usb_adapter". Sure, that grows those structures a tiny bit, and not every driver may need that mutex, but it simplifies things enormously. Mauro? Linus ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [v4.9-rc4] dvb-usb/cinergyT2 NULL pointer dereference 2016-11-09 19:07 ` Linus Torvalds @ 2016-11-09 20:21 ` Mauro Carvalho Chehab 2016-11-10 8:40 ` Mauro Carvalho Chehab 1 sibling, 0 replies; 14+ messages in thread From: Mauro Carvalho Chehab @ 2016-11-09 20:21 UTC (permalink / raw) To: Linus Torvalds Cc: Jörg Otte, Patrick Boettcher, Mauro Carvalho Chehab, Linux Kernel Mailing List, Linux Media Mailing List Em Wed, 9 Nov 2016 11:07:35 -0800 Linus Torvalds <torvalds@linux-foundation.org> escreveu: > On Wed, Nov 9, 2016 at 3:09 AM, Jörg Otte <jrg.otte@gmail.com> wrote: > > > > Tried patch with no success. Again a NULL ptr dereferece. > > That patch was pure garbage, I think. Pretty much all the other > drivers that use the same approach will have the same issue. Adding > that init function just for the semaphore is crazy. > > I suspect a much simpler approach is to just miove the "data_mutex" > away from the priv area and into "struct dvb_usb_device" and > "dvb_usb_adapter". Sure, that grows those structures a tiny bit, and > not every driver may need that mutex, but it simplifies things > enormously. Mauro? Yeah, makes sense. I'll work on moving data_mutex to struct dvb_usb_device. Thanks, Mauro ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [v4.9-rc4] dvb-usb/cinergyT2 NULL pointer dereference 2016-11-09 19:07 ` Linus Torvalds 2016-11-09 20:21 ` Mauro Carvalho Chehab @ 2016-11-10 8:40 ` Mauro Carvalho Chehab 2016-11-10 11:15 ` Jörg Otte 1 sibling, 1 reply; 14+ messages in thread From: Mauro Carvalho Chehab @ 2016-11-10 8:40 UTC (permalink / raw) To: Linus Torvalds Cc: Jörg Otte, Patrick Boettcher, Mauro Carvalho Chehab, Linux Kernel Mailing List, Linux Media Mailing List Em Wed, 9 Nov 2016 11:07:35 -0800 Linus Torvalds <torvalds@linux-foundation.org> escreveu: > On Wed, Nov 9, 2016 at 3:09 AM, Jörg Otte <jrg.otte@gmail.com> wrote: > > > > Tried patch with no success. Again a NULL ptr dereferece. > > That patch was pure garbage, I think. Pretty much all the other > drivers that use the same approach will have the same issue. Adding > that init function just for the semaphore is crazy. > > I suspect a much simpler approach is to just miove the "data_mutex" > away from the priv area and into "struct dvb_usb_device" and > "dvb_usb_adapter". Sure, that grows those structures a tiny bit, and > not every driver may need that mutex, but it simplifies things > enormously. Mauro? > > Linus [PATCH] cinergyT2-core: move data_mutex to struct dvb_usb_device The data_mutex is initialized too late, as it is needed for the device's power control, causing an OOPS: dvb-usb: found a 'TerraTec/qanu USB2.0 Highspeed DVB-T Receiver' in warm state. BUG: unable to handle kernel NULL pointer dereference at (null) IP: [<ffffffff846617af>] __mutex_lock_slowpath+0x6f/0x100 PGD 0 Oops: 0002 [#1] SMP Modules linked in: dvb_usb_cinergyT2(+) dvb_usb CPU: 0 PID: 2029 Comm: modprobe Not tainted 4.9.0-rc4-dvbmod #24 Hardware name: FUJITSU LIFEBOOK A544/FJNBB35 , BIOS Version 1.17 05/09/2014 task: ffff88020e943840 task.stack: ffff8801f36ec000 RIP: 0010:[<ffffffff846617af>] [<ffffffff846617af>] __mutex_lock_slowpath+0x6f/0x100 RSP: 0018:ffff8801f36efb10 EFLAGS: 00010282 RAX: 0000000000000000 RBX: ffff88021509bdc8 RCX: 00000000c0000100 RDX: 0000000000000001 RSI: 0000000000000000 RDI: ffff88021509bdcc RBP: ffff8801f36efb58 R08: ffff88021f216320 R09: 0000000000100000 R10: ffff88021f216320 R11: 00000023fee6c5a1 R12: ffff88020e943840 R13: ffff88021509bdcc R14: 00000000ffffffff R15: ffff88021509bdd0 FS: 00007f21adb86740(0000) GS:ffff88021f200000(0000) knlGS:0000000000000000 CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 CR2: 0000000000000000 CR3: 0000000215bce000 CR4: 00000000001406f0 Stack: ffff88021509bdd0 0000000000000000 0000000000000000 ffffffffc0137c80 ffff88021509bdc8 ffff8801f5944000 0000000000000001 ffffffffc0136b00 ffff880213e52000 ffff88021509bdc8 ffffffff84661856 ffff88021509bd80 Call Trace: [<ffffffff84661856>] ? mutex_lock+0x16/0x25 [<ffffffffc013616f>] ? cinergyt2_power_ctrl+0x1f/0x60 [dvb_usb_cinergyT2] [<ffffffffc012e67e>] ? dvb_usb_device_init+0x21e/0x5d0 [dvb_usb] [<ffffffffc0136021>] ? cinergyt2_usb_probe+0x21/0x50 [dvb_usb_cinergyT2] [<ffffffff844326f3>] ? usb_probe_interface+0xf3/0x2a0 [<ffffffff8438e348>] ? driver_probe_device+0x208/0x2b0 [<ffffffff8438e477>] ? __driver_attach+0x87/0x90 [<ffffffff8438e3f0>] ? driver_probe_device+0x2b0/0x2b0 [<ffffffff8438c612>] ? bus_for_each_dev+0x52/0x80 [<ffffffff8438d983>] ? bus_add_driver+0x1a3/0x220 [<ffffffff8438ec06>] ? driver_register+0x56/0xd0 [<ffffffff84431527>] ? usb_register_driver+0x77/0x130 [<ffffffffc013a000>] ? 0xffffffffc013a000 [<ffffffff84000426>] ? do_one_initcall+0x46/0x180 [<ffffffff840eb2c8>] ? free_vmap_area_noflush+0x38/0x70 [<ffffffff840f3844>] ? kmem_cache_alloc+0x84/0xc0 [<ffffffff840b802c>] ? do_init_module+0x50/0x1be [<ffffffff84095adb>] ? load_module+0x1d8b/0x2100 [<ffffffff84093020>] ? find_symbol_in_section+0xa0/0xa0 [<ffffffff84095fe9>] ? SyS_finit_module+0x89/0x90 [<ffffffff846637a0>] ? entry_SYSCALL_64_fastpath+0x13/0x94 Code: e8 a7 1d 00 00 8b 03 83 f8 01 0f 84 97 00 00 00 48 8b 43 10 4c 8d 7b 08 48 89 63 10 4c 89 3c 24 41 be ff ff ff ff 48 89 44 24 08 <48> 89 20 4c 89 64 24 10 eb 1a 49 c7 44 24 08 02 00 00 00 c6 43 RIP [<ffffffff846617af>] __mutex_lock_slowpath+0x6f/0x100 RSP <ffff8801f36efb10> CR2: 0000000000000000 So, move it to the struct dvb_usb_device and initialize it before calling the driver's callbacks. Reported-by: Jörg Otte <jrg.otte@gmail.com> Signed-off-by: Mauro Carvalho Chehab <mchehab@s-opensource.com> diff --git a/drivers/media/usb/dvb-usb/cinergyT2-core.c b/drivers/media/usb/dvb-usb/cinergyT2-core.c index 8ac825413d5a..87e3bd33900d 100644 --- a/drivers/media/usb/dvb-usb/cinergyT2-core.c +++ b/drivers/media/usb/dvb-usb/cinergyT2-core.c @@ -42,7 +42,6 @@ DVB_DEFINE_MOD_OPT_ADAPTER_NR(adapter_nr); struct cinergyt2_state { u8 rc_counter; unsigned char data[64]; - struct mutex data_mutex; }; /* We are missing a release hook with usb_device data */ @@ -56,12 +55,12 @@ static int cinergyt2_streaming_ctrl(struct dvb_usb_adapter *adap, int enable) struct cinergyt2_state *st = d->priv; int ret; - mutex_lock(&st->data_mutex); + mutex_lock(&d->data_mutex); st->data[0] = CINERGYT2_EP1_CONTROL_STREAM_TRANSFER; st->data[1] = enable ? 1 : 0; ret = dvb_usb_generic_rw(d, st->data, 2, st->data, 64, 0); - mutex_unlock(&st->data_mutex); + mutex_unlock(&d->data_mutex); return ret; } @@ -71,12 +70,12 @@ static int cinergyt2_power_ctrl(struct dvb_usb_device *d, int enable) struct cinergyt2_state *st = d->priv; int ret; - mutex_lock(&st->data_mutex); + mutex_lock(&d->data_mutex); st->data[0] = CINERGYT2_EP1_SLEEP_MODE; st->data[1] = enable ? 0 : 1; ret = dvb_usb_generic_rw(d, st->data, 2, st->data, 3, 0); - mutex_unlock(&st->data_mutex); + mutex_unlock(&d->data_mutex); return ret; } @@ -89,7 +88,7 @@ static int cinergyt2_frontend_attach(struct dvb_usb_adapter *adap) adap->fe_adap[0].fe = cinergyt2_fe_attach(adap->dev); - mutex_lock(&st->data_mutex); + mutex_lock(&d->data_mutex); st->data[0] = CINERGYT2_EP1_GET_FIRMWARE_VERSION; ret = dvb_usb_generic_rw(d, st->data, 1, st->data, 3, 0); @@ -97,7 +96,7 @@ static int cinergyt2_frontend_attach(struct dvb_usb_adapter *adap) deb_rc("cinergyt2_power_ctrl() Failed to retrieve sleep " "state info\n"); } - mutex_unlock(&st->data_mutex); + mutex_unlock(&d->data_mutex); /* Copy this pointer as we are gonna need it in the release phase */ cinergyt2_usb_device = adap->dev; @@ -166,7 +165,7 @@ static int cinergyt2_rc_query(struct dvb_usb_device *d, u32 *event, int *state) *state = REMOTE_NO_KEY_PRESSED; - mutex_lock(&st->data_mutex); + mutex_lock(&d->data_mutex); st->data[0] = CINERGYT2_EP1_GET_RC_EVENTS; ret = dvb_usb_generic_rw(d, st->data, 1, st->data, 5, 0); @@ -202,7 +201,7 @@ static int cinergyt2_rc_query(struct dvb_usb_device *d, u32 *event, int *state) } ret: - mutex_unlock(&st->data_mutex); + mutex_unlock(&d->data_mutex); return ret; } @@ -210,7 +209,6 @@ static int cinergyt2_usb_probe(struct usb_interface *intf, const struct usb_device_id *id) { struct dvb_usb_device *d; - struct cinergyt2_state *st; int ret; ret = dvb_usb_device_init(intf, &cinergyt2_properties, @@ -218,9 +216,6 @@ static int cinergyt2_usb_probe(struct usb_interface *intf, if (ret < 0) return ret; - st = d->priv; - mutex_init(&st->data_mutex); - return 0; } diff --git a/drivers/media/usb/dvb-usb/dvb-usb-init.c b/drivers/media/usb/dvb-usb/dvb-usb-init.c index 3896ba9a4179..84308569e7dc 100644 --- a/drivers/media/usb/dvb-usb/dvb-usb-init.c +++ b/drivers/media/usb/dvb-usb/dvb-usb-init.c @@ -142,6 +142,7 @@ static int dvb_usb_init(struct dvb_usb_device *d, short *adapter_nums) { int ret = 0; + mutex_init(&d->data_mutex); mutex_init(&d->usb_mutex); mutex_init(&d->i2c_mutex); diff --git a/drivers/media/usb/dvb-usb/dvb-usb.h b/drivers/media/usb/dvb-usb/dvb-usb.h index 1448c3d27ea2..12b71acee550 100644 --- a/drivers/media/usb/dvb-usb/dvb-usb.h +++ b/drivers/media/usb/dvb-usb/dvb-usb.h @@ -404,6 +404,7 @@ struct dvb_usb_adapter { * Powered is in/decremented for each call to modify the state. * @udev: pointer to the device's struct usb_device. * + * @data_mutex: mutex to protect the data structure used to store URB data * @usb_mutex: semaphore of USB control messages (reading needs two messages) * @i2c_mutex: semaphore for i2c-transfers * @@ -433,6 +434,7 @@ struct dvb_usb_device { int powered; /* locking */ + struct mutex data_mutex; struct mutex usb_mutex; /* i2c */ ^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [v4.9-rc4] dvb-usb/cinergyT2 NULL pointer dereference 2016-11-10 8:40 ` Mauro Carvalho Chehab @ 2016-11-10 11:15 ` Jörg Otte 2016-11-11 13:55 ` Mauro Carvalho Chehab 0 siblings, 1 reply; 14+ messages in thread From: Jörg Otte @ 2016-11-10 11:15 UTC (permalink / raw) To: Mauro Carvalho Chehab Cc: Linus Torvalds, Patrick Boettcher, Mauro Carvalho Chehab, Linux Kernel Mailing List, Linux Media Mailing List 2016-11-10 9:40 GMT+01:00 Mauro Carvalho Chehab <mchehab@s-opensource.com>: > Em Wed, 9 Nov 2016 11:07:35 -0800 > Linus Torvalds <torvalds@linux-foundation.org> escreveu: > >> On Wed, Nov 9, 2016 at 3:09 AM, Jörg Otte <jrg.otte@gmail.com> wrote: >> > >> > Tried patch with no success. Again a NULL ptr dereferece. >> >> That patch was pure garbage, I think. Pretty much all the other >> drivers that use the same approach will have the same issue. Adding >> that init function just for the semaphore is crazy. >> >> I suspect a much simpler approach is to just miove the "data_mutex" >> away from the priv area and into "struct dvb_usb_device" and >> "dvb_usb_adapter". Sure, that grows those structures a tiny bit, and >> not every driver may need that mutex, but it simplifies things >> enormously. Mauro? >> >> Linus > > > [PATCH] cinergyT2-core: move data_mutex to struct dvb_usb_device > > The data_mutex is initialized too late, as it is needed for > the device's power control, causing an OOPS: > > dvb-usb: found a 'TerraTec/qanu USB2.0 Highspeed DVB-T Receiver' in warm state. > BUG: unable to handle kernel NULL pointer dereference at (null) > IP: [<ffffffff846617af>] __mutex_lock_slowpath+0x6f/0x100 PGD 0 > Oops: 0002 [#1] SMP > Modules linked in: dvb_usb_cinergyT2(+) dvb_usb > CPU: 0 PID: 2029 Comm: modprobe Not tainted 4.9.0-rc4-dvbmod #24 > Hardware name: FUJITSU LIFEBOOK A544/FJNBB35 , BIOS Version 1.17 05/09/2014 > task: ffff88020e943840 task.stack: ffff8801f36ec000 > RIP: 0010:[<ffffffff846617af>] [<ffffffff846617af>] __mutex_lock_slowpath+0x6f/0x100 > RSP: 0018:ffff8801f36efb10 EFLAGS: 00010282 > RAX: 0000000000000000 RBX: ffff88021509bdc8 RCX: 00000000c0000100 > RDX: 0000000000000001 RSI: 0000000000000000 RDI: ffff88021509bdcc > RBP: ffff8801f36efb58 R08: ffff88021f216320 R09: 0000000000100000 > R10: ffff88021f216320 R11: 00000023fee6c5a1 R12: ffff88020e943840 > R13: ffff88021509bdcc R14: 00000000ffffffff R15: ffff88021509bdd0 > FS: 00007f21adb86740(0000) GS:ffff88021f200000(0000) knlGS:0000000000000000 > CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 > CR2: 0000000000000000 CR3: 0000000215bce000 CR4: 00000000001406f0 > Stack: > ffff88021509bdd0 0000000000000000 0000000000000000 ffffffffc0137c80 > ffff88021509bdc8 ffff8801f5944000 0000000000000001 ffffffffc0136b00 > ffff880213e52000 ffff88021509bdc8 ffffffff84661856 ffff88021509bd80 > Call Trace: > [<ffffffff84661856>] ? mutex_lock+0x16/0x25 > [<ffffffffc013616f>] ? cinergyt2_power_ctrl+0x1f/0x60 [dvb_usb_cinergyT2] > [<ffffffffc012e67e>] ? dvb_usb_device_init+0x21e/0x5d0 [dvb_usb] > [<ffffffffc0136021>] ? cinergyt2_usb_probe+0x21/0x50 [dvb_usb_cinergyT2] > [<ffffffff844326f3>] ? usb_probe_interface+0xf3/0x2a0 > [<ffffffff8438e348>] ? driver_probe_device+0x208/0x2b0 > [<ffffffff8438e477>] ? __driver_attach+0x87/0x90 > [<ffffffff8438e3f0>] ? driver_probe_device+0x2b0/0x2b0 > [<ffffffff8438c612>] ? bus_for_each_dev+0x52/0x80 > [<ffffffff8438d983>] ? bus_add_driver+0x1a3/0x220 > [<ffffffff8438ec06>] ? driver_register+0x56/0xd0 > [<ffffffff84431527>] ? usb_register_driver+0x77/0x130 > [<ffffffffc013a000>] ? 0xffffffffc013a000 > [<ffffffff84000426>] ? do_one_initcall+0x46/0x180 > [<ffffffff840eb2c8>] ? free_vmap_area_noflush+0x38/0x70 > [<ffffffff840f3844>] ? kmem_cache_alloc+0x84/0xc0 > [<ffffffff840b802c>] ? do_init_module+0x50/0x1be > [<ffffffff84095adb>] ? load_module+0x1d8b/0x2100 > [<ffffffff84093020>] ? find_symbol_in_section+0xa0/0xa0 > [<ffffffff84095fe9>] ? SyS_finit_module+0x89/0x90 > [<ffffffff846637a0>] ? entry_SYSCALL_64_fastpath+0x13/0x94 > Code: e8 a7 1d 00 00 8b 03 83 f8 01 0f 84 97 00 00 00 48 8b 43 10 4c 8d 7b 08 48 89 63 10 4c 89 3c 24 41 be ff ff ff ff 48 89 44 24 08 <48> 89 20 4c 89 64 24 10 eb 1a 49 c7 44 24 08 02 00 00 00 c6 43 RIP [<ffffffff846617af>] __mutex_lock_slowpath+0x6f/0x100 RSP <ffff8801f36efb10> > CR2: 0000000000000000 > > So, move it to the struct dvb_usb_device and initialize it > before calling the driver's callbacks. > > Reported-by: Jörg Otte <jrg.otte@gmail.com> > Signed-off-by: Mauro Carvalho Chehab <mchehab@s-opensource.com> > > diff --git a/drivers/media/usb/dvb-usb/cinergyT2-core.c b/drivers/media/usb/dvb-usb/cinergyT2-core.c > index 8ac825413d5a..87e3bd33900d 100644 > --- a/drivers/media/usb/dvb-usb/cinergyT2-core.c > +++ b/drivers/media/usb/dvb-usb/cinergyT2-core.c > @@ -42,7 +42,6 @@ DVB_DEFINE_MOD_OPT_ADAPTER_NR(adapter_nr); > struct cinergyt2_state { > u8 rc_counter; > unsigned char data[64]; > - struct mutex data_mutex; > }; > > /* We are missing a release hook with usb_device data */ > @@ -56,12 +55,12 @@ static int cinergyt2_streaming_ctrl(struct dvb_usb_adapter *adap, int enable) > struct cinergyt2_state *st = d->priv; > int ret; > > - mutex_lock(&st->data_mutex); > + mutex_lock(&d->data_mutex); > st->data[0] = CINERGYT2_EP1_CONTROL_STREAM_TRANSFER; > st->data[1] = enable ? 1 : 0; > > ret = dvb_usb_generic_rw(d, st->data, 2, st->data, 64, 0); > - mutex_unlock(&st->data_mutex); > + mutex_unlock(&d->data_mutex); > > return ret; > } > @@ -71,12 +70,12 @@ static int cinergyt2_power_ctrl(struct dvb_usb_device *d, int enable) > struct cinergyt2_state *st = d->priv; > int ret; > > - mutex_lock(&st->data_mutex); > + mutex_lock(&d->data_mutex); > st->data[0] = CINERGYT2_EP1_SLEEP_MODE; > st->data[1] = enable ? 0 : 1; > > ret = dvb_usb_generic_rw(d, st->data, 2, st->data, 3, 0); > - mutex_unlock(&st->data_mutex); > + mutex_unlock(&d->data_mutex); > > return ret; > } > @@ -89,7 +88,7 @@ static int cinergyt2_frontend_attach(struct dvb_usb_adapter *adap) > > adap->fe_adap[0].fe = cinergyt2_fe_attach(adap->dev); > > - mutex_lock(&st->data_mutex); > + mutex_lock(&d->data_mutex); > st->data[0] = CINERGYT2_EP1_GET_FIRMWARE_VERSION; > > ret = dvb_usb_generic_rw(d, st->data, 1, st->data, 3, 0); > @@ -97,7 +96,7 @@ static int cinergyt2_frontend_attach(struct dvb_usb_adapter *adap) > deb_rc("cinergyt2_power_ctrl() Failed to retrieve sleep " > "state info\n"); > } > - mutex_unlock(&st->data_mutex); > + mutex_unlock(&d->data_mutex); > > /* Copy this pointer as we are gonna need it in the release phase */ > cinergyt2_usb_device = adap->dev; > @@ -166,7 +165,7 @@ static int cinergyt2_rc_query(struct dvb_usb_device *d, u32 *event, int *state) > > *state = REMOTE_NO_KEY_PRESSED; > > - mutex_lock(&st->data_mutex); > + mutex_lock(&d->data_mutex); > st->data[0] = CINERGYT2_EP1_GET_RC_EVENTS; > > ret = dvb_usb_generic_rw(d, st->data, 1, st->data, 5, 0); > @@ -202,7 +201,7 @@ static int cinergyt2_rc_query(struct dvb_usb_device *d, u32 *event, int *state) > } > > ret: > - mutex_unlock(&st->data_mutex); > + mutex_unlock(&d->data_mutex); > return ret; > } > > @@ -210,7 +209,6 @@ static int cinergyt2_usb_probe(struct usb_interface *intf, > const struct usb_device_id *id) > { > struct dvb_usb_device *d; > - struct cinergyt2_state *st; > int ret; > > ret = dvb_usb_device_init(intf, &cinergyt2_properties, > @@ -218,9 +216,6 @@ static int cinergyt2_usb_probe(struct usb_interface *intf, > if (ret < 0) > return ret; > > - st = d->priv; > - mutex_init(&st->data_mutex); > - > return 0; > } > > diff --git a/drivers/media/usb/dvb-usb/dvb-usb-init.c b/drivers/media/usb/dvb-usb/dvb-usb-init.c > index 3896ba9a4179..84308569e7dc 100644 > --- a/drivers/media/usb/dvb-usb/dvb-usb-init.c > +++ b/drivers/media/usb/dvb-usb/dvb-usb-init.c > @@ -142,6 +142,7 @@ static int dvb_usb_init(struct dvb_usb_device *d, short *adapter_nums) > { > int ret = 0; > > + mutex_init(&d->data_mutex); > mutex_init(&d->usb_mutex); > mutex_init(&d->i2c_mutex); > > diff --git a/drivers/media/usb/dvb-usb/dvb-usb.h b/drivers/media/usb/dvb-usb/dvb-usb.h > index 1448c3d27ea2..12b71acee550 100644 > --- a/drivers/media/usb/dvb-usb/dvb-usb.h > +++ b/drivers/media/usb/dvb-usb/dvb-usb.h > @@ -404,6 +404,7 @@ struct dvb_usb_adapter { > * Powered is in/decremented for each call to modify the state. > * @udev: pointer to the device's struct usb_device. > * > + * @data_mutex: mutex to protect the data structure used to store URB data > * @usb_mutex: semaphore of USB control messages (reading needs two messages) > * @i2c_mutex: semaphore for i2c-transfers > * > @@ -433,6 +434,7 @@ struct dvb_usb_device { > int powered; > > /* locking */ > + struct mutex data_mutex; > struct mutex usb_mutex; > > /* i2c */ > > The patch works for me. Thanks, Jörg ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [v4.9-rc4] dvb-usb/cinergyT2 NULL pointer dereference 2016-11-10 11:15 ` Jörg Otte @ 2016-11-11 13:55 ` Mauro Carvalho Chehab 2016-11-11 16:43 ` Jörg Otte 0 siblings, 1 reply; 14+ messages in thread From: Mauro Carvalho Chehab @ 2016-11-11 13:55 UTC (permalink / raw) To: Jörg Otte, Benjamin Larsson Cc: Mauro Carvalho Chehab, Linus Torvalds, Patrick Boettcher, Mauro Carvalho Chehab, Linux Kernel Mailing List, Linux Media Mailing List Em Thu, 10 Nov 2016 12:15:39 +0100 Jörg Otte <jrg.otte@gmail.com> escreveu: > 2016-11-10 9:40 GMT+01:00 Mauro Carvalho Chehab <mchehab@s-opensource.com>: > > Em Wed, 9 Nov 2016 11:07:35 -0800 > > Linus Torvalds <torvalds@linux-foundation.org> escreveu: > > > >> On Wed, Nov 9, 2016 at 3:09 AM, Jörg Otte <jrg.otte@gmail.com> wrote: > >> > > >> > Tried patch with no success. Again a NULL ptr dereferece. > >> > >> I suspect a much simpler approach is to just miove the "data_mutex" > >> away from the priv area and into "struct dvb_usb_device" and > >> "dvb_usb_adapter". Sure, that grows those structures a tiny bit, and > >> not every driver may need that mutex, but it simplifies things > >> enormously. Mauro? > >> > >> Linus > The patch works for me. Thanks for testing! That's the (hopefully) final version of it, with the fix applied to the other dvb-usb drivers that use data_mutex (except for the frontend ones, with uses a different private structure, and where the mutex is initialized at attach). Benjamin, Could you please test it? Thanks! Mauro - [PATCH] dvb-usb: move data_mutex to struct dvb_usb_device The data_mutex is initialized too late, as it is needed for each device driver's power control, causing an OOPS: dvb-usb: found a 'TerraTec/qanu USB2.0 Highspeed DVB-T Receiver' in warm state. BUG: unable to handle kernel NULL pointer dereference at (null) IP: [<ffffffff846617af>] __mutex_lock_slowpath+0x6f/0x100 PGD 0 Oops: 0002 [#1] SMP Modules linked in: dvb_usb_cinergyT2(+) dvb_usb CPU: 0 PID: 2029 Comm: modprobe Not tainted 4.9.0-rc4-dvbmod #24 Hardware name: FUJITSU LIFEBOOK A544/FJNBB35 , BIOS Version 1.17 05/09/2014 task: ffff88020e943840 task.stack: ffff8801f36ec000 RIP: 0010:[<ffffffff846617af>] [<ffffffff846617af>] __mutex_lock_slowpath+0x6f/0x100 RSP: 0018:ffff8801f36efb10 EFLAGS: 00010282 RAX: 0000000000000000 RBX: ffff88021509bdc8 RCX: 00000000c0000100 RDX: 0000000000000001 RSI: 0000000000000000 RDI: ffff88021509bdcc RBP: ffff8801f36efb58 R08: ffff88021f216320 R09: 0000000000100000 R10: ffff88021f216320 R11: 00000023fee6c5a1 R12: ffff88020e943840 R13: ffff88021509bdcc R14: 00000000ffffffff R15: ffff88021509bdd0 FS: 00007f21adb86740(0000) GS:ffff88021f200000(0000) knlGS:0000000000000000 CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 CR2: 0000000000000000 CR3: 0000000215bce000 CR4: 00000000001406f0 Stack: ffff88021509bdd0 0000000000000000 0000000000000000 ffffffffc0137c80 ffff88021509bdc8 ffff8801f5944000 0000000000000001 ffffffffc0136b00 ffff880213e52000 ffff88021509bdc8 ffffffff84661856 ffff88021509bd80 Call Trace: [<ffffffff84661856>] ? mutex_lock+0x16/0x25 [<ffffffffc013616f>] ? cinergyt2_power_ctrl+0x1f/0x60 [dvb_usb_cinergyT2] [<ffffffffc012e67e>] ? dvb_usb_device_init+0x21e/0x5d0 [dvb_usb] [<ffffffffc0136021>] ? cinergyt2_usb_probe+0x21/0x50 [dvb_usb_cinergyT2] [<ffffffff844326f3>] ? usb_probe_interface+0xf3/0x2a0 [<ffffffff8438e348>] ? driver_probe_device+0x208/0x2b0 [<ffffffff8438e477>] ? __driver_attach+0x87/0x90 [<ffffffff8438e3f0>] ? driver_probe_device+0x2b0/0x2b0 [<ffffffff8438c612>] ? bus_for_each_dev+0x52/0x80 [<ffffffff8438d983>] ? bus_add_driver+0x1a3/0x220 [<ffffffff8438ec06>] ? driver_register+0x56/0xd0 [<ffffffff84431527>] ? usb_register_driver+0x77/0x130 [<ffffffffc013a000>] ? 0xffffffffc013a000 [<ffffffff84000426>] ? do_one_initcall+0x46/0x180 [<ffffffff840eb2c8>] ? free_vmap_area_noflush+0x38/0x70 [<ffffffff840f3844>] ? kmem_cache_alloc+0x84/0xc0 [<ffffffff840b802c>] ? do_init_module+0x50/0x1be [<ffffffff84095adb>] ? load_module+0x1d8b/0x2100 [<ffffffff84093020>] ? find_symbol_in_section+0xa0/0xa0 [<ffffffff84095fe9>] ? SyS_finit_module+0x89/0x90 [<ffffffff846637a0>] ? entry_SYSCALL_64_fastpath+0x13/0x94 Code: e8 a7 1d 00 00 8b 03 83 f8 01 0f 84 97 00 00 00 48 8b 43 10 4c 8d 7b 08 48 89 63 10 4c 89 3c 24 41 be ff ff ff ff 48 89 44 24 08 <48> 89 20 4c 89 64 24 10 eb 1a 49 c7 44 24 08 02 00 00 00 c6 43 RIP [<ffffffff846617af>] __mutex_lock_slowpath+0x6f/0x100 RSP <ffff8801f36efb10> CR2: 0000000000000000 So, move it to the struct dvb_usb_device and initialize it before calling the driver's callbacks. Reported-by: Jörg Otte <jrg.otte@gmail.com> Signed-off-by: Mauro Carvalho Chehab <mchehab@s-opensource.com> diff --git a/drivers/media/usb/dvb-usb/af9005.c b/drivers/media/usb/dvb-usb/af9005.c index b257780fb380..7853261906b1 100644 --- a/drivers/media/usb/dvb-usb/af9005.c +++ b/drivers/media/usb/dvb-usb/af9005.c @@ -53,7 +53,6 @@ struct af9005_device_state { u8 sequence; int led_state; unsigned char data[256]; - struct mutex data_mutex; }; static int af9005_generic_read_write(struct dvb_usb_device *d, u16 reg, @@ -72,7 +71,7 @@ static int af9005_generic_read_write(struct dvb_usb_device *d, u16 reg, return -EINVAL; } - mutex_lock(&st->data_mutex); + mutex_lock(&d->data_mutex); st->data[0] = 14; /* rest of buffer length low */ st->data[1] = 0; /* rest of buffer length high */ @@ -140,7 +139,7 @@ static int af9005_generic_read_write(struct dvb_usb_device *d, u16 reg, values[i] = st->data[8 + i]; ret: - mutex_unlock(&st->data_mutex); + mutex_unlock(&d->data_mutex); return ret; } @@ -481,7 +480,7 @@ int af9005_send_command(struct dvb_usb_device *d, u8 command, u8 * wbuf, } packet_len = wlen + 5; - mutex_lock(&st->data_mutex); + mutex_lock(&d->data_mutex); st->data[0] = (u8) (packet_len & 0xff); st->data[1] = (u8) ((packet_len & 0xff00) >> 8); @@ -512,7 +511,7 @@ int af9005_send_command(struct dvb_usb_device *d, u8 command, u8 * wbuf, rbuf[i] = st->data[i + 7]; } - mutex_unlock(&st->data_mutex); + mutex_unlock(&d->data_mutex); return ret; } @@ -523,7 +522,7 @@ int af9005_read_eeprom(struct dvb_usb_device *d, u8 address, u8 * values, u8 seq; int ret, i; - mutex_lock(&st->data_mutex); + mutex_lock(&d->data_mutex); memset(st->data, 0, sizeof(st->data)); @@ -559,7 +558,7 @@ int af9005_read_eeprom(struct dvb_usb_device *d, u8 address, u8 * values, for (i = 0; i < len; i++) values[i] = st->data[6 + i]; } - mutex_unlock(&st->data_mutex); + mutex_unlock(&d->data_mutex); return ret; } @@ -847,7 +846,7 @@ static int af9005_rc_query(struct dvb_usb_device *d, u32 * event, int *state) return 0; } - mutex_lock(&st->data_mutex); + mutex_lock(&d->data_mutex); /* deb_info("rc_query\n"); */ st->data[0] = 3; /* rest of packet length low */ @@ -890,7 +889,7 @@ static int af9005_rc_query(struct dvb_usb_device *d, u32 * event, int *state) } ret: - mutex_unlock(&st->data_mutex); + mutex_unlock(&d->data_mutex); return ret; } @@ -1004,20 +1003,8 @@ static struct dvb_usb_device_properties af9005_properties; static int af9005_usb_probe(struct usb_interface *intf, const struct usb_device_id *id) { - struct dvb_usb_device *d; - struct af9005_device_state *st; - int ret; - - ret = dvb_usb_device_init(intf, &af9005_properties, - THIS_MODULE, &d, adapter_nr); - - if (ret < 0) - return ret; - - st = d->priv; - mutex_init(&st->data_mutex); - - return 0; + return dvb_usb_device_init(intf, &af9005_properties, + THIS_MODULE, NULL, adapter_nr); } enum af9005_usb_table_entry { diff --git a/drivers/media/usb/dvb-usb/cinergyT2-core.c b/drivers/media/usb/dvb-usb/cinergyT2-core.c index 8ac825413d5a..290275bc7fde 100644 --- a/drivers/media/usb/dvb-usb/cinergyT2-core.c +++ b/drivers/media/usb/dvb-usb/cinergyT2-core.c @@ -42,7 +42,6 @@ DVB_DEFINE_MOD_OPT_ADAPTER_NR(adapter_nr); struct cinergyt2_state { u8 rc_counter; unsigned char data[64]; - struct mutex data_mutex; }; /* We are missing a release hook with usb_device data */ @@ -56,12 +55,12 @@ static int cinergyt2_streaming_ctrl(struct dvb_usb_adapter *adap, int enable) struct cinergyt2_state *st = d->priv; int ret; - mutex_lock(&st->data_mutex); + mutex_lock(&d->data_mutex); st->data[0] = CINERGYT2_EP1_CONTROL_STREAM_TRANSFER; st->data[1] = enable ? 1 : 0; ret = dvb_usb_generic_rw(d, st->data, 2, st->data, 64, 0); - mutex_unlock(&st->data_mutex); + mutex_unlock(&d->data_mutex); return ret; } @@ -71,12 +70,12 @@ static int cinergyt2_power_ctrl(struct dvb_usb_device *d, int enable) struct cinergyt2_state *st = d->priv; int ret; - mutex_lock(&st->data_mutex); + mutex_lock(&d->data_mutex); st->data[0] = CINERGYT2_EP1_SLEEP_MODE; st->data[1] = enable ? 0 : 1; ret = dvb_usb_generic_rw(d, st->data, 2, st->data, 3, 0); - mutex_unlock(&st->data_mutex); + mutex_unlock(&d->data_mutex); return ret; } @@ -89,7 +88,7 @@ static int cinergyt2_frontend_attach(struct dvb_usb_adapter *adap) adap->fe_adap[0].fe = cinergyt2_fe_attach(adap->dev); - mutex_lock(&st->data_mutex); + mutex_lock(&d->data_mutex); st->data[0] = CINERGYT2_EP1_GET_FIRMWARE_VERSION; ret = dvb_usb_generic_rw(d, st->data, 1, st->data, 3, 0); @@ -97,7 +96,7 @@ static int cinergyt2_frontend_attach(struct dvb_usb_adapter *adap) deb_rc("cinergyt2_power_ctrl() Failed to retrieve sleep " "state info\n"); } - mutex_unlock(&st->data_mutex); + mutex_unlock(&d->data_mutex); /* Copy this pointer as we are gonna need it in the release phase */ cinergyt2_usb_device = adap->dev; @@ -166,7 +165,7 @@ static int cinergyt2_rc_query(struct dvb_usb_device *d, u32 *event, int *state) *state = REMOTE_NO_KEY_PRESSED; - mutex_lock(&st->data_mutex); + mutex_lock(&d->data_mutex); st->data[0] = CINERGYT2_EP1_GET_RC_EVENTS; ret = dvb_usb_generic_rw(d, st->data, 1, st->data, 5, 0); @@ -202,29 +201,17 @@ static int cinergyt2_rc_query(struct dvb_usb_device *d, u32 *event, int *state) } ret: - mutex_unlock(&st->data_mutex); + mutex_unlock(&d->data_mutex); return ret; } static int cinergyt2_usb_probe(struct usb_interface *intf, const struct usb_device_id *id) { - struct dvb_usb_device *d; - struct cinergyt2_state *st; - int ret; - - ret = dvb_usb_device_init(intf, &cinergyt2_properties, - THIS_MODULE, &d, adapter_nr); - if (ret < 0) - return ret; - - st = d->priv; - mutex_init(&st->data_mutex); - - return 0; + return dvb_usb_device_init(intf, &cinergyt2_properties, + THIS_MODULE, NULL, adapter_nr); } - static struct usb_device_id cinergyt2_usb_table[] = { { USB_DEVICE(USB_VID_TERRATEC, 0x0038) }, { 0 } diff --git a/drivers/media/usb/dvb-usb/cxusb.c b/drivers/media/usb/dvb-usb/cxusb.c index 39772812269d..243403081fa5 100644 --- a/drivers/media/usb/dvb-usb/cxusb.c +++ b/drivers/media/usb/dvb-usb/cxusb.c @@ -68,7 +68,7 @@ static int cxusb_ctrl_msg(struct dvb_usb_device *d, wo = (rbuf == NULL || rlen == 0); /* write-only */ - mutex_lock(&st->data_mutex); + mutex_lock(&d->data_mutex); st->data[0] = cmd; memcpy(&st->data[1], wbuf, wlen); if (wo) @@ -77,7 +77,7 @@ static int cxusb_ctrl_msg(struct dvb_usb_device *d, ret = dvb_usb_generic_rw(d, st->data, 1 + wlen, rbuf, rlen, 0); - mutex_unlock(&st->data_mutex); + mutex_unlock(&d->data_mutex); return ret; } @@ -1461,43 +1461,36 @@ static struct dvb_usb_device_properties cxusb_mygica_t230_properties; static int cxusb_probe(struct usb_interface *intf, const struct usb_device_id *id) { - struct dvb_usb_device *d; - struct cxusb_state *st; - if (0 == dvb_usb_device_init(intf, &cxusb_medion_properties, - THIS_MODULE, &d, adapter_nr) || + THIS_MODULE, NULL, adapter_nr) || 0 == dvb_usb_device_init(intf, &cxusb_bluebird_lgh064f_properties, - THIS_MODULE, &d, adapter_nr) || + THIS_MODULE, NULL, adapter_nr) || 0 == dvb_usb_device_init(intf, &cxusb_bluebird_dee1601_properties, - THIS_MODULE, &d, adapter_nr) || + THIS_MODULE, NULL, adapter_nr) || 0 == dvb_usb_device_init(intf, &cxusb_bluebird_lgz201_properties, - THIS_MODULE, &d, adapter_nr) || + THIS_MODULE, NULL, adapter_nr) || 0 == dvb_usb_device_init(intf, &cxusb_bluebird_dtt7579_properties, - THIS_MODULE, &d, adapter_nr) || + THIS_MODULE, NULL, adapter_nr) || 0 == dvb_usb_device_init(intf, &cxusb_bluebird_dualdig4_properties, - THIS_MODULE, &d, adapter_nr) || + THIS_MODULE, NULL, adapter_nr) || 0 == dvb_usb_device_init(intf, &cxusb_bluebird_nano2_properties, - THIS_MODULE, &d, adapter_nr) || + THIS_MODULE, NULL, adapter_nr) || 0 == dvb_usb_device_init(intf, &cxusb_bluebird_nano2_needsfirmware_properties, - THIS_MODULE, &d, adapter_nr) || + THIS_MODULE, NULL, adapter_nr) || 0 == dvb_usb_device_init(intf, &cxusb_aver_a868r_properties, - THIS_MODULE, &d, adapter_nr) || + THIS_MODULE, NULL, adapter_nr) || 0 == dvb_usb_device_init(intf, &cxusb_bluebird_dualdig4_rev2_properties, - THIS_MODULE, &d, adapter_nr) || + THIS_MODULE, NULL, adapter_nr) || 0 == dvb_usb_device_init(intf, &cxusb_d680_dmb_properties, - THIS_MODULE, &d, adapter_nr) || + THIS_MODULE, NULL, adapter_nr) || 0 == dvb_usb_device_init(intf, &cxusb_mygica_d689_properties, - THIS_MODULE, &d, adapter_nr) || + THIS_MODULE, NULL, adapter_nr) || 0 == dvb_usb_device_init(intf, &cxusb_mygica_t230_properties, - THIS_MODULE, &d, adapter_nr) || - 0) { - st = d->priv; - mutex_init(&st->data_mutex); - + THIS_MODULE, NULL, adapter_nr) || + 0) return 0; - } return -EINVAL; } diff --git a/drivers/media/usb/dvb-usb/cxusb.h b/drivers/media/usb/dvb-usb/cxusb.h index 9f3ee0e47d5c..18acda19527a 100644 --- a/drivers/media/usb/dvb-usb/cxusb.h +++ b/drivers/media/usb/dvb-usb/cxusb.h @@ -37,7 +37,6 @@ struct cxusb_state { struct i2c_client *i2c_client_tuner; unsigned char data[MAX_XFER_SIZE]; - struct mutex data_mutex; }; #endif diff --git a/drivers/media/usb/dvb-usb/dtt200u.c b/drivers/media/usb/dvb-usb/dtt200u.c index f88572c7ae7c..fcbff7fb0c4e 100644 --- a/drivers/media/usb/dvb-usb/dtt200u.c +++ b/drivers/media/usb/dvb-usb/dtt200u.c @@ -22,7 +22,6 @@ DVB_DEFINE_MOD_OPT_ADAPTER_NR(adapter_nr); struct dtt200u_state { unsigned char data[80]; - struct mutex data_mutex; }; static int dtt200u_power_ctrl(struct dvb_usb_device *d, int onoff) @@ -30,23 +29,24 @@ static int dtt200u_power_ctrl(struct dvb_usb_device *d, int onoff) struct dtt200u_state *st = d->priv; int ret = 0; - mutex_lock(&st->data_mutex); + mutex_lock(&d->data_mutex); st->data[0] = SET_INIT; if (onoff) ret = dvb_usb_generic_write(d, st->data, 2); - mutex_unlock(&st->data_mutex); + mutex_unlock(&d->data_mutex); return ret; } static int dtt200u_streaming_ctrl(struct dvb_usb_adapter *adap, int onoff) { - struct dtt200u_state *st = adap->dev->priv; + struct dvb_usb_device *d = adap->dev; + struct dtt200u_state *st = d->priv; int ret; - mutex_lock(&st->data_mutex); + mutex_lock(&d->data_mutex); st->data[0] = SET_STREAMING; st->data[1] = onoff; @@ -61,26 +61,27 @@ static int dtt200u_streaming_ctrl(struct dvb_usb_adapter *adap, int onoff) ret = dvb_usb_generic_write(adap->dev, st->data, 1); ret: - mutex_unlock(&st->data_mutex); + mutex_unlock(&d->data_mutex); return ret; } static int dtt200u_pid_filter(struct dvb_usb_adapter *adap, int index, u16 pid, int onoff) { - struct dtt200u_state *st = adap->dev->priv; + struct dvb_usb_device *d = adap->dev; + struct dtt200u_state *st = d->priv; int ret; pid = onoff ? pid : 0; - mutex_lock(&st->data_mutex); + mutex_lock(&d->data_mutex); st->data[0] = SET_PID_FILTER; st->data[1] = index; st->data[2] = pid & 0xff; st->data[3] = (pid >> 8) & 0x1f; ret = dvb_usb_generic_write(adap->dev, st->data, 4); - mutex_unlock(&st->data_mutex); + mutex_unlock(&d->data_mutex); return ret; } @@ -91,7 +92,7 @@ static int dtt200u_rc_query(struct dvb_usb_device *d) u32 scancode; int ret; - mutex_lock(&st->data_mutex); + mutex_lock(&d->data_mutex); st->data[0] = GET_RC_CODE; ret = dvb_usb_generic_rw(d, st->data, 1, st->data, 5, 0); @@ -126,7 +127,7 @@ static int dtt200u_rc_query(struct dvb_usb_device *d) deb_info("st->data: %*ph\n", 5, st->data); ret: - mutex_unlock(&st->data_mutex); + mutex_unlock(&d->data_mutex); return ret; } @@ -145,24 +146,17 @@ static struct dvb_usb_device_properties wt220u_miglia_properties; static int dtt200u_usb_probe(struct usb_interface *intf, const struct usb_device_id *id) { - struct dvb_usb_device *d; - struct dtt200u_state *st; - if (0 == dvb_usb_device_init(intf, &dtt200u_properties, - THIS_MODULE, &d, adapter_nr) || + THIS_MODULE, NULL, adapter_nr) || 0 == dvb_usb_device_init(intf, &wt220u_properties, - THIS_MODULE, &d, adapter_nr) || + THIS_MODULE, NULL, adapter_nr) || 0 == dvb_usb_device_init(intf, &wt220u_fc_properties, - THIS_MODULE, &d, adapter_nr) || + THIS_MODULE, NULL, adapter_nr) || 0 == dvb_usb_device_init(intf, &wt220u_zl0353_properties, - THIS_MODULE, &d, adapter_nr) || + THIS_MODULE, NULL, adapter_nr) || 0 == dvb_usb_device_init(intf, &wt220u_miglia_properties, - THIS_MODULE, &d, adapter_nr)) { - st = d->priv; - mutex_init(&st->data_mutex); - + THIS_MODULE, NULL, adapter_nr)) return 0; - } return -ENODEV; } diff --git a/drivers/media/usb/dvb-usb/dvb-usb-init.c b/drivers/media/usb/dvb-usb/dvb-usb-init.c index 3896ba9a4179..84308569e7dc 100644 --- a/drivers/media/usb/dvb-usb/dvb-usb-init.c +++ b/drivers/media/usb/dvb-usb/dvb-usb-init.c @@ -142,6 +142,7 @@ static int dvb_usb_init(struct dvb_usb_device *d, short *adapter_nums) { int ret = 0; + mutex_init(&d->data_mutex); mutex_init(&d->usb_mutex); mutex_init(&d->i2c_mutex); diff --git a/drivers/media/usb/dvb-usb/dvb-usb.h b/drivers/media/usb/dvb-usb/dvb-usb.h index 1448c3d27ea2..67f898b6f6d0 100644 --- a/drivers/media/usb/dvb-usb/dvb-usb.h +++ b/drivers/media/usb/dvb-usb/dvb-usb.h @@ -404,8 +404,12 @@ struct dvb_usb_adapter { * Powered is in/decremented for each call to modify the state. * @udev: pointer to the device's struct usb_device. * - * @usb_mutex: semaphore of USB control messages (reading needs two messages) - * @i2c_mutex: semaphore for i2c-transfers + * @data_mutex: mutex to protect the data structure used to store URB data + * @usb_mutex: mutex of USB control messages (reading needs two messages). + * Please notice that this mutex is used internally at the generic + * URB control functions. So, drivers using dvb_usb_generic_rw() and + * derivated functions should not lock it internally. + * @i2c_mutex: mutex for i2c-transfers * * @i2c_adap: device's i2c_adapter if it uses I2CoverUSB * @@ -433,6 +437,7 @@ struct dvb_usb_device { int powered; /* locking */ + struct mutex data_mutex; struct mutex usb_mutex; /* i2c */ ^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [v4.9-rc4] dvb-usb/cinergyT2 NULL pointer dereference 2016-11-11 13:55 ` Mauro Carvalho Chehab @ 2016-11-11 16:43 ` Jörg Otte 0 siblings, 0 replies; 14+ messages in thread From: Jörg Otte @ 2016-11-11 16:43 UTC (permalink / raw) To: Mauro Carvalho Chehab Cc: Benjamin Larsson, Mauro Carvalho Chehab, Linus Torvalds, Patrick Boettcher, Mauro Carvalho Chehab, Linux Kernel Mailing List, Linux Media Mailing List 2016-11-11 14:55 GMT+01:00 Mauro Carvalho Chehab <mchehab@osg.samsung.com>: > Em Thu, 10 Nov 2016 12:15:39 +0100 > Jörg Otte <jrg.otte@gmail.com> escreveu: > >> 2016-11-10 9:40 GMT+01:00 Mauro Carvalho Chehab <mchehab@s-opensource.com>: >> > Em Wed, 9 Nov 2016 11:07:35 -0800 >> > Linus Torvalds <torvalds@linux-foundation.org> escreveu: >> > >> >> On Wed, Nov 9, 2016 at 3:09 AM, Jörg Otte <jrg.otte@gmail.com> wrote: >> >> > >> >> > Tried patch with no success. Again a NULL ptr dereferece. >> >> >> >> I suspect a much simpler approach is to just miove the "data_mutex" >> >> away from the priv area and into "struct dvb_usb_device" and >> >> "dvb_usb_adapter". Sure, that grows those structures a tiny bit, and >> >> not every driver may need that mutex, but it simplifies things >> >> enormously. Mauro? >> >> >> >> Linus > >> The patch works for me. > > Thanks for testing! That's the (hopefully) final version of it, > with the fix applied to the other dvb-usb drivers that use > data_mutex (except for the frontend ones, with uses a different > private structure, and where the mutex is initialized at attach). > > Benjamin, > > Could you please test it? > > Thanks! > Mauro > > - > > [PATCH] dvb-usb: move data_mutex to struct dvb_usb_device > > The data_mutex is initialized too late, as it is needed for > each device driver's power control, causing an OOPS: > > dvb-usb: found a 'TerraTec/qanu USB2.0 Highspeed DVB-T Receiver' in warm state. > BUG: unable to handle kernel NULL pointer dereference at (null) > IP: [<ffffffff846617af>] __mutex_lock_slowpath+0x6f/0x100 PGD 0 > Oops: 0002 [#1] SMP > Modules linked in: dvb_usb_cinergyT2(+) dvb_usb > CPU: 0 PID: 2029 Comm: modprobe Not tainted 4.9.0-rc4-dvbmod #24 > Hardware name: FUJITSU LIFEBOOK A544/FJNBB35 , BIOS Version 1.17 05/09/2014 > task: ffff88020e943840 task.stack: ffff8801f36ec000 > RIP: 0010:[<ffffffff846617af>] [<ffffffff846617af>] __mutex_lock_slowpath+0x6f/0x100 > RSP: 0018:ffff8801f36efb10 EFLAGS: 00010282 > RAX: 0000000000000000 RBX: ffff88021509bdc8 RCX: 00000000c0000100 > RDX: 0000000000000001 RSI: 0000000000000000 RDI: ffff88021509bdcc > RBP: ffff8801f36efb58 R08: ffff88021f216320 R09: 0000000000100000 > R10: ffff88021f216320 R11: 00000023fee6c5a1 R12: ffff88020e943840 > R13: ffff88021509bdcc R14: 00000000ffffffff R15: ffff88021509bdd0 > FS: 00007f21adb86740(0000) GS:ffff88021f200000(0000) knlGS:0000000000000000 > CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 > CR2: 0000000000000000 CR3: 0000000215bce000 CR4: 00000000001406f0 > Stack: > ffff88021509bdd0 0000000000000000 0000000000000000 ffffffffc0137c80 > ffff88021509bdc8 ffff8801f5944000 0000000000000001 ffffffffc0136b00 > ffff880213e52000 ffff88021509bdc8 ffffffff84661856 ffff88021509bd80 > Call Trace: > [<ffffffff84661856>] ? mutex_lock+0x16/0x25 > [<ffffffffc013616f>] ? cinergyt2_power_ctrl+0x1f/0x60 [dvb_usb_cinergyT2] > [<ffffffffc012e67e>] ? dvb_usb_device_init+0x21e/0x5d0 [dvb_usb] > [<ffffffffc0136021>] ? cinergyt2_usb_probe+0x21/0x50 [dvb_usb_cinergyT2] > [<ffffffff844326f3>] ? usb_probe_interface+0xf3/0x2a0 > [<ffffffff8438e348>] ? driver_probe_device+0x208/0x2b0 > [<ffffffff8438e477>] ? __driver_attach+0x87/0x90 > [<ffffffff8438e3f0>] ? driver_probe_device+0x2b0/0x2b0 > [<ffffffff8438c612>] ? bus_for_each_dev+0x52/0x80 > [<ffffffff8438d983>] ? bus_add_driver+0x1a3/0x220 > [<ffffffff8438ec06>] ? driver_register+0x56/0xd0 > [<ffffffff84431527>] ? usb_register_driver+0x77/0x130 > [<ffffffffc013a000>] ? 0xffffffffc013a000 > [<ffffffff84000426>] ? do_one_initcall+0x46/0x180 > [<ffffffff840eb2c8>] ? free_vmap_area_noflush+0x38/0x70 > [<ffffffff840f3844>] ? kmem_cache_alloc+0x84/0xc0 > [<ffffffff840b802c>] ? do_init_module+0x50/0x1be > [<ffffffff84095adb>] ? load_module+0x1d8b/0x2100 > [<ffffffff84093020>] ? find_symbol_in_section+0xa0/0xa0 > [<ffffffff84095fe9>] ? SyS_finit_module+0x89/0x90 > [<ffffffff846637a0>] ? entry_SYSCALL_64_fastpath+0x13/0x94 > Code: e8 a7 1d 00 00 8b 03 83 f8 01 0f 84 97 00 00 00 48 8b 43 10 4c 8d 7b 08 48 89 63 10 4c 89 3c 24 41 be ff ff ff ff 48 89 44 24 08 <48> 89 20 4c 89 64 24 10 eb 1a 49 c7 44 24 08 02 00 00 00 c6 43 RIP [<ffffffff846617af>] __mutex_lock_slowpath+0x6f/0x100 RSP <ffff8801f36efb10> > CR2: 0000000000000000 > > So, move it to the struct dvb_usb_device and initialize it > before calling the driver's callbacks. > > Reported-by: Jörg Otte <jrg.otte@gmail.com> > Signed-off-by: Mauro Carvalho Chehab <mchehab@s-opensource.com> > > diff --git a/drivers/media/usb/dvb-usb/af9005.c b/drivers/media/usb/dvb-usb/af9005.c > index b257780fb380..7853261906b1 100644 > --- a/drivers/media/usb/dvb-usb/af9005.c > +++ b/drivers/media/usb/dvb-usb/af9005.c > @@ -53,7 +53,6 @@ struct af9005_device_state { > u8 sequence; > int led_state; > unsigned char data[256]; > - struct mutex data_mutex; > }; > > static int af9005_generic_read_write(struct dvb_usb_device *d, u16 reg, > @@ -72,7 +71,7 @@ static int af9005_generic_read_write(struct dvb_usb_device *d, u16 reg, > return -EINVAL; > } > > - mutex_lock(&st->data_mutex); > + mutex_lock(&d->data_mutex); > st->data[0] = 14; /* rest of buffer length low */ > st->data[1] = 0; /* rest of buffer length high */ > > @@ -140,7 +139,7 @@ static int af9005_generic_read_write(struct dvb_usb_device *d, u16 reg, > values[i] = st->data[8 + i]; > > ret: > - mutex_unlock(&st->data_mutex); > + mutex_unlock(&d->data_mutex); > return ret; > > } > @@ -481,7 +480,7 @@ int af9005_send_command(struct dvb_usb_device *d, u8 command, u8 * wbuf, > } > packet_len = wlen + 5; > > - mutex_lock(&st->data_mutex); > + mutex_lock(&d->data_mutex); > > st->data[0] = (u8) (packet_len & 0xff); > st->data[1] = (u8) ((packet_len & 0xff00) >> 8); > @@ -512,7 +511,7 @@ int af9005_send_command(struct dvb_usb_device *d, u8 command, u8 * wbuf, > rbuf[i] = st->data[i + 7]; > } > > - mutex_unlock(&st->data_mutex); > + mutex_unlock(&d->data_mutex); > return ret; > } > > @@ -523,7 +522,7 @@ int af9005_read_eeprom(struct dvb_usb_device *d, u8 address, u8 * values, > u8 seq; > int ret, i; > > - mutex_lock(&st->data_mutex); > + mutex_lock(&d->data_mutex); > > memset(st->data, 0, sizeof(st->data)); > > @@ -559,7 +558,7 @@ int af9005_read_eeprom(struct dvb_usb_device *d, u8 address, u8 * values, > for (i = 0; i < len; i++) > values[i] = st->data[6 + i]; > } > - mutex_unlock(&st->data_mutex); > + mutex_unlock(&d->data_mutex); > > return ret; > } > @@ -847,7 +846,7 @@ static int af9005_rc_query(struct dvb_usb_device *d, u32 * event, int *state) > return 0; > } > > - mutex_lock(&st->data_mutex); > + mutex_lock(&d->data_mutex); > > /* deb_info("rc_query\n"); */ > st->data[0] = 3; /* rest of packet length low */ > @@ -890,7 +889,7 @@ static int af9005_rc_query(struct dvb_usb_device *d, u32 * event, int *state) > } > > ret: > - mutex_unlock(&st->data_mutex); > + mutex_unlock(&d->data_mutex); > return ret; > } > > @@ -1004,20 +1003,8 @@ static struct dvb_usb_device_properties af9005_properties; > static int af9005_usb_probe(struct usb_interface *intf, > const struct usb_device_id *id) > { > - struct dvb_usb_device *d; > - struct af9005_device_state *st; > - int ret; > - > - ret = dvb_usb_device_init(intf, &af9005_properties, > - THIS_MODULE, &d, adapter_nr); > - > - if (ret < 0) > - return ret; > - > - st = d->priv; > - mutex_init(&st->data_mutex); > - > - return 0; > + return dvb_usb_device_init(intf, &af9005_properties, > + THIS_MODULE, NULL, adapter_nr); > } > > enum af9005_usb_table_entry { > diff --git a/drivers/media/usb/dvb-usb/cinergyT2-core.c b/drivers/media/usb/dvb-usb/cinergyT2-core.c > index 8ac825413d5a..290275bc7fde 100644 > --- a/drivers/media/usb/dvb-usb/cinergyT2-core.c > +++ b/drivers/media/usb/dvb-usb/cinergyT2-core.c > @@ -42,7 +42,6 @@ DVB_DEFINE_MOD_OPT_ADAPTER_NR(adapter_nr); > struct cinergyt2_state { > u8 rc_counter; > unsigned char data[64]; > - struct mutex data_mutex; > }; > > /* We are missing a release hook with usb_device data */ > @@ -56,12 +55,12 @@ static int cinergyt2_streaming_ctrl(struct dvb_usb_adapter *adap, int enable) > struct cinergyt2_state *st = d->priv; > int ret; > > - mutex_lock(&st->data_mutex); > + mutex_lock(&d->data_mutex); > st->data[0] = CINERGYT2_EP1_CONTROL_STREAM_TRANSFER; > st->data[1] = enable ? 1 : 0; > > ret = dvb_usb_generic_rw(d, st->data, 2, st->data, 64, 0); > - mutex_unlock(&st->data_mutex); > + mutex_unlock(&d->data_mutex); > > return ret; > } > @@ -71,12 +70,12 @@ static int cinergyt2_power_ctrl(struct dvb_usb_device *d, int enable) > struct cinergyt2_state *st = d->priv; > int ret; > > - mutex_lock(&st->data_mutex); > + mutex_lock(&d->data_mutex); > st->data[0] = CINERGYT2_EP1_SLEEP_MODE; > st->data[1] = enable ? 0 : 1; > > ret = dvb_usb_generic_rw(d, st->data, 2, st->data, 3, 0); > - mutex_unlock(&st->data_mutex); > + mutex_unlock(&d->data_mutex); > > return ret; > } > @@ -89,7 +88,7 @@ static int cinergyt2_frontend_attach(struct dvb_usb_adapter *adap) > > adap->fe_adap[0].fe = cinergyt2_fe_attach(adap->dev); > > - mutex_lock(&st->data_mutex); > + mutex_lock(&d->data_mutex); > st->data[0] = CINERGYT2_EP1_GET_FIRMWARE_VERSION; > > ret = dvb_usb_generic_rw(d, st->data, 1, st->data, 3, 0); > @@ -97,7 +96,7 @@ static int cinergyt2_frontend_attach(struct dvb_usb_adapter *adap) > deb_rc("cinergyt2_power_ctrl() Failed to retrieve sleep " > "state info\n"); > } > - mutex_unlock(&st->data_mutex); > + mutex_unlock(&d->data_mutex); > > /* Copy this pointer as we are gonna need it in the release phase */ > cinergyt2_usb_device = adap->dev; > @@ -166,7 +165,7 @@ static int cinergyt2_rc_query(struct dvb_usb_device *d, u32 *event, int *state) > > *state = REMOTE_NO_KEY_PRESSED; > > - mutex_lock(&st->data_mutex); > + mutex_lock(&d->data_mutex); > st->data[0] = CINERGYT2_EP1_GET_RC_EVENTS; > > ret = dvb_usb_generic_rw(d, st->data, 1, st->data, 5, 0); > @@ -202,29 +201,17 @@ static int cinergyt2_rc_query(struct dvb_usb_device *d, u32 *event, int *state) > } > > ret: > - mutex_unlock(&st->data_mutex); > + mutex_unlock(&d->data_mutex); > return ret; > } > > static int cinergyt2_usb_probe(struct usb_interface *intf, > const struct usb_device_id *id) > { > - struct dvb_usb_device *d; > - struct cinergyt2_state *st; > - int ret; > - > - ret = dvb_usb_device_init(intf, &cinergyt2_properties, > - THIS_MODULE, &d, adapter_nr); > - if (ret < 0) > - return ret; > - > - st = d->priv; > - mutex_init(&st->data_mutex); > - > - return 0; > + return dvb_usb_device_init(intf, &cinergyt2_properties, > + THIS_MODULE, NULL, adapter_nr); > } > > - > static struct usb_device_id cinergyt2_usb_table[] = { > { USB_DEVICE(USB_VID_TERRATEC, 0x0038) }, > { 0 } > diff --git a/drivers/media/usb/dvb-usb/cxusb.c b/drivers/media/usb/dvb-usb/cxusb.c > index 39772812269d..243403081fa5 100644 > --- a/drivers/media/usb/dvb-usb/cxusb.c > +++ b/drivers/media/usb/dvb-usb/cxusb.c > @@ -68,7 +68,7 @@ static int cxusb_ctrl_msg(struct dvb_usb_device *d, > > wo = (rbuf == NULL || rlen == 0); /* write-only */ > > - mutex_lock(&st->data_mutex); > + mutex_lock(&d->data_mutex); > st->data[0] = cmd; > memcpy(&st->data[1], wbuf, wlen); > if (wo) > @@ -77,7 +77,7 @@ static int cxusb_ctrl_msg(struct dvb_usb_device *d, > ret = dvb_usb_generic_rw(d, st->data, 1 + wlen, > rbuf, rlen, 0); > > - mutex_unlock(&st->data_mutex); > + mutex_unlock(&d->data_mutex); > return ret; > } > > @@ -1461,43 +1461,36 @@ static struct dvb_usb_device_properties cxusb_mygica_t230_properties; > static int cxusb_probe(struct usb_interface *intf, > const struct usb_device_id *id) > { > - struct dvb_usb_device *d; > - struct cxusb_state *st; > - > if (0 == dvb_usb_device_init(intf, &cxusb_medion_properties, > - THIS_MODULE, &d, adapter_nr) || > + THIS_MODULE, NULL, adapter_nr) || > 0 == dvb_usb_device_init(intf, &cxusb_bluebird_lgh064f_properties, > - THIS_MODULE, &d, adapter_nr) || > + THIS_MODULE, NULL, adapter_nr) || > 0 == dvb_usb_device_init(intf, &cxusb_bluebird_dee1601_properties, > - THIS_MODULE, &d, adapter_nr) || > + THIS_MODULE, NULL, adapter_nr) || > 0 == dvb_usb_device_init(intf, &cxusb_bluebird_lgz201_properties, > - THIS_MODULE, &d, adapter_nr) || > + THIS_MODULE, NULL, adapter_nr) || > 0 == dvb_usb_device_init(intf, &cxusb_bluebird_dtt7579_properties, > - THIS_MODULE, &d, adapter_nr) || > + THIS_MODULE, NULL, adapter_nr) || > 0 == dvb_usb_device_init(intf, &cxusb_bluebird_dualdig4_properties, > - THIS_MODULE, &d, adapter_nr) || > + THIS_MODULE, NULL, adapter_nr) || > 0 == dvb_usb_device_init(intf, &cxusb_bluebird_nano2_properties, > - THIS_MODULE, &d, adapter_nr) || > + THIS_MODULE, NULL, adapter_nr) || > 0 == dvb_usb_device_init(intf, > &cxusb_bluebird_nano2_needsfirmware_properties, > - THIS_MODULE, &d, adapter_nr) || > + THIS_MODULE, NULL, adapter_nr) || > 0 == dvb_usb_device_init(intf, &cxusb_aver_a868r_properties, > - THIS_MODULE, &d, adapter_nr) || > + THIS_MODULE, NULL, adapter_nr) || > 0 == dvb_usb_device_init(intf, > &cxusb_bluebird_dualdig4_rev2_properties, > - THIS_MODULE, &d, adapter_nr) || > + THIS_MODULE, NULL, adapter_nr) || > 0 == dvb_usb_device_init(intf, &cxusb_d680_dmb_properties, > - THIS_MODULE, &d, adapter_nr) || > + THIS_MODULE, NULL, adapter_nr) || > 0 == dvb_usb_device_init(intf, &cxusb_mygica_d689_properties, > - THIS_MODULE, &d, adapter_nr) || > + THIS_MODULE, NULL, adapter_nr) || > 0 == dvb_usb_device_init(intf, &cxusb_mygica_t230_properties, > - THIS_MODULE, &d, adapter_nr) || > - 0) { > - st = d->priv; > - mutex_init(&st->data_mutex); > - > + THIS_MODULE, NULL, adapter_nr) || > + 0) > return 0; > - } > > return -EINVAL; > } > diff --git a/drivers/media/usb/dvb-usb/cxusb.h b/drivers/media/usb/dvb-usb/cxusb.h > index 9f3ee0e47d5c..18acda19527a 100644 > --- a/drivers/media/usb/dvb-usb/cxusb.h > +++ b/drivers/media/usb/dvb-usb/cxusb.h > @@ -37,7 +37,6 @@ struct cxusb_state { > struct i2c_client *i2c_client_tuner; > > unsigned char data[MAX_XFER_SIZE]; > - struct mutex data_mutex; > }; > > #endif > diff --git a/drivers/media/usb/dvb-usb/dtt200u.c b/drivers/media/usb/dvb-usb/dtt200u.c > index f88572c7ae7c..fcbff7fb0c4e 100644 > --- a/drivers/media/usb/dvb-usb/dtt200u.c > +++ b/drivers/media/usb/dvb-usb/dtt200u.c > @@ -22,7 +22,6 @@ DVB_DEFINE_MOD_OPT_ADAPTER_NR(adapter_nr); > > struct dtt200u_state { > unsigned char data[80]; > - struct mutex data_mutex; > }; > > static int dtt200u_power_ctrl(struct dvb_usb_device *d, int onoff) > @@ -30,23 +29,24 @@ static int dtt200u_power_ctrl(struct dvb_usb_device *d, int onoff) > struct dtt200u_state *st = d->priv; > int ret = 0; > > - mutex_lock(&st->data_mutex); > + mutex_lock(&d->data_mutex); > > st->data[0] = SET_INIT; > > if (onoff) > ret = dvb_usb_generic_write(d, st->data, 2); > > - mutex_unlock(&st->data_mutex); > + mutex_unlock(&d->data_mutex); > return ret; > } > > static int dtt200u_streaming_ctrl(struct dvb_usb_adapter *adap, int onoff) > { > - struct dtt200u_state *st = adap->dev->priv; > + struct dvb_usb_device *d = adap->dev; > + struct dtt200u_state *st = d->priv; > int ret; > > - mutex_lock(&st->data_mutex); > + mutex_lock(&d->data_mutex); > st->data[0] = SET_STREAMING; > st->data[1] = onoff; > > @@ -61,26 +61,27 @@ static int dtt200u_streaming_ctrl(struct dvb_usb_adapter *adap, int onoff) > ret = dvb_usb_generic_write(adap->dev, st->data, 1); > > ret: > - mutex_unlock(&st->data_mutex); > + mutex_unlock(&d->data_mutex); > > return ret; > } > > static int dtt200u_pid_filter(struct dvb_usb_adapter *adap, int index, u16 pid, int onoff) > { > - struct dtt200u_state *st = adap->dev->priv; > + struct dvb_usb_device *d = adap->dev; > + struct dtt200u_state *st = d->priv; > int ret; > > pid = onoff ? pid : 0; > > - mutex_lock(&st->data_mutex); > + mutex_lock(&d->data_mutex); > st->data[0] = SET_PID_FILTER; > st->data[1] = index; > st->data[2] = pid & 0xff; > st->data[3] = (pid >> 8) & 0x1f; > > ret = dvb_usb_generic_write(adap->dev, st->data, 4); > - mutex_unlock(&st->data_mutex); > + mutex_unlock(&d->data_mutex); > > return ret; > } > @@ -91,7 +92,7 @@ static int dtt200u_rc_query(struct dvb_usb_device *d) > u32 scancode; > int ret; > > - mutex_lock(&st->data_mutex); > + mutex_lock(&d->data_mutex); > st->data[0] = GET_RC_CODE; > > ret = dvb_usb_generic_rw(d, st->data, 1, st->data, 5, 0); > @@ -126,7 +127,7 @@ static int dtt200u_rc_query(struct dvb_usb_device *d) > deb_info("st->data: %*ph\n", 5, st->data); > > ret: > - mutex_unlock(&st->data_mutex); > + mutex_unlock(&d->data_mutex); > return ret; > } > > @@ -145,24 +146,17 @@ static struct dvb_usb_device_properties wt220u_miglia_properties; > static int dtt200u_usb_probe(struct usb_interface *intf, > const struct usb_device_id *id) > { > - struct dvb_usb_device *d; > - struct dtt200u_state *st; > - > if (0 == dvb_usb_device_init(intf, &dtt200u_properties, > - THIS_MODULE, &d, adapter_nr) || > + THIS_MODULE, NULL, adapter_nr) || > 0 == dvb_usb_device_init(intf, &wt220u_properties, > - THIS_MODULE, &d, adapter_nr) || > + THIS_MODULE, NULL, adapter_nr) || > 0 == dvb_usb_device_init(intf, &wt220u_fc_properties, > - THIS_MODULE, &d, adapter_nr) || > + THIS_MODULE, NULL, adapter_nr) || > 0 == dvb_usb_device_init(intf, &wt220u_zl0353_properties, > - THIS_MODULE, &d, adapter_nr) || > + THIS_MODULE, NULL, adapter_nr) || > 0 == dvb_usb_device_init(intf, &wt220u_miglia_properties, > - THIS_MODULE, &d, adapter_nr)) { > - st = d->priv; > - mutex_init(&st->data_mutex); > - > + THIS_MODULE, NULL, adapter_nr)) > return 0; > - } > > return -ENODEV; > } > diff --git a/drivers/media/usb/dvb-usb/dvb-usb-init.c b/drivers/media/usb/dvb-usb/dvb-usb-init.c > index 3896ba9a4179..84308569e7dc 100644 > --- a/drivers/media/usb/dvb-usb/dvb-usb-init.c > +++ b/drivers/media/usb/dvb-usb/dvb-usb-init.c > @@ -142,6 +142,7 @@ static int dvb_usb_init(struct dvb_usb_device *d, short *adapter_nums) > { > int ret = 0; > > + mutex_init(&d->data_mutex); > mutex_init(&d->usb_mutex); > mutex_init(&d->i2c_mutex); > > diff --git a/drivers/media/usb/dvb-usb/dvb-usb.h b/drivers/media/usb/dvb-usb/dvb-usb.h > index 1448c3d27ea2..67f898b6f6d0 100644 > --- a/drivers/media/usb/dvb-usb/dvb-usb.h > +++ b/drivers/media/usb/dvb-usb/dvb-usb.h > @@ -404,8 +404,12 @@ struct dvb_usb_adapter { > * Powered is in/decremented for each call to modify the state. > * @udev: pointer to the device's struct usb_device. > * > - * @usb_mutex: semaphore of USB control messages (reading needs two messages) > - * @i2c_mutex: semaphore for i2c-transfers > + * @data_mutex: mutex to protect the data structure used to store URB data > + * @usb_mutex: mutex of USB control messages (reading needs two messages). > + * Please notice that this mutex is used internally at the generic > + * URB control functions. So, drivers using dvb_usb_generic_rw() and > + * derivated functions should not lock it internally. > + * @i2c_mutex: mutex for i2c-transfers > * > * @i2c_adap: device's i2c_adapter if it uses I2CoverUSB > * > @@ -433,6 +437,7 @@ struct dvb_usb_device { > int powered; > > /* locking */ > + struct mutex data_mutex; > struct mutex usb_mutex; > > /* i2c */ The patch works for me. Note that my build environment builds a minimum kernel and only compiles cinergyT2 parts of the patch. Thanks, Jörg ^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2016-11-11 16:43 UTC | newest] Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2016-11-06 15:40 [v4.9-rc4] dvb-usb/cinergyT2 NULL pointer dereference Jörg Otte 2016-11-08 18:42 ` Linus Torvalds 2016-11-08 20:22 ` Mauro Carvalho Chehab 2016-11-08 21:15 ` Benjamin Larsson 2016-11-08 21:38 ` Mauro Carvalho Chehab 2016-11-09 19:57 ` Malcolm Priestley 2016-11-09 20:25 ` Mauro Carvalho Chehab 2016-11-09 11:09 ` Jörg Otte 2016-11-09 19:07 ` Linus Torvalds 2016-11-09 20:21 ` Mauro Carvalho Chehab 2016-11-10 8:40 ` Mauro Carvalho Chehab 2016-11-10 11:15 ` Jörg Otte 2016-11-11 13:55 ` Mauro Carvalho Chehab 2016-11-11 16:43 ` Jörg Otte
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).