diff options
author | Alexey Khoroshilov <khoroshilov@ispras.ru> | 2013-10-02 12:00:26 -0400 |
---|---|---|
committer | Mauro Carvalho Chehab <m.chehab@samsung.com> | 2013-10-03 06:33:59 -0400 |
commit | cf732b5fa30f5dc421b4c1911bc2446b9bcbc025 (patch) | |
tree | 0145c0655be36a809e84d172fa6dd58d1cac1b78 | |
parent | b4559ace2ca8c88666584279f582b998c6591fb0 (diff) |
[media] dvb-usb: fix error handling in ttusb_dec_probe()
There is an asymmetry in ttusb_dec_init_usb()-ttusb_init_rc()
and ttusb_dec_exit_usb()-ttusb_dec_exit_rc() in terms of resources
allocated-deallocated. irq_urb and irq_buffer are allocated in
ttusb_dec_init_usb(), while they are deallocated in ttusb_dec_exit_rc().
As a result there is a leak of them in ttusb_dec_probe().
The patch fixes the asymmetry and a leak on a failure path in ttusb_dec_init_usb().
By the way, it
- removes usage of -1 as a custom error code,
- replaces GFP_ATOMIC by GFP_KERNEL in usb_alloc_coherent() in ttusb_dec_init_usb()
as soon as all other memory allocation done with GFP_KERNEL;
- refactors ttusb_dec_boot_dsp() in an equivalent way except for returning 0
instead of 1 if ttusb_dec_boot_dsp() succeed in (!mode) branch.
Found by Linux Driver Verification project (linuxtesting.org).
Signed-off-by: Alexey Khoroshilov <khoroshilov@ispras.ru>
Signed-off-by: Michael Krufky <mkrufky@linuxtv.org>
Signed-off-by: Mauro Carvalho Chehab <m.chehab@samsung.com>
-rw-r--r-- | drivers/media/usb/ttusb-dec/ttusb_dec.c | 152 |
1 files changed, 82 insertions, 70 deletions
diff --git a/drivers/media/usb/ttusb-dec/ttusb_dec.c b/drivers/media/usb/ttusb-dec/ttusb_dec.c index e52c3b97f304..29724af9b9ab 100644 --- a/drivers/media/usb/ttusb-dec/ttusb_dec.c +++ b/drivers/media/usb/ttusb-dec/ttusb_dec.c | |||
@@ -366,7 +366,7 @@ static int ttusb_dec_get_stb_state (struct ttusb_dec *dec, unsigned int *mode, | |||
366 | } | 366 | } |
367 | return 0; | 367 | return 0; |
368 | } else { | 368 | } else { |
369 | return -1; | 369 | return -ENOENT; |
370 | } | 370 | } |
371 | } | 371 | } |
372 | 372 | ||
@@ -1241,6 +1241,8 @@ static void ttusb_dec_init_v_pes(struct ttusb_dec *dec) | |||
1241 | 1241 | ||
1242 | static int ttusb_dec_init_usb(struct ttusb_dec *dec) | 1242 | static int ttusb_dec_init_usb(struct ttusb_dec *dec) |
1243 | { | 1243 | { |
1244 | int result; | ||
1245 | |||
1244 | dprintk("%s\n", __func__); | 1246 | dprintk("%s\n", __func__); |
1245 | 1247 | ||
1246 | mutex_init(&dec->usb_mutex); | 1248 | mutex_init(&dec->usb_mutex); |
@@ -1258,7 +1260,7 @@ static int ttusb_dec_init_usb(struct ttusb_dec *dec) | |||
1258 | return -ENOMEM; | 1260 | return -ENOMEM; |
1259 | } | 1261 | } |
1260 | dec->irq_buffer = usb_alloc_coherent(dec->udev,IRQ_PACKET_SIZE, | 1262 | dec->irq_buffer = usb_alloc_coherent(dec->udev,IRQ_PACKET_SIZE, |
1261 | GFP_ATOMIC, &dec->irq_dma_handle); | 1263 | GFP_KERNEL, &dec->irq_dma_handle); |
1262 | if(!dec->irq_buffer) { | 1264 | if(!dec->irq_buffer) { |
1263 | usb_free_urb(dec->irq_urb); | 1265 | usb_free_urb(dec->irq_urb); |
1264 | return -ENOMEM; | 1266 | return -ENOMEM; |
@@ -1270,7 +1272,13 @@ static int ttusb_dec_init_usb(struct ttusb_dec *dec) | |||
1270 | dec->irq_urb->transfer_flags |= URB_NO_TRANSFER_DMA_MAP; | 1272 | dec->irq_urb->transfer_flags |= URB_NO_TRANSFER_DMA_MAP; |
1271 | } | 1273 | } |
1272 | 1274 | ||
1273 | return ttusb_dec_alloc_iso_urbs(dec); | 1275 | result = ttusb_dec_alloc_iso_urbs(dec); |
1276 | if (result) { | ||
1277 | usb_free_urb(dec->irq_urb); | ||
1278 | usb_free_coherent(dec->udev, IRQ_PACKET_SIZE, | ||
1279 | dec->irq_buffer, dec->irq_dma_handle); | ||
1280 | } | ||
1281 | return result; | ||
1274 | } | 1282 | } |
1275 | 1283 | ||
1276 | static int ttusb_dec_boot_dsp(struct ttusb_dec *dec) | 1284 | static int ttusb_dec_boot_dsp(struct ttusb_dec *dec) |
@@ -1293,10 +1301,11 @@ static int ttusb_dec_boot_dsp(struct ttusb_dec *dec) | |||
1293 | 1301 | ||
1294 | dprintk("%s\n", __func__); | 1302 | dprintk("%s\n", __func__); |
1295 | 1303 | ||
1296 | if (request_firmware(&fw_entry, dec->firmware_name, &dec->udev->dev)) { | 1304 | result = request_firmware(&fw_entry, dec->firmware_name, &dec->udev->dev); |
1305 | if (result) { | ||
1297 | printk(KERN_ERR "%s: Firmware (%s) unavailable.\n", | 1306 | printk(KERN_ERR "%s: Firmware (%s) unavailable.\n", |
1298 | __func__, dec->firmware_name); | 1307 | __func__, dec->firmware_name); |
1299 | return 1; | 1308 | return result; |
1300 | } | 1309 | } |
1301 | 1310 | ||
1302 | firmware = fw_entry->data; | 1311 | firmware = fw_entry->data; |
@@ -1306,7 +1315,7 @@ static int ttusb_dec_boot_dsp(struct ttusb_dec *dec) | |||
1306 | printk("%s: firmware size too small for DSP code (%zu < 60).\n", | 1315 | printk("%s: firmware size too small for DSP code (%zu < 60).\n", |
1307 | __func__, firmware_size); | 1316 | __func__, firmware_size); |
1308 | release_firmware(fw_entry); | 1317 | release_firmware(fw_entry); |
1309 | return -1; | 1318 | return -ENOENT; |
1310 | } | 1319 | } |
1311 | 1320 | ||
1312 | /* a 32 bit checksum over the first 56 bytes of the DSP Code is stored | 1321 | /* a 32 bit checksum over the first 56 bytes of the DSP Code is stored |
@@ -1320,7 +1329,7 @@ static int ttusb_dec_boot_dsp(struct ttusb_dec *dec) | |||
1320 | "0x%08x != 0x%08x in file), file invalid.\n", | 1329 | "0x%08x != 0x%08x in file), file invalid.\n", |
1321 | __func__, crc32_csum, crc32_check); | 1330 | __func__, crc32_csum, crc32_check); |
1322 | release_firmware(fw_entry); | 1331 | release_firmware(fw_entry); |
1323 | return -1; | 1332 | return -ENOENT; |
1324 | } | 1333 | } |
1325 | memcpy(idstring, &firmware[36], 20); | 1334 | memcpy(idstring, &firmware[36], 20); |
1326 | idstring[20] = '\0'; | 1335 | idstring[20] = '\0'; |
@@ -1389,55 +1398,48 @@ static int ttusb_dec_init_stb(struct ttusb_dec *dec) | |||
1389 | dprintk("%s\n", __func__); | 1398 | dprintk("%s\n", __func__); |
1390 | 1399 | ||
1391 | result = ttusb_dec_get_stb_state(dec, &mode, &model, &version); | 1400 | result = ttusb_dec_get_stb_state(dec, &mode, &model, &version); |
1401 | if (result) | ||
1402 | return result; | ||
1392 | 1403 | ||
1393 | if (!result) { | 1404 | if (!mode) { |
1394 | if (!mode) { | 1405 | if (version == 0xABCDEFAB) |
1395 | if (version == 0xABCDEFAB) | 1406 | printk(KERN_INFO "ttusb_dec: no version " |
1396 | printk(KERN_INFO "ttusb_dec: no version " | 1407 | "info in Firmware\n"); |
1397 | "info in Firmware\n"); | 1408 | else |
1398 | else | 1409 | printk(KERN_INFO "ttusb_dec: Firmware " |
1399 | printk(KERN_INFO "ttusb_dec: Firmware " | 1410 | "%x.%02x%c%c\n", |
1400 | "%x.%02x%c%c\n", | 1411 | version >> 24, (version >> 16) & 0xff, |
1401 | version >> 24, (version >> 16) & 0xff, | 1412 | (version >> 8) & 0xff, version & 0xff); |
1402 | (version >> 8) & 0xff, version & 0xff); | ||
1403 | |||
1404 | result = ttusb_dec_boot_dsp(dec); | ||
1405 | if (result) | ||
1406 | return result; | ||
1407 | else | ||
1408 | return 1; | ||
1409 | } else { | ||
1410 | /* We can't trust the USB IDs that some firmwares | ||
1411 | give the box */ | ||
1412 | switch (model) { | ||
1413 | case 0x00070001: | ||
1414 | case 0x00070008: | ||
1415 | case 0x0007000c: | ||
1416 | ttusb_dec_set_model(dec, TTUSB_DEC3000S); | ||
1417 | break; | ||
1418 | case 0x00070009: | ||
1419 | case 0x00070013: | ||
1420 | ttusb_dec_set_model(dec, TTUSB_DEC2000T); | ||
1421 | break; | ||
1422 | case 0x00070011: | ||
1423 | ttusb_dec_set_model(dec, TTUSB_DEC2540T); | ||
1424 | break; | ||
1425 | default: | ||
1426 | printk(KERN_ERR "%s: unknown model returned " | ||
1427 | "by firmware (%08x) - please report\n", | ||
1428 | __func__, model); | ||
1429 | return -1; | ||
1430 | break; | ||
1431 | } | ||
1432 | 1413 | ||
1414 | result = ttusb_dec_boot_dsp(dec); | ||
1415 | if (result) | ||
1416 | return result; | ||
1417 | } else { | ||
1418 | /* We can't trust the USB IDs that some firmwares | ||
1419 | give the box */ | ||
1420 | switch (model) { | ||
1421 | case 0x00070001: | ||
1422 | case 0x00070008: | ||
1423 | case 0x0007000c: | ||
1424 | ttusb_dec_set_model(dec, TTUSB_DEC3000S); | ||
1425 | break; | ||
1426 | case 0x00070009: | ||
1427 | case 0x00070013: | ||
1428 | ttusb_dec_set_model(dec, TTUSB_DEC2000T); | ||
1429 | break; | ||
1430 | case 0x00070011: | ||
1431 | ttusb_dec_set_model(dec, TTUSB_DEC2540T); | ||
1432 | break; | ||
1433 | default: | ||
1434 | printk(KERN_ERR "%s: unknown model returned " | ||
1435 | "by firmware (%08x) - please report\n", | ||
1436 | __func__, model); | ||
1437 | return -ENOENT; | ||
1438 | } | ||
1433 | if (version >= 0x01770000) | 1439 | if (version >= 0x01770000) |
1434 | dec->can_playback = 1; | 1440 | dec->can_playback = 1; |
1435 | |||
1436 | return 0; | ||
1437 | } | ||
1438 | } | 1441 | } |
1439 | else | 1442 | return 0; |
1440 | return result; | ||
1441 | } | 1443 | } |
1442 | 1444 | ||
1443 | static int ttusb_dec_init_dvb(struct ttusb_dec *dec) | 1445 | static int ttusb_dec_init_dvb(struct ttusb_dec *dec) |
@@ -1539,19 +1541,7 @@ static void ttusb_dec_exit_dvb(struct ttusb_dec *dec) | |||
1539 | 1541 | ||
1540 | static void ttusb_dec_exit_rc(struct ttusb_dec *dec) | 1542 | static void ttusb_dec_exit_rc(struct ttusb_dec *dec) |
1541 | { | 1543 | { |
1542 | |||
1543 | dprintk("%s\n", __func__); | 1544 | dprintk("%s\n", __func__); |
1544 | /* we have to check whether the irq URB is already submitted. | ||
1545 | * As the irq is submitted after the interface is changed, | ||
1546 | * this is the best method i figured out. | ||
1547 | * Any others?*/ | ||
1548 | if (dec->interface == TTUSB_DEC_INTERFACE_IN) | ||
1549 | usb_kill_urb(dec->irq_urb); | ||
1550 | |||
1551 | usb_free_urb(dec->irq_urb); | ||
1552 | |||
1553 | usb_free_coherent(dec->udev,IRQ_PACKET_SIZE, | ||
1554 | dec->irq_buffer, dec->irq_dma_handle); | ||
1555 | 1545 | ||
1556 | if (dec->rc_input_dev) { | 1546 | if (dec->rc_input_dev) { |
1557 | input_unregister_device(dec->rc_input_dev); | 1547 | input_unregister_device(dec->rc_input_dev); |
@@ -1566,6 +1556,20 @@ static void ttusb_dec_exit_usb(struct ttusb_dec *dec) | |||
1566 | 1556 | ||
1567 | dprintk("%s\n", __func__); | 1557 | dprintk("%s\n", __func__); |
1568 | 1558 | ||
1559 | if (enable_rc) { | ||
1560 | /* we have to check whether the irq URB is already submitted. | ||
1561 | * As the irq is submitted after the interface is changed, | ||
1562 | * this is the best method i figured out. | ||
1563 | * Any others?*/ | ||
1564 | if (dec->interface == TTUSB_DEC_INTERFACE_IN) | ||
1565 | usb_kill_urb(dec->irq_urb); | ||
1566 | |||
1567 | usb_free_urb(dec->irq_urb); | ||
1568 | |||
1569 | usb_free_coherent(dec->udev, IRQ_PACKET_SIZE, | ||
1570 | dec->irq_buffer, dec->irq_dma_handle); | ||
1571 | } | ||
1572 | |||
1569 | dec->iso_stream_count = 0; | 1573 | dec->iso_stream_count = 0; |
1570 | 1574 | ||
1571 | for (i = 0; i < ISO_BUF_COUNT; i++) | 1575 | for (i = 0; i < ISO_BUF_COUNT; i++) |
@@ -1623,6 +1627,7 @@ static int ttusb_dec_probe(struct usb_interface *intf, | |||
1623 | { | 1627 | { |
1624 | struct usb_device *udev; | 1628 | struct usb_device *udev; |
1625 | struct ttusb_dec *dec; | 1629 | struct ttusb_dec *dec; |
1630 | int result; | ||
1626 | 1631 | ||
1627 | dprintk("%s\n", __func__); | 1632 | dprintk("%s\n", __func__); |
1628 | 1633 | ||
@@ -1651,13 +1656,15 @@ static int ttusb_dec_probe(struct usb_interface *intf, | |||
1651 | 1656 | ||
1652 | dec->udev = udev; | 1657 | dec->udev = udev; |
1653 | 1658 | ||
1654 | if (ttusb_dec_init_usb(dec)) | 1659 | result = ttusb_dec_init_usb(dec); |
1655 | return 0; | 1660 | if (result) |
1656 | if (ttusb_dec_init_stb(dec)) { | 1661 | goto err_usb; |
1657 | ttusb_dec_exit_usb(dec); | 1662 | result = ttusb_dec_init_stb(dec); |
1658 | return 0; | 1663 | if (result) |
1659 | } | 1664 | goto err_stb; |
1660 | ttusb_dec_init_dvb(dec); | 1665 | result = ttusb_dec_init_dvb(dec); |
1666 | if (result) | ||
1667 | goto err_stb; | ||
1661 | 1668 | ||
1662 | dec->adapter.priv = dec; | 1669 | dec->adapter.priv = dec; |
1663 | switch (id->idProduct) { | 1670 | switch (id->idProduct) { |
@@ -1696,6 +1703,11 @@ static int ttusb_dec_probe(struct usb_interface *intf, | |||
1696 | ttusb_init_rc(dec); | 1703 | ttusb_init_rc(dec); |
1697 | 1704 | ||
1698 | return 0; | 1705 | return 0; |
1706 | err_stb: | ||
1707 | ttusb_dec_exit_usb(dec); | ||
1708 | err_usb: | ||
1709 | kfree(dec); | ||
1710 | return result; | ||
1699 | } | 1711 | } |
1700 | 1712 | ||
1701 | static void ttusb_dec_disconnect(struct usb_interface *intf) | 1713 | static void ttusb_dec_disconnect(struct usb_interface *intf) |