Linux kernel mirror (for testing) git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel os linux

[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>

authored by

Alexey Khoroshilov and committed by
Mauro Carvalho Chehab
cf732b5f b4559ace

+82 -70
+82 -70
drivers/media/usb/ttusb-dec/ttusb_dec.c
··· 366 366 } 367 367 return 0; 368 368 } else { 369 - return -1; 369 + return -ENOENT; 370 370 } 371 371 } 372 372 ··· 1241 1241 1242 1242 static int ttusb_dec_init_usb(struct ttusb_dec *dec) 1243 1243 { 1244 + int result; 1245 + 1244 1246 dprintk("%s\n", __func__); 1245 1247 1246 1248 mutex_init(&dec->usb_mutex); ··· 1260 1258 return -ENOMEM; 1261 1259 } 1262 1260 dec->irq_buffer = usb_alloc_coherent(dec->udev,IRQ_PACKET_SIZE, 1263 - GFP_ATOMIC, &dec->irq_dma_handle); 1261 + GFP_KERNEL, &dec->irq_dma_handle); 1264 1262 if(!dec->irq_buffer) { 1265 1263 usb_free_urb(dec->irq_urb); 1266 1264 return -ENOMEM; ··· 1272 1270 dec->irq_urb->transfer_flags |= URB_NO_TRANSFER_DMA_MAP; 1273 1271 } 1274 1272 1275 - return ttusb_dec_alloc_iso_urbs(dec); 1273 + result = ttusb_dec_alloc_iso_urbs(dec); 1274 + if (result) { 1275 + usb_free_urb(dec->irq_urb); 1276 + usb_free_coherent(dec->udev, IRQ_PACKET_SIZE, 1277 + dec->irq_buffer, dec->irq_dma_handle); 1278 + } 1279 + return result; 1276 1280 } 1277 1281 1278 1282 static int ttusb_dec_boot_dsp(struct ttusb_dec *dec) ··· 1301 1293 1302 1294 dprintk("%s\n", __func__); 1303 1295 1304 - if (request_firmware(&fw_entry, dec->firmware_name, &dec->udev->dev)) { 1296 + result = request_firmware(&fw_entry, dec->firmware_name, &dec->udev->dev); 1297 + if (result) { 1305 1298 printk(KERN_ERR "%s: Firmware (%s) unavailable.\n", 1306 1299 __func__, dec->firmware_name); 1307 - return 1; 1300 + return result; 1308 1301 } 1309 1302 1310 1303 firmware = fw_entry->data; ··· 1315 1306 printk("%s: firmware size too small for DSP code (%zu < 60).\n", 1316 1307 __func__, firmware_size); 1317 1308 release_firmware(fw_entry); 1318 - return -1; 1309 + return -ENOENT; 1319 1310 } 1320 1311 1321 1312 /* a 32 bit checksum over the first 56 bytes of the DSP Code is stored ··· 1329 1320 "0x%08x != 0x%08x in file), file invalid.\n", 1330 1321 __func__, crc32_csum, crc32_check); 1331 1322 release_firmware(fw_entry); 1332 - return -1; 1323 + return -ENOENT; 1333 1324 } 1334 1325 memcpy(idstring, &firmware[36], 20); 1335 1326 idstring[20] = '\0'; ··· 1398 1389 dprintk("%s\n", __func__); 1399 1390 1400 1391 result = ttusb_dec_get_stb_state(dec, &mode, &model, &version); 1392 + if (result) 1393 + return result; 1401 1394 1402 - if (!result) { 1403 - if (!mode) { 1404 - if (version == 0xABCDEFAB) 1405 - printk(KERN_INFO "ttusb_dec: no version " 1406 - "info in Firmware\n"); 1407 - else 1408 - printk(KERN_INFO "ttusb_dec: Firmware " 1409 - "%x.%02x%c%c\n", 1410 - version >> 24, (version >> 16) & 0xff, 1411 - (version >> 8) & 0xff, version & 0xff); 1395 + if (!mode) { 1396 + if (version == 0xABCDEFAB) 1397 + printk(KERN_INFO "ttusb_dec: no version " 1398 + "info in Firmware\n"); 1399 + else 1400 + printk(KERN_INFO "ttusb_dec: Firmware " 1401 + "%x.%02x%c%c\n", 1402 + version >> 24, (version >> 16) & 0xff, 1403 + (version >> 8) & 0xff, version & 0xff); 1412 1404 1413 - result = ttusb_dec_boot_dsp(dec); 1414 - if (result) 1415 - return result; 1416 - else 1417 - return 1; 1418 - } else { 1419 - /* We can't trust the USB IDs that some firmwares 1420 - give the box */ 1421 - switch (model) { 1422 - case 0x00070001: 1423 - case 0x00070008: 1424 - case 0x0007000c: 1425 - ttusb_dec_set_model(dec, TTUSB_DEC3000S); 1426 - break; 1427 - case 0x00070009: 1428 - case 0x00070013: 1429 - ttusb_dec_set_model(dec, TTUSB_DEC2000T); 1430 - break; 1431 - case 0x00070011: 1432 - ttusb_dec_set_model(dec, TTUSB_DEC2540T); 1433 - break; 1434 - default: 1435 - printk(KERN_ERR "%s: unknown model returned " 1436 - "by firmware (%08x) - please report\n", 1437 - __func__, model); 1438 - return -1; 1439 - break; 1440 - } 1441 - 1405 + result = ttusb_dec_boot_dsp(dec); 1406 + if (result) 1407 + return result; 1408 + } else { 1409 + /* We can't trust the USB IDs that some firmwares 1410 + give the box */ 1411 + switch (model) { 1412 + case 0x00070001: 1413 + case 0x00070008: 1414 + case 0x0007000c: 1415 + ttusb_dec_set_model(dec, TTUSB_DEC3000S); 1416 + break; 1417 + case 0x00070009: 1418 + case 0x00070013: 1419 + ttusb_dec_set_model(dec, TTUSB_DEC2000T); 1420 + break; 1421 + case 0x00070011: 1422 + ttusb_dec_set_model(dec, TTUSB_DEC2540T); 1423 + break; 1424 + default: 1425 + printk(KERN_ERR "%s: unknown model returned " 1426 + "by firmware (%08x) - please report\n", 1427 + __func__, model); 1428 + return -ENOENT; 1429 + } 1442 1430 if (version >= 0x01770000) 1443 1431 dec->can_playback = 1; 1444 - 1445 - return 0; 1446 - } 1447 1432 } 1448 - else 1449 - return result; 1433 + return 0; 1450 1434 } 1451 1435 1452 1436 static int ttusb_dec_init_dvb(struct ttusb_dec *dec) ··· 1541 1539 1542 1540 static void ttusb_dec_exit_rc(struct ttusb_dec *dec) 1543 1541 { 1544 - 1545 1542 dprintk("%s\n", __func__); 1546 - /* we have to check whether the irq URB is already submitted. 1547 - * As the irq is submitted after the interface is changed, 1548 - * this is the best method i figured out. 1549 - * Any others?*/ 1550 - if (dec->interface == TTUSB_DEC_INTERFACE_IN) 1551 - usb_kill_urb(dec->irq_urb); 1552 - 1553 - usb_free_urb(dec->irq_urb); 1554 - 1555 - usb_free_coherent(dec->udev,IRQ_PACKET_SIZE, 1556 - dec->irq_buffer, dec->irq_dma_handle); 1557 1543 1558 1544 if (dec->rc_input_dev) { 1559 1545 input_unregister_device(dec->rc_input_dev); ··· 1555 1565 int i; 1556 1566 1557 1567 dprintk("%s\n", __func__); 1568 + 1569 + if (enable_rc) { 1570 + /* we have to check whether the irq URB is already submitted. 1571 + * As the irq is submitted after the interface is changed, 1572 + * this is the best method i figured out. 1573 + * Any others?*/ 1574 + if (dec->interface == TTUSB_DEC_INTERFACE_IN) 1575 + usb_kill_urb(dec->irq_urb); 1576 + 1577 + usb_free_urb(dec->irq_urb); 1578 + 1579 + usb_free_coherent(dec->udev, IRQ_PACKET_SIZE, 1580 + dec->irq_buffer, dec->irq_dma_handle); 1581 + } 1558 1582 1559 1583 dec->iso_stream_count = 0; 1560 1584 ··· 1627 1623 { 1628 1624 struct usb_device *udev; 1629 1625 struct ttusb_dec *dec; 1626 + int result; 1630 1627 1631 1628 dprintk("%s\n", __func__); 1632 1629 ··· 1656 1651 1657 1652 dec->udev = udev; 1658 1653 1659 - if (ttusb_dec_init_usb(dec)) 1660 - return 0; 1661 - if (ttusb_dec_init_stb(dec)) { 1662 - ttusb_dec_exit_usb(dec); 1663 - return 0; 1664 - } 1665 - ttusb_dec_init_dvb(dec); 1654 + result = ttusb_dec_init_usb(dec); 1655 + if (result) 1656 + goto err_usb; 1657 + result = ttusb_dec_init_stb(dec); 1658 + if (result) 1659 + goto err_stb; 1660 + result = ttusb_dec_init_dvb(dec); 1661 + if (result) 1662 + goto err_stb; 1666 1663 1667 1664 dec->adapter.priv = dec; 1668 1665 switch (id->idProduct) { ··· 1703 1696 ttusb_init_rc(dec); 1704 1697 1705 1698 return 0; 1699 + err_stb: 1700 + ttusb_dec_exit_usb(dec); 1701 + err_usb: 1702 + kfree(dec); 1703 + return result; 1706 1704 } 1707 1705 1708 1706 static void ttusb_dec_disconnect(struct usb_interface *intf)