Bug 188991 - Function r592_probe() forgets to set error codes on failures, which may result in user-after-free bugs
Summary: Function r592_probe() forgets to set error codes on failures, which may resul...
Status: NEW
Alias: None
Product: Drivers
Classification: Unclassified
Component: Other (show other bugs)
Hardware: All Linux
: P1 normal
Assignee: drivers_other
Depends on:
Reported: 2016-11-25 11:15 UTC by bianpan
Modified: 2016-11-25 11:15 UTC (History)
0 users

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


Description bianpan 2016-11-25 11:15:15 UTC
Function r592_probe() defined in file drivers/memstick/host/r592.c calls pci_ioremap_bar() and checks the return value against NULL (at line 765). If the return value is NULL, the control flow jumps to label "error4", frees memory and returns variable error. Because variable error is checked at line 761, its value must be 0 when pci_ioremap_bar() is called. So r592_probe() may return 0 (indicates success) even on failures. Because the allocated memory are freed on failures, pointer dev->host will point to freed memory. However, because r592_probe() may return 0 on failures, its caller cannot detect the errors (such as pci_ioremap_bar() fails), and will try to access dev->host, which causes use-after-free bugs.
There are other two similar bugs at lines 793 and 798. 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.

r592_probe @@ drivers/memstick/host/r592.c
734 static int r592_probe(struct pci_dev *pdev, const struct pci_device_id *id)
735 {
736     int error = -ENOMEM;
737     struct memstick_host *host;
760     error = pci_request_regions(pdev, DRV_NAME);
761     if (error)
762         goto error3;
        // insert "error = -ENOMEM;" may mitigate the following bugs
764     dev->mmio = pci_ioremap_bar(pdev, 0);
765     if (!dev->mmio)
        // Bug (1): forgets to set error code
766         goto error4;
793     if (request_irq(dev->irq, &r592_irq, IRQF_SHARED,
794               DRV_NAME, dev))
        // Bug (2): forgets to set error code
795         goto error6;
797     r592_update_card_detect(dev);
798     if (memstick_add_host(host))
        // Bug (3): forgets to set error code
799         goto error7;
801     message("driver successfully loaded");
802     return 0;
803 error7:
804     free_irq(dev->irq, dev);
805 error6:
806     if (dev->dummy_dma_page)
807         dma_free_coherent(&pdev->dev, PAGE_SIZE, dev->dummy_dma_page,
808             dev->dummy_dma_page_physical_address);
810     kthread_stop(dev->io_thread);
811 error5:
812     iounmap(dev->mmio);
813 error4:
814     pci_release_regions(pdev);
815 error3:
816     pci_disable_device(pdev);
817 error2:
818     memstick_free_host(host);
819 error1:
820     return error;
821 }

Thanks very much!

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