Bug 189011 - Function meye_probe() does not set error codes on some failures
Summary: Function meye_probe() does not set error codes on some failures
Status: RESOLVED CODE_FIX
Alias: None
Product: Drivers
Classification: Unclassified
Component: Other (show other bugs)
Hardware: All Linux
: P1 normal
Assignee: drivers_other
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2016-11-25 11:17 UTC by bianpan
Modified: 2017-05-12 00:24 UTC (History)
0 users

See Also:
Kernel Version: linux-4.9-rc6
Subsystem:
Regression: No
Bisected commit-id:


Attachments
The patch fixes the bug (1.05 KB, patch)
2017-05-12 00:24 UTC, bianpan
Details | Diff

Description bianpan 2016-11-25 11:17:08 UTC
In function meye_probe() defined in file drivers/media/pci/meye/meye.c, because variable ret is checked at line 1667, its value must be 0 when pci_resource_start() is called (at line 1672). If pci_resource_start() returns a NULL pointer, the control flow jumps to label "outregions", cleans and returns the value of ret (i.e. 0). As a result, function meye_probe() returns 0 (indicates success) even if there are errors. The behavior of its caller may be misled. 
There are other 5 similar bugs when the function calls fail at lines 1677, 1684, 1690, 1734, and 1743. Though these errors may occur rarely, I think it is better to set the correct error codes on failures. Codes related to these bugs are summarised as follows.

meye_probe @@ drivers/media/pci/meye/meye.c
1592 static int meye_probe(struct pci_dev *pcidev, const struct pci_device_id *ent)
1593 {
         ...
1667     if ((ret = pci_enable_device(meye.mchip_dev))) {
1668         v4l2_err(v4l2_dev, "meye: pci_enable_device failed\n");
1669         goto outenabledev;
1670     }
1671 
         // insert "ret = -EIO;" ?
1672     mchip_adr = pci_resource_start(meye.mchip_dev,0);
1673     if (!mchip_adr) {
1674         v4l2_err(v4l2_dev, "meye: mchip has no device base address\n");
             // Bug (1): variable ret takes value 0
1675         goto outregions;
1676     }
1677     if (!request_mem_region(pci_resource_start(meye.mchip_dev, 0),
1678                 pci_resource_len(meye.mchip_dev, 0),
1679                 "meye")) {
1680         v4l2_err(v4l2_dev, "meye: request_mem_region failed\n");
             // Bug (2): variable ret takes value 0
1681         goto outregions;
1682     }
1683     meye.mchip_mmregs = ioremap(mchip_adr, MCHIP_MM_REGS);
1684     if (!meye.mchip_mmregs) {
1685         v4l2_err(v4l2_dev, "meye: ioremap failed\n");
             // Bug (3): variable ret takes value 0
1686         goto outremap;
1687     }
1688 
1689     meye.mchip_irq = pcidev->irq;
1690     if (request_irq(meye.mchip_irq, meye_irq,
1691             IRQF_SHARED, "meye", meye_irq)) {
1692         v4l2_err(v4l2_dev, "request_irq failed\n");
             // Bug (4): variable ret takes value 0
1693         goto outreqirq;
1694     }

         ...
1735     if (meye.hdl.error) {
1736         v4l2_err(v4l2_dev, "couldn't register controls\n");
             // Bug (5): variable ret takes value 0
1737         goto outvideoreg;
1738     }
1739 
1740     v4l2_ctrl_handler_setup(&meye.hdl);
1741     meye.vdev.ctrl_handler = &meye.hdl;
1742 
1743     if (video_register_device(&meye.vdev, VFL_TYPE_GRABBER,
1744                   video_nr) < 0) {
1745         v4l2_err(v4l2_dev, "video_register_device failed\n");
             // Bug (6): variable ret takes value 0
1746         goto outvideoreg;
1747     }
1748 
1749     v4l2_info(v4l2_dev, "Motion Eye Camera Driver v%s.\n",
1750            MEYE_DRIVER_VERSION);
1751     v4l2_info(v4l2_dev, "mchip KL5A72002 rev. %d, base %lx, irq %d\n",
1752            meye.mchip_dev->revision, mchip_adr, meye.mchip_irq);
1753 
1754     return 0;
1755 
1756 outvideoreg:
1757     v4l2_ctrl_handler_free(&meye.hdl);
1758     free_irq(meye.mchip_irq, meye_irq);
1759 outreqirq:
1760     iounmap(meye.mchip_mmregs);
1761 outremap:
1762     release_mem_region(pci_resource_start(meye.mchip_dev, 0),
1763                pci_resource_len(meye.mchip_dev, 0));
1764 outregions:
1765     pci_disable_device(meye.mchip_dev);
1766 outenabledev:
1767     sony_pic_camera_command(SONY_PIC_COMMAND_SETCAMERA, 0);
1768 outsonypienable:
1769     kfifo_free(&meye.doneq);
1770 outkfifoalloc2:
1771     kfifo_free(&meye.grabq);
1772 outkfifoalloc1:
1773     vfree(meye.grab_temp);
1774 outvmalloc:
1775     return ret;
1776 }

Thanks very much!
Comment 1 bianpan 2017-05-12 00:24:31 UTC
Created attachment 256447 [details]
The patch fixes the bug

The patch has been merged into the latest version of the Linux kernel. So I will close the bug.

Note You need to log in before you can comment on or make changes to this bug.