Skip to contentSkip to conversation

media: ipu6: Move isys fw mapping to pci_probe

bug report Dan Carpenter 1 participant 1 message Sep 15 Media (V4L/DVB)

lore
Share & tools
lore
#0 lore raw

Hello Antti Laakso,

Commit 69ebb1bfc865 ("media: ipu6: Move isys fw mapping to pci_probe") from Aug 27, 2026 (linux-next), leads to the following Smatch static checker warning:

	drivers/media/pci/intel/ipu6/ipu6.c:865 ipu6_pci_remove()
	warn: variable dereferenced before check 'isp->psys' (see line 844)

drivers/media/pci/intel/ipu6/ipu6.c

    836 static void ipu6_pci_remove(struct pci_dev *pdev)
    837 {
    838         struct ipu6_device *isp = pci_get_drvdata(pdev);
    839         unsigned long dir;
    840 
    841         devm_free_irq(&pdev->dev, pdev->irq, isp);
    842 
    843         dir = IS_IPU7(isp) ? DMA_BIDIRECTIONAL : DMA_TO_DEVICE;
    844         ipu6_cpd_free_pkg_dir(isp->psys);

ipu6_cpd_free_pkg_dir() dereferences isp->psys so

    845         ipu6_unmap_fw_region(isp->psys, dir);
    846 
    847         if (isp->isys) {

is too late. Fortunately, it's also unnecessary. Just delete it.

    848                 ipu6_cpd_free_pkg_dir(isp->isys);
    849                 if (isp->isys->fw_sgt.nents)
    850                         ipu6_unmap_fw_region(isp->isys, dir);
    851         }
    852 
    853         vfree(isp->fw_code_region);
    854         isp->fw_code_region = NULL;
    855 
    856         ipu6_buttress_exit(isp);
    857 
    858         ipu6_bus_del_devices(pdev);

I'm using ChatGPT to review Smatch warnings these days and it gives a bonus bug that ipu6_bus_del_devices() frees isp->psys.

"ipu6_bus_del_devices() calls:

	auxiliary_device_delete(&adev->auxdev);
	auxiliary_device_uninit(&adev->auxdev);

The uninit drops the device reference, and ipu6_bus_release() frees the containing ipu6_bus_device. Consequently, accessing isp->psys->mmu or isp->isys->mmu afterward can use freed memory."

    859 
    860         pm_runtime_forbid(&pdev->dev);
    861         pm_runtime_get_noresume(&pdev->dev);
    862 
    863         release_firmware(isp->cpd_fw);
    864 
--> 865         if (isp->psys)
    866                 ipu6_mmu_cleanup(isp->psys->mmu);
                                         ^^^^^^^^^

    867         if (isp->isys)
    868                 ipu6_mmu_cleanup(isp->isys->mmu);
                                         ^^^^^^^^^

So these dereferences are a use after free. Originally, this code saved the pointer the mmus before calling ipu6_bus_del_devices().

    869 }

This email is a free service from the Smatch-CI project [smatch.sf.net].

regards,
dan carpenter