* [PATCH] media: dw2102: Fix use after free
[not found] <fcae86f2137822d7658d2bbd4bd8dd35bfb319b0.camel@decadent.org.uk>
@ 2019-08-22 10:41 ` Anton Vasilyev
2019-08-29 16:56 ` Kees Cook
0 siblings, 1 reply; 3+ 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] 3+ 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; 3+ 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] 3+ 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; 3+ 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] 3+ messages in thread
end of thread, other threads:[~2020-04-14 10:10 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
[not found] <fcae86f2137822d7658d2bbd4bd8dd35bfb319b0.camel@decadent.org.uk>
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).