* [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).