* [PATCH] dvb: usb: fix use after free in dvb_usb_device_exit @ 2019-04-30 13:07 Oliver Neukum 2019-08-20 18:55 ` Ben Hutchings 0 siblings, 1 reply; 7+ messages in thread From: Oliver Neukum @ 2019-04-30 13:07 UTC (permalink / raw) To: mchehab, linux-media; +Cc: Oliver Neukum dvb_usb_device_exit() frees and uses the device name in that order Fix by storing the name in a buffer before freeing it v2: fixed style issues v3: strscpy used and variable names changed v4: really use strscpy everywhere Signed-off-by: Oliver Neukum <oneukum@suse.com> Reported-by: syzbot+26ec41e9f788b3eba396@syzkaller.appspotmail.com --- drivers/media/usb/dvb-usb/dvb-usb-init.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/drivers/media/usb/dvb-usb/dvb-usb-init.c b/drivers/media/usb/dvb-usb/dvb-usb-init.c index 99951e02a880..dd063a736df5 100644 --- a/drivers/media/usb/dvb-usb/dvb-usb-init.c +++ b/drivers/media/usb/dvb-usb/dvb-usb-init.c @@ -287,12 +287,15 @@ EXPORT_SYMBOL(dvb_usb_device_init); void dvb_usb_device_exit(struct usb_interface *intf) { struct dvb_usb_device *d = usb_get_intfdata(intf); - const char *name = "generic DVB-USB module"; + const char *default_name = "generic DVB-USB module"; + char name[40]; usb_set_intfdata(intf, NULL); if (d != NULL && d->desc != NULL) { - name = d->desc->name; + strscpy(name, d->desc->name, sizeof(name)); dvb_usb_exit(d); + } else { + strscpy(name, default_name, sizeof(name)); } info("%s successfully deinitialized and disconnected.", name); -- 2.16.4 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH] dvb: usb: fix use after free in dvb_usb_device_exit 2019-04-30 13:07 [PATCH] dvb: usb: fix use after free in dvb_usb_device_exit Oliver Neukum @ 2019-08-20 18:55 ` Ben Hutchings 2019-08-21 8:18 ` Oliver Neukum 0 siblings, 1 reply; 7+ messages in thread From: Ben Hutchings @ 2019-08-20 18:55 UTC (permalink / raw) To: Oliver Neukum, mchehab; +Cc: linux-media, Anton Vasilyev [-- Attachment #1: Type: text/plain, Size: 2466 bytes --] On Tue, 2019-04-30 at 15:07 +0200, Oliver Neukum wrote: > dvb_usb_device_exit() frees and uses the device name in that order > Fix by storing the name in a buffer before freeing it > > v2: fixed style issues > v3: strscpy used and variable names changed > v4: really use strscpy everywhere > > Signed-off-by: Oliver Neukum <oneukum@suse.com> > Reported-by: syzbot+26ec41e9f788b3eba396@syzkaller.appspotmail.com This doesn't fix that bug (and I don't think it fixes a bug at all). The name string is static and doesn't get freed until the module it's in is freed. Look again at the stack traces in <https://syzkaller.appspot.com/bug?extid=26ec41e9f788b3eba396>: > Allocated by task 21: [...] > kmemdup+0x23/0x50 mm/util.c:118 > kmemdup include/linux/string.h:428 [inline] > dw2102_probe+0x62c/0xc50 drivers/media/usb/dvb-usb/dw2102.c:2375 [...] > Freed by task 21: [...] > kfree+0xce/0x290 mm/slub.c:3958 > dw2102_probe+0x876/0xc50 drivers/media/usb/dvb-usb/dw2102.c:2409 So, d->desc was freed during probe, and is a dangling pointer before dvb_usb_device_exit() runs at all. The bug seems to have been introduced by: commit 299c7007e93645067e1d2743f4e50156de78c4ff Author: Anton Vasilyev <vasilyev@ispras.ru> Date: Mon Jul 23 13:04:54 2018 -0400 media: dw2102: Fix memleak on sequence of probes Ben. > --- > drivers/media/usb/dvb-usb/dvb-usb-init.c | 7 +++++-- > 1 file changed, 5 insertions(+), 2 deletions(-) > > diff --git a/drivers/media/usb/dvb-usb/dvb-usb-init.c b/drivers/media/usb/dvb-usb/dvb-usb-init.c > index 99951e02a880..dd063a736df5 100644 > --- a/drivers/media/usb/dvb-usb/dvb-usb-init.c > +++ b/drivers/media/usb/dvb-usb/dvb-usb-init.c > @@ -287,12 +287,15 @@ EXPORT_SYMBOL(dvb_usb_device_init); > void dvb_usb_device_exit(struct usb_interface *intf) > { > struct dvb_usb_device *d = usb_get_intfdata(intf); > - const char *name = "generic DVB-USB module"; > + const char *default_name = "generic DVB-USB module"; > + char name[40]; > > usb_set_intfdata(intf, NULL); > if (d != NULL && d->desc != NULL) { > - name = d->desc->name; > + strscpy(name, d->desc->name, sizeof(name)); > dvb_usb_exit(d); > + } else { > + strscpy(name, default_name, sizeof(name)); > } > info("%s successfully deinitialized and disconnected.", name); > -- Ben Hutchings Experience is what causes a person to make new mistakes instead of old ones. [-- Attachment #2: This is a digitally signed message part --] [-- Type: application/pgp-signature, Size: 833 bytes --] ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] dvb: usb: fix use after free in dvb_usb_device_exit 2019-08-20 18:55 ` Ben Hutchings @ 2019-08-21 8:18 ` Oliver Neukum 2019-08-21 12:47 ` Ben Hutchings 0 siblings, 1 reply; 7+ messages in thread From: Oliver Neukum @ 2019-08-21 8:18 UTC (permalink / raw) To: Ben Hutchings, mchehab; +Cc: Anton Vasilyev, linux-media Am Dienstag, den 20.08.2019, 19:55 +0100 schrieb Ben Hutchings: > On Tue, 2019-04-30 at 15:07 +0200, Oliver Neukum wrote: > > dvb_usb_device_exit() frees and uses the device name in that order > > Fix by storing the name in a buffer before freeing it > > > > v2: fixed style issues > > v3: strscpy used and variable names changed > > v4: really use strscpy everywhere > > > > Signed-off-by: Oliver Neukum <oneukum@suse.com> > > Reported-by: syzbot+26ec41e9f788b3eba396@syzkaller.appspotmail.com > > This doesn't fix that bug (and I don't think it fixes a bug at all). > The name string is static and doesn't get freed until the module it's > in is freed. I see. > Look again at the stack traces in > <https://syzkaller.appspot.com/bug?extid=26ec41e9f788b3eba396>: > > > Allocated by task 21: > > [...] > > kmemdup+0x23/0x50 mm/util.c:118 > > > > kmemdup include/linux/string.h:428 [inline] > > dw2102_probe+0x62c/0xc50 > > drivers/media/usb/dvb-usb/dw2102.c:2375 > [...] > > Freed by task 21: > > [...] > > > > kfree+0xce/0x290 mm/slub.c:3958 > > dw2102_probe+0x876/0xc50 > > drivers/media/usb/dvb-usb/dw2102.c:2409 > > So, d->desc was freed during probe, and is a dangling pointer before > dvb_usb_device_exit() runs at all. In that case KASAN would have reported a double free in testing, which it did not. > The bug seems to have been introduced by: > > commit 299c7007e93645067e1d2743f4e50156de78c4ff > Author: Anton Vasilyev <vasilyev@ispras.ru> > Date: Mon Jul 23 13:04:54 2018 -0400 > > media: dw2102: Fix memleak on sequence of probes AFAICT this patch only does anything if probe() succeeds, which it does not. Something is strange. Regards Oliver ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] dvb: usb: fix use after free in dvb_usb_device_exit 2019-08-21 8:18 ` Oliver Neukum @ 2019-08-21 12:47 ` Ben Hutchings 2019-08-22 10:41 ` [PATCH] media: dw2102: Fix use after free Anton Vasilyev 0 siblings, 1 reply; 7+ messages in thread From: Ben Hutchings @ 2019-08-21 12:47 UTC (permalink / raw) To: Oliver Neukum, mchehab; +Cc: Anton Vasilyev, linux-media [-- Attachment #1: Type: text/plain, Size: 2276 bytes --] On Wed, 2019-08-21 at 10:18 +0200, Oliver Neukum wrote: > Am Dienstag, den 20.08.2019, 19:55 +0100 schrieb Ben Hutchings: > > On Tue, 2019-04-30 at 15:07 +0200, Oliver Neukum wrote: > > > dvb_usb_device_exit() frees and uses the device name in that order > > > Fix by storing the name in a buffer before freeing it > > > > > > v2: fixed style issues > > > v3: strscpy used and variable names changed > > > v4: really use strscpy everywhere > > > > > > Signed-off-by: Oliver Neukum <oneukum@suse.com> > > > Reported-by: syzbot+26ec41e9f788b3eba396@syzkaller.appspotmail.com > > > > This doesn't fix that bug (and I don't think it fixes a bug at all). > > The name string is static and doesn't get freed until the module it's > > in is freed. > > I see. > > > Look again at the stack traces in > > <https://syzkaller.appspot.com/bug?extid=26ec41e9f788b3eba396>;: > > > > > Allocated by task 21: > > > > [...] > > > kmemdup+0x23/0x50 mm/util.c:118 > > > > > > > kmemdup include/linux/string.h:428 [inline] > > > dw2102_probe+0x62c/0xc50 > > > > drivers/media/usb/dvb-usb/dw2102.c:2375 > > [...] > > > Freed by task 21: > > > > [...] > > > > kfree+0xce/0x290 mm/slub.c:3958 > > > dw2102_probe+0x876/0xc50 > > > > drivers/media/usb/dvb-usb/dw2102.c:2409 > > > > So, d->desc was freed during probe, and is a dangling pointer before > > dvb_usb_device_exit() runs at all. > > In that case KASAN would have reported a double free in testing, which > it did not. So far as I could see, the descriptors are normally static data in the driver and nothing in the DVB core frees them. dw2102 is unusual in that it heap-allocated descriptors, which is why this patched fixes a leak: > > The bug seems to have been introduced by: > > > > commit 299c7007e93645067e1d2743f4e50156de78c4ff > > Author: Anton Vasilyev <vasilyev@ispras.ru> > > Date: Mon Jul 23 13:04:54 2018 -0400 > > > > media: dw2102: Fix memleak on sequence of probes > > AFAICT this patch only does anything if probe() succeeds, which it does > not. Something is strange. How did you determine that it doesn't succeed? Ben. -- Ben Hutchings Quantity is no substitute for quality, but it's the only one we've got. [-- Attachment #2: This is a digitally signed message part --] [-- Type: application/pgp-signature, Size: 833 bytes --] ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH] media: dw2102: Fix use after free 2019-08-21 12:47 ` Ben Hutchings @ 2019-08-22 10:41 ` Anton Vasilyev 2019-08-29 16:56 ` Kees Cook 0 siblings, 1 reply; 7+ messages in thread From: Anton Vasilyev @ 2019-08-22 10:41 UTC (permalink / raw) To: Mauro Carvalho Chehab Cc: Anton Vasilyev, Greg Kroah-Hartman, Thomas Gleixner, Kees Cook, Oliver Neukum, Ben Hutchings, ldv-project, linux-media, linux-kernel dvb_usb_device_init stores parts of properties at d->props and d->desc and uses it on dvb_usb_device_exit. Free of properties on module probe leads to use after free. Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=204597 The patch makes properties static instead of allocated on heap to prevent memleak and use after free. Also fixes s421_properties.devices initialization to have 2 element instead of 6 copied from p7500_properties. Signed-off-by: Anton Vasilyev <vasilyev@ispras.ru> Fixes: 299c7007e936 ("media: dw2102: Fix memleak on sequence of probes") --- drivers/media/usb/dvb-usb/dw2102.c | 338 ++++++++++++++++++----------- 1 file changed, 215 insertions(+), 123 deletions(-) diff --git a/drivers/media/usb/dvb-usb/dw2102.c b/drivers/media/usb/dvb-usb/dw2102.c index b960abd00d48..7ea3aa0fee40 100644 --- a/drivers/media/usb/dvb-usb/dw2102.c +++ b/drivers/media/usb/dvb-usb/dw2102.c @@ -2098,46 +2098,153 @@ static struct dvb_usb_device_properties s6x0_properties = { } }; -static const struct dvb_usb_device_description d1100 = { - "Prof 1100 USB ", - {&dw2102_table[PROF_1100], NULL}, - {NULL}, -}; +static struct dvb_usb_device_properties p1100_properties = { + .caps = DVB_USB_IS_AN_I2C_ADAPTER, + .usb_ctrl = DEVICE_SPECIFIC, + .size_of_priv = sizeof(struct dw2102_state), + .firmware = P1100_FIRMWARE, + .no_reconnect = 1, -static const struct dvb_usb_device_description d660 = { - "TeVii S660 USB", - {&dw2102_table[TEVII_S660], NULL}, - {NULL}, -}; + .i2c_algo = &s6x0_i2c_algo, + .rc.core = { + .rc_interval = 150, + .rc_codes = RC_MAP_TBS_NEC, + .module_name = "dw2102", + .allowed_protos = RC_PROTO_BIT_NEC, + .rc_query = prof_rc_query, + }, -static const struct dvb_usb_device_description d480_1 = { - "TeVii S480.1 USB", - {&dw2102_table[TEVII_S480_1], NULL}, - {NULL}, + .generic_bulk_ctrl_endpoint = 0x81, + .num_adapters = 1, + .download_firmware = dw2102_load_firmware, + .read_mac_address = s6x0_read_mac_address, + .adapter = { + { + .num_frontends = 1, + .fe = {{ + .frontend_attach = stv0288_frontend_attach, + .stream = { + .type = USB_BULK, + .count = 8, + .endpoint = 0x82, + .u = { + .bulk = { + .buffersize = 4096, + } + } + }, + } }, + } + }, + .num_device_descs = 1, + .devices = { + {"Prof 1100 USB ", + {&dw2102_table[PROF_1100], NULL}, + {NULL}, + }, + } }; -static const struct dvb_usb_device_description d480_2 = { - "TeVii S480.2 USB", - {&dw2102_table[TEVII_S480_2], NULL}, - {NULL}, -}; +static struct dvb_usb_device_properties s660_properties = { + .caps = DVB_USB_IS_AN_I2C_ADAPTER, + .usb_ctrl = DEVICE_SPECIFIC, + .size_of_priv = sizeof(struct dw2102_state), + .firmware = S660_FIRMWARE, + .no_reconnect = 1, -static const struct dvb_usb_device_description d7500 = { - "Prof 7500 USB DVB-S2", - {&dw2102_table[PROF_7500], NULL}, - {NULL}, -}; + .i2c_algo = &s6x0_i2c_algo, + .rc.core = { + .rc_interval = 150, + .rc_codes = RC_MAP_TEVII_NEC, + .module_name = "dw2102", + .allowed_protos = RC_PROTO_BIT_NEC, + .rc_query = dw2102_rc_query, + }, -static const struct dvb_usb_device_description d421 = { - "TeVii S421 PCI", - {&dw2102_table[TEVII_S421], NULL}, - {NULL}, + .generic_bulk_ctrl_endpoint = 0x81, + .num_adapters = 1, + .download_firmware = dw2102_load_firmware, + .read_mac_address = s6x0_read_mac_address, + .adapter = { + { + .num_frontends = 1, + .fe = {{ + .frontend_attach = ds3000_frontend_attach, + .stream = { + .type = USB_BULK, + .count = 8, + .endpoint = 0x82, + .u = { + .bulk = { + .buffersize = 4096, + } + } + }, + } }, + } + }, + .num_device_descs = 3, + .devices = { + {"TeVii S660 USB", + {&dw2102_table[TEVII_S660], NULL}, + {NULL}, + }, + {"TeVii S480.1 USB", + {&dw2102_table[TEVII_S480_1], NULL}, + {NULL}, + }, + {"TeVii S480.2 USB", + {&dw2102_table[TEVII_S480_2], NULL}, + {NULL}, + }, + } }; -static const struct dvb_usb_device_description d632 = { - "TeVii S632 USB", - {&dw2102_table[TEVII_S632], NULL}, - {NULL}, +static struct dvb_usb_device_properties p7500_properties = { + .caps = DVB_USB_IS_AN_I2C_ADAPTER, + .usb_ctrl = DEVICE_SPECIFIC, + .size_of_priv = sizeof(struct dw2102_state), + .firmware = P7500_FIRMWARE, + .no_reconnect = 1, + + .i2c_algo = &s6x0_i2c_algo, + .rc.core = { + .rc_interval = 150, + .rc_codes = RC_MAP_TBS_NEC, + .module_name = "dw2102", + .allowed_protos = RC_PROTO_BIT_NEC, + .rc_query = prof_rc_query, + }, + + .generic_bulk_ctrl_endpoint = 0x81, + .num_adapters = 1, + .download_firmware = dw2102_load_firmware, + .read_mac_address = s6x0_read_mac_address, + .adapter = { + { + .num_frontends = 1, + .fe = {{ + .frontend_attach = prof_7500_frontend_attach, + .stream = { + .type = USB_BULK, + .count = 8, + .endpoint = 0x82, + .u = { + .bulk = { + .buffersize = 4096, + } + } + }, + } }, + } + }, + .num_device_descs = 1, + .devices = { + {"Prof 7500 USB DVB-S2", + {&dw2102_table[PROF_7500], NULL}, + {NULL}, + }, + } }; static struct dvb_usb_device_properties su3000_properties = { @@ -2209,6 +2316,59 @@ static struct dvb_usb_device_properties su3000_properties = { } }; +static struct dvb_usb_device_properties s421_properties = { + .caps = DVB_USB_IS_AN_I2C_ADAPTER, + .usb_ctrl = DEVICE_SPECIFIC, + .size_of_priv = sizeof(struct dw2102_state), + .power_ctrl = su3000_power_ctrl, + .num_adapters = 1, + .identify_state = su3000_identify_state, + .i2c_algo = &su3000_i2c_algo, + + .rc.core = { + .rc_interval = 150, + .rc_codes = RC_MAP_SU3000, + .module_name = "dw2102", + .allowed_protos = RC_PROTO_BIT_RC5, + .rc_query = su3000_rc_query, + }, + + .read_mac_address = su3000_read_mac_address, + + .generic_bulk_ctrl_endpoint = 0x01, + + .adapter = { + { + .num_frontends = 1, + .fe = {{ + .streaming_ctrl = su3000_streaming_ctrl, + .frontend_attach = m88rs2000_frontend_attach, + .stream = { + .type = USB_BULK, + .count = 8, + .endpoint = 0x82, + .u = { + .bulk = { + .buffersize = 4096, + } + } + } + } }, + } + }, + .num_device_descs = 2, + .devices = { + { "TeVii S421 PCI", + { &dw2102_table[TEVII_S421], NULL }, + { NULL }, + }, + { "TeVii S632 USB", + { &dw2102_table[TEVII_S632], NULL }, + { NULL }, + }, + } +}; + static struct dvb_usb_device_properties t220_properties = { .caps = DVB_USB_IS_AN_I2C_ADAPTER, .usb_ctrl = DEVICE_SPECIFIC, @@ -2326,101 +2486,33 @@ static struct dvb_usb_device_properties tt_s2_4600_properties = { static int dw2102_probe(struct usb_interface *intf, const struct usb_device_id *id) { - int retval = -ENOMEM; - struct dvb_usb_device_properties *p1100; - struct dvb_usb_device_properties *s660; - struct dvb_usb_device_properties *p7500; - struct dvb_usb_device_properties *s421; - - p1100 = kmemdup(&s6x0_properties, - sizeof(struct dvb_usb_device_properties), GFP_KERNEL); - if (!p1100) - goto err0; - - /* copy default structure */ - /* fill only different fields */ - p1100->firmware = P1100_FIRMWARE; - p1100->devices[0] = d1100; - p1100->rc.core.rc_query = prof_rc_query; - p1100->rc.core.rc_codes = RC_MAP_TBS_NEC; - p1100->adapter->fe[0].frontend_attach = stv0288_frontend_attach; - - s660 = kmemdup(&s6x0_properties, - sizeof(struct dvb_usb_device_properties), GFP_KERNEL); - if (!s660) - goto err1; - - s660->firmware = S660_FIRMWARE; - s660->num_device_descs = 3; - s660->devices[0] = d660; - s660->devices[1] = d480_1; - s660->devices[2] = d480_2; - s660->adapter->fe[0].frontend_attach = ds3000_frontend_attach; - - p7500 = kmemdup(&s6x0_properties, - sizeof(struct dvb_usb_device_properties), GFP_KERNEL); - if (!p7500) - goto err2; - - p7500->firmware = P7500_FIRMWARE; - p7500->devices[0] = d7500; - p7500->rc.core.rc_query = prof_rc_query; - p7500->rc.core.rc_codes = RC_MAP_TBS_NEC; - p7500->adapter->fe[0].frontend_attach = prof_7500_frontend_attach; - - - s421 = kmemdup(&su3000_properties, - sizeof(struct dvb_usb_device_properties), GFP_KERNEL); - if (!s421) - goto err3; - - s421->num_device_descs = 2; - s421->devices[0] = d421; - s421->devices[1] = d632; - s421->adapter->fe[0].frontend_attach = m88rs2000_frontend_attach; - - if (0 == dvb_usb_device_init(intf, &dw2102_properties, - THIS_MODULE, NULL, adapter_nr) || - 0 == dvb_usb_device_init(intf, &dw2104_properties, - THIS_MODULE, NULL, adapter_nr) || - 0 == dvb_usb_device_init(intf, &dw3101_properties, - THIS_MODULE, NULL, adapter_nr) || - 0 == dvb_usb_device_init(intf, &s6x0_properties, - THIS_MODULE, NULL, adapter_nr) || - 0 == dvb_usb_device_init(intf, p1100, - THIS_MODULE, NULL, adapter_nr) || - 0 == dvb_usb_device_init(intf, s660, - THIS_MODULE, NULL, adapter_nr) || - 0 == dvb_usb_device_init(intf, p7500, - THIS_MODULE, NULL, adapter_nr) || - 0 == dvb_usb_device_init(intf, s421, - THIS_MODULE, NULL, adapter_nr) || - 0 == dvb_usb_device_init(intf, &su3000_properties, - THIS_MODULE, NULL, adapter_nr) || - 0 == dvb_usb_device_init(intf, &t220_properties, - THIS_MODULE, NULL, adapter_nr) || - 0 == dvb_usb_device_init(intf, &tt_s2_4600_properties, - THIS_MODULE, NULL, adapter_nr)) { - - /* clean up copied properties */ - kfree(s421); - kfree(p7500); - kfree(s660); - kfree(p1100); + if (!(dvb_usb_device_init(intf, &dw2102_properties, + THIS_MODULE, NULL, adapter_nr) && + dvb_usb_device_init(intf, &dw2104_properties, + THIS_MODULE, NULL, adapter_nr) && + dvb_usb_device_init(intf, &dw3101_properties, + THIS_MODULE, NULL, adapter_nr) && + dvb_usb_device_init(intf, &s6x0_properties, + THIS_MODULE, NULL, adapter_nr) && + dvb_usb_device_init(intf, &p1100_properties, + THIS_MODULE, NULL, adapter_nr) && + dvb_usb_device_init(intf, &s660_properties, + THIS_MODULE, NULL, adapter_nr) && + dvb_usb_device_init(intf, &p7500_properties, + THIS_MODULE, NULL, adapter_nr) && + dvb_usb_device_init(intf, &s421_properties, + THIS_MODULE, NULL, adapter_nr) && + dvb_usb_device_init(intf, &su3000_properties, + THIS_MODULE, NULL, adapter_nr) && + dvb_usb_device_init(intf, &t220_properties, + THIS_MODULE, NULL, adapter_nr) && + dvb_usb_device_init(intf, &tt_s2_4600_properties, + THIS_MODULE, NULL, adapter_nr))) { return 0; } - retval = -ENODEV; - kfree(s421); -err3: - kfree(p7500); -err2: - kfree(s660); -err1: - kfree(p1100); -err0: - return retval; + return -ENODEV; } static void dw2102_disconnect(struct usb_interface *intf) -- 2.23.0 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH] media: dw2102: Fix use after free 2019-08-22 10:41 ` [PATCH] media: dw2102: Fix use after free Anton Vasilyev @ 2019-08-29 16:56 ` Kees Cook 2020-04-14 10:10 ` Sean Young 0 siblings, 1 reply; 7+ messages in thread From: Kees Cook @ 2019-08-29 16:56 UTC (permalink / raw) To: Anton Vasilyev Cc: Mauro Carvalho Chehab, Greg Kroah-Hartman, Thomas Gleixner, Oliver Neukum, Ben Hutchings, ldv-project, linux-media, linux-kernel On Thu, Aug 22, 2019 at 01:41:47PM +0300, Anton Vasilyev wrote: > dvb_usb_device_init stores parts of properties at d->props > and d->desc and uses it on dvb_usb_device_exit. > Free of properties on module probe leads to use after free. > Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=204597 > > The patch makes properties static instead of allocated on heap to prevent > memleak and use after free. > Also fixes s421_properties.devices initialization to have 2 element > instead of 6 copied from p7500_properties. Can these be const? If not, is the probe() logic safe for these globals when there are multiple users? (I don't know this driver, but what happens if two of these devices get plugged in, for example.) -Kees > > > Signed-off-by: Anton Vasilyev <vasilyev@ispras.ru> > Fixes: 299c7007e936 ("media: dw2102: Fix memleak on sequence of probes") > --- > drivers/media/usb/dvb-usb/dw2102.c | 338 ++++++++++++++++++----------- > 1 file changed, 215 insertions(+), 123 deletions(-) > > diff --git a/drivers/media/usb/dvb-usb/dw2102.c b/drivers/media/usb/dvb-usb/dw2102.c > index b960abd00d48..7ea3aa0fee40 100644 > --- a/drivers/media/usb/dvb-usb/dw2102.c > +++ b/drivers/media/usb/dvb-usb/dw2102.c > @@ -2098,46 +2098,153 @@ static struct dvb_usb_device_properties s6x0_properties = { > } > }; > > -static const struct dvb_usb_device_description d1100 = { > - "Prof 1100 USB ", > - {&dw2102_table[PROF_1100], NULL}, > - {NULL}, > -}; > +static struct dvb_usb_device_properties p1100_properties = { > + .caps = DVB_USB_IS_AN_I2C_ADAPTER, > + .usb_ctrl = DEVICE_SPECIFIC, > + .size_of_priv = sizeof(struct dw2102_state), > + .firmware = P1100_FIRMWARE, > + .no_reconnect = 1, > > -static const struct dvb_usb_device_description d660 = { > - "TeVii S660 USB", > - {&dw2102_table[TEVII_S660], NULL}, > - {NULL}, > -}; > + .i2c_algo = &s6x0_i2c_algo, > + .rc.core = { > + .rc_interval = 150, > + .rc_codes = RC_MAP_TBS_NEC, > + .module_name = "dw2102", > + .allowed_protos = RC_PROTO_BIT_NEC, > + .rc_query = prof_rc_query, > + }, > > -static const struct dvb_usb_device_description d480_1 = { > - "TeVii S480.1 USB", > - {&dw2102_table[TEVII_S480_1], NULL}, > - {NULL}, > + .generic_bulk_ctrl_endpoint = 0x81, > + .num_adapters = 1, > + .download_firmware = dw2102_load_firmware, > + .read_mac_address = s6x0_read_mac_address, > + .adapter = { > + { > + .num_frontends = 1, > + .fe = {{ > + .frontend_attach = stv0288_frontend_attach, > + .stream = { > + .type = USB_BULK, > + .count = 8, > + .endpoint = 0x82, > + .u = { > + .bulk = { > + .buffersize = 4096, > + } > + } > + }, > + } }, > + } > + }, > + .num_device_descs = 1, > + .devices = { > + {"Prof 1100 USB ", > + {&dw2102_table[PROF_1100], NULL}, > + {NULL}, > + }, > + } > }; > > -static const struct dvb_usb_device_description d480_2 = { > - "TeVii S480.2 USB", > - {&dw2102_table[TEVII_S480_2], NULL}, > - {NULL}, > -}; > +static struct dvb_usb_device_properties s660_properties = { > + .caps = DVB_USB_IS_AN_I2C_ADAPTER, > + .usb_ctrl = DEVICE_SPECIFIC, > + .size_of_priv = sizeof(struct dw2102_state), > + .firmware = S660_FIRMWARE, > + .no_reconnect = 1, > > -static const struct dvb_usb_device_description d7500 = { > - "Prof 7500 USB DVB-S2", > - {&dw2102_table[PROF_7500], NULL}, > - {NULL}, > -}; > + .i2c_algo = &s6x0_i2c_algo, > + .rc.core = { > + .rc_interval = 150, > + .rc_codes = RC_MAP_TEVII_NEC, > + .module_name = "dw2102", > + .allowed_protos = RC_PROTO_BIT_NEC, > + .rc_query = dw2102_rc_query, > + }, > > -static const struct dvb_usb_device_description d421 = { > - "TeVii S421 PCI", > - {&dw2102_table[TEVII_S421], NULL}, > - {NULL}, > + .generic_bulk_ctrl_endpoint = 0x81, > + .num_adapters = 1, > + .download_firmware = dw2102_load_firmware, > + .read_mac_address = s6x0_read_mac_address, > + .adapter = { > + { > + .num_frontends = 1, > + .fe = {{ > + .frontend_attach = ds3000_frontend_attach, > + .stream = { > + .type = USB_BULK, > + .count = 8, > + .endpoint = 0x82, > + .u = { > + .bulk = { > + .buffersize = 4096, > + } > + } > + }, > + } }, > + } > + }, > + .num_device_descs = 3, > + .devices = { > + {"TeVii S660 USB", > + {&dw2102_table[TEVII_S660], NULL}, > + {NULL}, > + }, > + {"TeVii S480.1 USB", > + {&dw2102_table[TEVII_S480_1], NULL}, > + {NULL}, > + }, > + {"TeVii S480.2 USB", > + {&dw2102_table[TEVII_S480_2], NULL}, > + {NULL}, > + }, > + } > }; > > -static const struct dvb_usb_device_description d632 = { > - "TeVii S632 USB", > - {&dw2102_table[TEVII_S632], NULL}, > - {NULL}, > +static struct dvb_usb_device_properties p7500_properties = { > + .caps = DVB_USB_IS_AN_I2C_ADAPTER, > + .usb_ctrl = DEVICE_SPECIFIC, > + .size_of_priv = sizeof(struct dw2102_state), > + .firmware = P7500_FIRMWARE, > + .no_reconnect = 1, > + > + .i2c_algo = &s6x0_i2c_algo, > + .rc.core = { > + .rc_interval = 150, > + .rc_codes = RC_MAP_TBS_NEC, > + .module_name = "dw2102", > + .allowed_protos = RC_PROTO_BIT_NEC, > + .rc_query = prof_rc_query, > + }, > + > + .generic_bulk_ctrl_endpoint = 0x81, > + .num_adapters = 1, > + .download_firmware = dw2102_load_firmware, > + .read_mac_address = s6x0_read_mac_address, > + .adapter = { > + { > + .num_frontends = 1, > + .fe = {{ > + .frontend_attach = prof_7500_frontend_attach, > + .stream = { > + .type = USB_BULK, > + .count = 8, > + .endpoint = 0x82, > + .u = { > + .bulk = { > + .buffersize = 4096, > + } > + } > + }, > + } }, > + } > + }, > + .num_device_descs = 1, > + .devices = { > + {"Prof 7500 USB DVB-S2", > + {&dw2102_table[PROF_7500], NULL}, > + {NULL}, > + }, > + } > }; > > static struct dvb_usb_device_properties su3000_properties = { > @@ -2209,6 +2316,59 @@ static struct dvb_usb_device_properties su3000_properties = { > } > }; > > +static struct dvb_usb_device_properties s421_properties = { > + .caps = DVB_USB_IS_AN_I2C_ADAPTER, > + .usb_ctrl = DEVICE_SPECIFIC, > + .size_of_priv = sizeof(struct dw2102_state), > + .power_ctrl = su3000_power_ctrl, > + .num_adapters = 1, > + .identify_state = su3000_identify_state, > + .i2c_algo = &su3000_i2c_algo, > + > + .rc.core = { > + .rc_interval = 150, > + .rc_codes = RC_MAP_SU3000, > + .module_name = "dw2102", > + .allowed_protos = RC_PROTO_BIT_RC5, > + .rc_query = su3000_rc_query, > + }, > + > + .read_mac_address = su3000_read_mac_address, > + > + .generic_bulk_ctrl_endpoint = 0x01, > + > + .adapter = { > + { > + .num_frontends = 1, > + .fe = {{ > + .streaming_ctrl = su3000_streaming_ctrl, > + .frontend_attach = m88rs2000_frontend_attach, > + .stream = { > + .type = USB_BULK, > + .count = 8, > + .endpoint = 0x82, > + .u = { > + .bulk = { > + .buffersize = 4096, > + } > + } > + } > + } }, > + } > + }, > + .num_device_descs = 2, > + .devices = { > + { "TeVii S421 PCI", > + { &dw2102_table[TEVII_S421], NULL }, > + { NULL }, > + }, > + { "TeVii S632 USB", > + { &dw2102_table[TEVII_S632], NULL }, > + { NULL }, > + }, > + } > +}; > + > static struct dvb_usb_device_properties t220_properties = { > .caps = DVB_USB_IS_AN_I2C_ADAPTER, > .usb_ctrl = DEVICE_SPECIFIC, > @@ -2326,101 +2486,33 @@ static struct dvb_usb_device_properties tt_s2_4600_properties = { > static int dw2102_probe(struct usb_interface *intf, > const struct usb_device_id *id) > { > - int retval = -ENOMEM; > - struct dvb_usb_device_properties *p1100; > - struct dvb_usb_device_properties *s660; > - struct dvb_usb_device_properties *p7500; > - struct dvb_usb_device_properties *s421; > - > - p1100 = kmemdup(&s6x0_properties, > - sizeof(struct dvb_usb_device_properties), GFP_KERNEL); > - if (!p1100) > - goto err0; > - > - /* copy default structure */ > - /* fill only different fields */ > - p1100->firmware = P1100_FIRMWARE; > - p1100->devices[0] = d1100; > - p1100->rc.core.rc_query = prof_rc_query; > - p1100->rc.core.rc_codes = RC_MAP_TBS_NEC; > - p1100->adapter->fe[0].frontend_attach = stv0288_frontend_attach; > - > - s660 = kmemdup(&s6x0_properties, > - sizeof(struct dvb_usb_device_properties), GFP_KERNEL); > - if (!s660) > - goto err1; > - > - s660->firmware = S660_FIRMWARE; > - s660->num_device_descs = 3; > - s660->devices[0] = d660; > - s660->devices[1] = d480_1; > - s660->devices[2] = d480_2; > - s660->adapter->fe[0].frontend_attach = ds3000_frontend_attach; > - > - p7500 = kmemdup(&s6x0_properties, > - sizeof(struct dvb_usb_device_properties), GFP_KERNEL); > - if (!p7500) > - goto err2; > - > - p7500->firmware = P7500_FIRMWARE; > - p7500->devices[0] = d7500; > - p7500->rc.core.rc_query = prof_rc_query; > - p7500->rc.core.rc_codes = RC_MAP_TBS_NEC; > - p7500->adapter->fe[0].frontend_attach = prof_7500_frontend_attach; > - > - > - s421 = kmemdup(&su3000_properties, > - sizeof(struct dvb_usb_device_properties), GFP_KERNEL); > - if (!s421) > - goto err3; > - > - s421->num_device_descs = 2; > - s421->devices[0] = d421; > - s421->devices[1] = d632; > - s421->adapter->fe[0].frontend_attach = m88rs2000_frontend_attach; > - > - if (0 == dvb_usb_device_init(intf, &dw2102_properties, > - THIS_MODULE, NULL, adapter_nr) || > - 0 == dvb_usb_device_init(intf, &dw2104_properties, > - THIS_MODULE, NULL, adapter_nr) || > - 0 == dvb_usb_device_init(intf, &dw3101_properties, > - THIS_MODULE, NULL, adapter_nr) || > - 0 == dvb_usb_device_init(intf, &s6x0_properties, > - THIS_MODULE, NULL, adapter_nr) || > - 0 == dvb_usb_device_init(intf, p1100, > - THIS_MODULE, NULL, adapter_nr) || > - 0 == dvb_usb_device_init(intf, s660, > - THIS_MODULE, NULL, adapter_nr) || > - 0 == dvb_usb_device_init(intf, p7500, > - THIS_MODULE, NULL, adapter_nr) || > - 0 == dvb_usb_device_init(intf, s421, > - THIS_MODULE, NULL, adapter_nr) || > - 0 == dvb_usb_device_init(intf, &su3000_properties, > - THIS_MODULE, NULL, adapter_nr) || > - 0 == dvb_usb_device_init(intf, &t220_properties, > - THIS_MODULE, NULL, adapter_nr) || > - 0 == dvb_usb_device_init(intf, &tt_s2_4600_properties, > - THIS_MODULE, NULL, adapter_nr)) { > - > - /* clean up copied properties */ > - kfree(s421); > - kfree(p7500); > - kfree(s660); > - kfree(p1100); > + if (!(dvb_usb_device_init(intf, &dw2102_properties, > + THIS_MODULE, NULL, adapter_nr) && > + dvb_usb_device_init(intf, &dw2104_properties, > + THIS_MODULE, NULL, adapter_nr) && > + dvb_usb_device_init(intf, &dw3101_properties, > + THIS_MODULE, NULL, adapter_nr) && > + dvb_usb_device_init(intf, &s6x0_properties, > + THIS_MODULE, NULL, adapter_nr) && > + dvb_usb_device_init(intf, &p1100_properties, > + THIS_MODULE, NULL, adapter_nr) && > + dvb_usb_device_init(intf, &s660_properties, > + THIS_MODULE, NULL, adapter_nr) && > + dvb_usb_device_init(intf, &p7500_properties, > + THIS_MODULE, NULL, adapter_nr) && > + dvb_usb_device_init(intf, &s421_properties, > + THIS_MODULE, NULL, adapter_nr) && > + dvb_usb_device_init(intf, &su3000_properties, > + THIS_MODULE, NULL, adapter_nr) && > + dvb_usb_device_init(intf, &t220_properties, > + THIS_MODULE, NULL, adapter_nr) && > + dvb_usb_device_init(intf, &tt_s2_4600_properties, > + THIS_MODULE, NULL, adapter_nr))) { > > return 0; > } > > - retval = -ENODEV; > - kfree(s421); > -err3: > - kfree(p7500); > -err2: > - kfree(s660); > -err1: > - kfree(p1100); > -err0: > - return retval; > + return -ENODEV; > } > > static void dw2102_disconnect(struct usb_interface *intf) > -- > 2.23.0 > -- Kees Cook ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] media: dw2102: Fix use after free 2019-08-29 16:56 ` Kees Cook @ 2020-04-14 10:10 ` Sean Young 0 siblings, 0 replies; 7+ messages in thread From: Sean Young @ 2020-04-14 10:10 UTC (permalink / raw) To: Kees Cook Cc: Anton Vasilyev, Mauro Carvalho Chehab, Greg Kroah-Hartman, Thomas Gleixner, Oliver Neukum, Ben Hutchings, ldv-project, linux-media, linux-kernel On Thu, Aug 29, 2019 at 09:56:04AM -0700, Kees Cook wrote: > On Thu, Aug 22, 2019 at 01:41:47PM +0300, Anton Vasilyev wrote: > > dvb_usb_device_init stores parts of properties at d->props > > and d->desc and uses it on dvb_usb_device_exit. > > Free of properties on module probe leads to use after free. > > Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=204597 > > > > The patch makes properties static instead of allocated on heap to prevent > > memleak and use after free. > > Also fixes s421_properties.devices initialization to have 2 element > > instead of 6 copied from p7500_properties. > > Can these be const? If not, is the probe() logic safe for these globals > when there are multiple users? (I don't know this driver, but what > happens if two of these devices get plugged in, for example.) Sorry for the super-late review. So this is exactly right. dvb_usb_device_init() copies the properties, so that is fine (I've just written a patch to make the props argument const*). However, this code changes the properties based on the usb id. See here: https://github.com/torvalds/linux/blob/master/drivers/media/usb/dvb-usb/dw2102.c#L1900-L1916 So, these different variants need to be split into two or something like that. Sean > > -Kees > > > > > > > Signed-off-by: Anton Vasilyev <vasilyev@ispras.ru> > > Fixes: 299c7007e936 ("media: dw2102: Fix memleak on sequence of probes") > > --- > > drivers/media/usb/dvb-usb/dw2102.c | 338 ++++++++++++++++++----------- > > 1 file changed, 215 insertions(+), 123 deletions(-) > > > > diff --git a/drivers/media/usb/dvb-usb/dw2102.c b/drivers/media/usb/dvb-usb/dw2102.c > > index b960abd00d48..7ea3aa0fee40 100644 > > --- a/drivers/media/usb/dvb-usb/dw2102.c > > +++ b/drivers/media/usb/dvb-usb/dw2102.c > > @@ -2098,46 +2098,153 @@ static struct dvb_usb_device_properties s6x0_properties = { > > } > > }; > > > > -static const struct dvb_usb_device_description d1100 = { > > - "Prof 1100 USB ", > > - {&dw2102_table[PROF_1100], NULL}, > > - {NULL}, > > -}; > > +static struct dvb_usb_device_properties p1100_properties = { > > + .caps = DVB_USB_IS_AN_I2C_ADAPTER, > > + .usb_ctrl = DEVICE_SPECIFIC, > > + .size_of_priv = sizeof(struct dw2102_state), > > + .firmware = P1100_FIRMWARE, > > + .no_reconnect = 1, > > > > -static const struct dvb_usb_device_description d660 = { > > - "TeVii S660 USB", > > - {&dw2102_table[TEVII_S660], NULL}, > > - {NULL}, > > -}; > > + .i2c_algo = &s6x0_i2c_algo, > > + .rc.core = { > > + .rc_interval = 150, > > + .rc_codes = RC_MAP_TBS_NEC, > > + .module_name = "dw2102", > > + .allowed_protos = RC_PROTO_BIT_NEC, > > + .rc_query = prof_rc_query, > > + }, > > > > -static const struct dvb_usb_device_description d480_1 = { > > - "TeVii S480.1 USB", > > - {&dw2102_table[TEVII_S480_1], NULL}, > > - {NULL}, > > + .generic_bulk_ctrl_endpoint = 0x81, > > + .num_adapters = 1, > > + .download_firmware = dw2102_load_firmware, > > + .read_mac_address = s6x0_read_mac_address, > > + .adapter = { > > + { > > + .num_frontends = 1, > > + .fe = {{ > > + .frontend_attach = stv0288_frontend_attach, > > + .stream = { > > + .type = USB_BULK, > > + .count = 8, > > + .endpoint = 0x82, > > + .u = { > > + .bulk = { > > + .buffersize = 4096, > > + } > > + } > > + }, > > + } }, > > + } > > + }, > > + .num_device_descs = 1, > > + .devices = { > > + {"Prof 1100 USB ", > > + {&dw2102_table[PROF_1100], NULL}, > > + {NULL}, > > + }, > > + } > > }; > > > > -static const struct dvb_usb_device_description d480_2 = { > > - "TeVii S480.2 USB", > > - {&dw2102_table[TEVII_S480_2], NULL}, > > - {NULL}, > > -}; > > +static struct dvb_usb_device_properties s660_properties = { > > + .caps = DVB_USB_IS_AN_I2C_ADAPTER, > > + .usb_ctrl = DEVICE_SPECIFIC, > > + .size_of_priv = sizeof(struct dw2102_state), > > + .firmware = S660_FIRMWARE, > > + .no_reconnect = 1, > > > > -static const struct dvb_usb_device_description d7500 = { > > - "Prof 7500 USB DVB-S2", > > - {&dw2102_table[PROF_7500], NULL}, > > - {NULL}, > > -}; > > + .i2c_algo = &s6x0_i2c_algo, > > + .rc.core = { > > + .rc_interval = 150, > > + .rc_codes = RC_MAP_TEVII_NEC, > > + .module_name = "dw2102", > > + .allowed_protos = RC_PROTO_BIT_NEC, > > + .rc_query = dw2102_rc_query, > > + }, > > > > -static const struct dvb_usb_device_description d421 = { > > - "TeVii S421 PCI", > > - {&dw2102_table[TEVII_S421], NULL}, > > - {NULL}, > > + .generic_bulk_ctrl_endpoint = 0x81, > > + .num_adapters = 1, > > + .download_firmware = dw2102_load_firmware, > > + .read_mac_address = s6x0_read_mac_address, > > + .adapter = { > > + { > > + .num_frontends = 1, > > + .fe = {{ > > + .frontend_attach = ds3000_frontend_attach, > > + .stream = { > > + .type = USB_BULK, > > + .count = 8, > > + .endpoint = 0x82, > > + .u = { > > + .bulk = { > > + .buffersize = 4096, > > + } > > + } > > + }, > > + } }, > > + } > > + }, > > + .num_device_descs = 3, > > + .devices = { > > + {"TeVii S660 USB", > > + {&dw2102_table[TEVII_S660], NULL}, > > + {NULL}, > > + }, > > + {"TeVii S480.1 USB", > > + {&dw2102_table[TEVII_S480_1], NULL}, > > + {NULL}, > > + }, > > + {"TeVii S480.2 USB", > > + {&dw2102_table[TEVII_S480_2], NULL}, > > + {NULL}, > > + }, > > + } > > }; > > > > -static const struct dvb_usb_device_description d632 = { > > - "TeVii S632 USB", > > - {&dw2102_table[TEVII_S632], NULL}, > > - {NULL}, > > +static struct dvb_usb_device_properties p7500_properties = { > > + .caps = DVB_USB_IS_AN_I2C_ADAPTER, > > + .usb_ctrl = DEVICE_SPECIFIC, > > + .size_of_priv = sizeof(struct dw2102_state), > > + .firmware = P7500_FIRMWARE, > > + .no_reconnect = 1, > > + > > + .i2c_algo = &s6x0_i2c_algo, > > + .rc.core = { > > + .rc_interval = 150, > > + .rc_codes = RC_MAP_TBS_NEC, > > + .module_name = "dw2102", > > + .allowed_protos = RC_PROTO_BIT_NEC, > > + .rc_query = prof_rc_query, > > + }, > > + > > + .generic_bulk_ctrl_endpoint = 0x81, > > + .num_adapters = 1, > > + .download_firmware = dw2102_load_firmware, > > + .read_mac_address = s6x0_read_mac_address, > > + .adapter = { > > + { > > + .num_frontends = 1, > > + .fe = {{ > > + .frontend_attach = prof_7500_frontend_attach, > > + .stream = { > > + .type = USB_BULK, > > + .count = 8, > > + .endpoint = 0x82, > > + .u = { > > + .bulk = { > > + .buffersize = 4096, > > + } > > + } > > + }, > > + } }, > > + } > > + }, > > + .num_device_descs = 1, > > + .devices = { > > + {"Prof 7500 USB DVB-S2", > > + {&dw2102_table[PROF_7500], NULL}, > > + {NULL}, > > + }, > > + } > > }; > > > > static struct dvb_usb_device_properties su3000_properties = { > > @@ -2209,6 +2316,59 @@ static struct dvb_usb_device_properties su3000_properties = { > > } > > }; > > > > +static struct dvb_usb_device_properties s421_properties = { > > + .caps = DVB_USB_IS_AN_I2C_ADAPTER, > > + .usb_ctrl = DEVICE_SPECIFIC, > > + .size_of_priv = sizeof(struct dw2102_state), > > + .power_ctrl = su3000_power_ctrl, > > + .num_adapters = 1, > > + .identify_state = su3000_identify_state, > > + .i2c_algo = &su3000_i2c_algo, > > + > > + .rc.core = { > > + .rc_interval = 150, > > + .rc_codes = RC_MAP_SU3000, > > + .module_name = "dw2102", > > + .allowed_protos = RC_PROTO_BIT_RC5, > > + .rc_query = su3000_rc_query, > > + }, > > + > > + .read_mac_address = su3000_read_mac_address, > > + > > + .generic_bulk_ctrl_endpoint = 0x01, > > + > > + .adapter = { > > + { > > + .num_frontends = 1, > > + .fe = {{ > > + .streaming_ctrl = su3000_streaming_ctrl, > > + .frontend_attach = m88rs2000_frontend_attach, > > + .stream = { > > + .type = USB_BULK, > > + .count = 8, > > + .endpoint = 0x82, > > + .u = { > > + .bulk = { > > + .buffersize = 4096, > > + } > > + } > > + } > > + } }, > > + } > > + }, > > + .num_device_descs = 2, > > + .devices = { > > + { "TeVii S421 PCI", > > + { &dw2102_table[TEVII_S421], NULL }, > > + { NULL }, > > + }, > > + { "TeVii S632 USB", > > + { &dw2102_table[TEVII_S632], NULL }, > > + { NULL }, > > + }, > > + } > > +}; > > + > > static struct dvb_usb_device_properties t220_properties = { > > .caps = DVB_USB_IS_AN_I2C_ADAPTER, > > .usb_ctrl = DEVICE_SPECIFIC, > > @@ -2326,101 +2486,33 @@ static struct dvb_usb_device_properties tt_s2_4600_properties = { > > static int dw2102_probe(struct usb_interface *intf, > > const struct usb_device_id *id) > > { > > - int retval = -ENOMEM; > > - struct dvb_usb_device_properties *p1100; > > - struct dvb_usb_device_properties *s660; > > - struct dvb_usb_device_properties *p7500; > > - struct dvb_usb_device_properties *s421; > > - > > - p1100 = kmemdup(&s6x0_properties, > > - sizeof(struct dvb_usb_device_properties), GFP_KERNEL); > > - if (!p1100) > > - goto err0; > > - > > - /* copy default structure */ > > - /* fill only different fields */ > > - p1100->firmware = P1100_FIRMWARE; > > - p1100->devices[0] = d1100; > > - p1100->rc.core.rc_query = prof_rc_query; > > - p1100->rc.core.rc_codes = RC_MAP_TBS_NEC; > > - p1100->adapter->fe[0].frontend_attach = stv0288_frontend_attach; > > - > > - s660 = kmemdup(&s6x0_properties, > > - sizeof(struct dvb_usb_device_properties), GFP_KERNEL); > > - if (!s660) > > - goto err1; > > - > > - s660->firmware = S660_FIRMWARE; > > - s660->num_device_descs = 3; > > - s660->devices[0] = d660; > > - s660->devices[1] = d480_1; > > - s660->devices[2] = d480_2; > > - s660->adapter->fe[0].frontend_attach = ds3000_frontend_attach; > > - > > - p7500 = kmemdup(&s6x0_properties, > > - sizeof(struct dvb_usb_device_properties), GFP_KERNEL); > > - if (!p7500) > > - goto err2; > > - > > - p7500->firmware = P7500_FIRMWARE; > > - p7500->devices[0] = d7500; > > - p7500->rc.core.rc_query = prof_rc_query; > > - p7500->rc.core.rc_codes = RC_MAP_TBS_NEC; > > - p7500->adapter->fe[0].frontend_attach = prof_7500_frontend_attach; > > - > > - > > - s421 = kmemdup(&su3000_properties, > > - sizeof(struct dvb_usb_device_properties), GFP_KERNEL); > > - if (!s421) > > - goto err3; > > - > > - s421->num_device_descs = 2; > > - s421->devices[0] = d421; > > - s421->devices[1] = d632; > > - s421->adapter->fe[0].frontend_attach = m88rs2000_frontend_attach; > > - > > - if (0 == dvb_usb_device_init(intf, &dw2102_properties, > > - THIS_MODULE, NULL, adapter_nr) || > > - 0 == dvb_usb_device_init(intf, &dw2104_properties, > > - THIS_MODULE, NULL, adapter_nr) || > > - 0 == dvb_usb_device_init(intf, &dw3101_properties, > > - THIS_MODULE, NULL, adapter_nr) || > > - 0 == dvb_usb_device_init(intf, &s6x0_properties, > > - THIS_MODULE, NULL, adapter_nr) || > > - 0 == dvb_usb_device_init(intf, p1100, > > - THIS_MODULE, NULL, adapter_nr) || > > - 0 == dvb_usb_device_init(intf, s660, > > - THIS_MODULE, NULL, adapter_nr) || > > - 0 == dvb_usb_device_init(intf, p7500, > > - THIS_MODULE, NULL, adapter_nr) || > > - 0 == dvb_usb_device_init(intf, s421, > > - THIS_MODULE, NULL, adapter_nr) || > > - 0 == dvb_usb_device_init(intf, &su3000_properties, > > - THIS_MODULE, NULL, adapter_nr) || > > - 0 == dvb_usb_device_init(intf, &t220_properties, > > - THIS_MODULE, NULL, adapter_nr) || > > - 0 == dvb_usb_device_init(intf, &tt_s2_4600_properties, > > - THIS_MODULE, NULL, adapter_nr)) { > > - > > - /* clean up copied properties */ > > - kfree(s421); > > - kfree(p7500); > > - kfree(s660); > > - kfree(p1100); > > + if (!(dvb_usb_device_init(intf, &dw2102_properties, > > + THIS_MODULE, NULL, adapter_nr) && > > + dvb_usb_device_init(intf, &dw2104_properties, > > + THIS_MODULE, NULL, adapter_nr) && > > + dvb_usb_device_init(intf, &dw3101_properties, > > + THIS_MODULE, NULL, adapter_nr) && > > + dvb_usb_device_init(intf, &s6x0_properties, > > + THIS_MODULE, NULL, adapter_nr) && > > + dvb_usb_device_init(intf, &p1100_properties, > > + THIS_MODULE, NULL, adapter_nr) && > > + dvb_usb_device_init(intf, &s660_properties, > > + THIS_MODULE, NULL, adapter_nr) && > > + dvb_usb_device_init(intf, &p7500_properties, > > + THIS_MODULE, NULL, adapter_nr) && > > + dvb_usb_device_init(intf, &s421_properties, > > + THIS_MODULE, NULL, adapter_nr) && > > + dvb_usb_device_init(intf, &su3000_properties, > > + THIS_MODULE, NULL, adapter_nr) && > > + dvb_usb_device_init(intf, &t220_properties, > > + THIS_MODULE, NULL, adapter_nr) && > > + dvb_usb_device_init(intf, &tt_s2_4600_properties, > > + THIS_MODULE, NULL, adapter_nr))) { > > > > return 0; > > } > > > > - retval = -ENODEV; > > - kfree(s421); > > -err3: > > - kfree(p7500); > > -err2: > > - kfree(s660); > > -err1: > > - kfree(p1100); > > -err0: > > - return retval; > > + return -ENODEV; > > } > > > > static void dw2102_disconnect(struct usb_interface *intf) > > -- > > 2.23.0 > > > > -- > Kees Cook ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2020-04-14 10:10 UTC | newest] Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-04-30 13:07 [PATCH] dvb: usb: fix use after free in dvb_usb_device_exit Oliver Neukum 2019-08-20 18:55 ` Ben Hutchings 2019-08-21 8:18 ` Oliver Neukum 2019-08-21 12:47 ` Ben Hutchings 2019-08-22 10:41 ` [PATCH] media: dw2102: Fix use after free Anton Vasilyev 2019-08-29 16:56 ` Kees Cook 2020-04-14 10:10 ` Sean Young
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).