iommu: check if group is NULL before remove device
PATCH 1/1 iommu: check if group is NULL before remove device
From: Frank Wunderlich <[email protected]>
if probe is failing, iommu_group may be not initialized, so freeing it will result in NULL pointer access
--- drivers/iommu/iommu.c | 3 +++ 1 file changed, 3 insertions(+)
drivers/iommu/iommu.c +3 -0
@@ -924,6 +924,9 @@ void iommu_group_remove_device(struct device *dev)struct iommu_group *group = dev->iommu_group;struct group_device *tmp_device, *device =NULL ; +if (!group) +return ; +dev_info (dev,"Removing from iommu group %d\n" , group->id);/* Pre-notify listeners that a device is being removed. */
Signature · 1 line
2.25.1
Joerg Roedel
maintainerQuoted 3 lines · Frank Wunderlich
From: Frank Wunderlich <[email protected]>
if probe is failing,
iommu_groupmay be not initialized,
Sentences start with capital letters.
IOMMU patch subjects too, after the 'iommu:' prefix.
Quoted 1 line
so freeing it will result in NULL pointer access
Please describe in more detail how this NULL-ptr dereference is triggered.
Regards,
Joerg
Hi Joerg,
Sorry for late reply, somehow i marked message as read without answering it.
Am 15. Juli 2021 09:20:04 MESZ schrieb Joerg Roedel <[email protected]>:
Quoted 8 lines · Frank Wunderlich
Quoted 3 lines
From: Frank Wunderlich <[email protected]>
if probe is failing,
iommu_groupmay be not initialized,Sentences start with capital letters.
IOMMU patch subjects too, after the 'iommu:' prefix.
Will fix these in v2
Quoted 4 lines
Quoted 1 line
so freeing it will result in NULL pointer access
Please describe in more detail how this NULL-ptr dereference is triggered.
I had this by testing this series:
https://patchwork.kernel.org/project/linux-mediatek/list/?series=515129
Initialization in mtk driver was failed (i guess the iommu group was not yet created), cleanup was started and so this function is called with a NULL group pointer. I can try to find my debug-trace if you need a kind of backtrace.
regards Frank
Am 2021-07-15 09:20, schrieb Joerg Roedel:
Quoted 5 lines · Frank Wunderlich
Quoted 3 lines
From: Frank Wunderlich <[email protected]>
if probe is failing,
iommu_groupmay be not initialized,
Quoted 8 lines
Sentences start with capital letters.
IOMMU patch subjects too, after the 'iommu:' prefix.
Quoted 1 line
so freeing it will result in NULL pointer access
Please describe in more detail how this NULL-ptr dereference is triggered.
in my case probe (mtk_iommu_probe_device called from __iommu_probe_device) is failing due to fwspec missing and then dev_iommu_free/iommu_fwspec_free is called, later iommu_group_remove_device with group=NULL
i think i've found problem:
iommu_probe_device:
group = iommu_group_get(dev);
if (!group) { //group is checked here for NULL but accessed later
ret = -ENODEV;
goto err_release; <<<
}
err_release:<<<
iommu_release_device(dev);------------------------------------------------------------------------------
void iommu_release_device(struct device *dev)
{
...
iommu_group_remove_device(dev);
------------------------------------------------------------------------------
void iommu_group_remove_device(struct device *dev)
{
struct iommu_group *group = dev->iommu_group;
struct group_device *tmp_device, *device = NULL;
...
dev_info(dev, "Removing from iommu group %d\n", group->id); //crash as group is NULL and not checked