Commit 797a8b4d authored by Robin Murphy's avatar Robin Murphy Committed by Joerg Roedel
Browse files

iommu: Handle default domain attach failure 



We wouldn't normally expect ops->attach_dev() to fail, but on IOMMUs
with limited hardware resources, or generally misconfigured systems,
it is certainly possible. We report failure correctly from the external
iommu_attach_device() interface, but do not do so in iommu_group_add()
when attaching to the default domain. The result of failure there is
that the device, group and domain all get left in a broken,
part-configured state which leads to weird errors and misbehaviour down
the line when IOMMU API calls sort-of-but-don't-quite work.

Check the return value of __iommu_attach_device() on the default domain,
and refactor the error handling paths to cope with its failure and clean
up correctly in such cases.

Fixes: e39cb8a3 ("iommu: Make sure a device is always attached to a domain")
Reported-by: default avatarPunit Agrawal <punit.agrawal@arm.com>
Signed-off-by: default avatarRobin Murphy <robin.murphy@arm.com>
Signed-off-by: default avatarJoerg Roedel <jroedel@suse.de>
parent eba484b5

drivers/iommu/iommu.c

+24 −13
Original line number Diff line number Diff line
@@ -383,36 +383,30 @@ int iommu_group_add_device(struct iommu_group *group, struct device *dev) 
	device->dev = dev;



	ret = sysfs_create_link(&dev->kobj, &group->kobj, "iommu_group");

	if (ret) {

		kfree(device);

		return ret;

	}

	if (ret)

		goto err_free_device;



	device->name = kasprintf(GFP_KERNEL, "%s", kobject_name(&dev->kobj));

rename:

	if (!device->name) {

		sysfs_remove_link(&dev->kobj, "iommu_group");

		kfree(device);

		return -ENOMEM;

		ret = -ENOMEM;

		goto err_remove_link;

	}



	ret = sysfs_create_link_nowarn(group->devices_kobj,

				       &dev->kobj, device->name);

	if (ret) {

		kfree(device->name);

		if (ret == -EEXIST && i >= 0) {

			/*

			 * Account for the slim chance of collision

			 * and append an instance to the name.

			 */

			kfree(device->name);

			device->name = kasprintf(GFP_KERNEL, "%s.%d",

						 kobject_name(&dev->kobj), i++);

			goto rename;

		}



		sysfs_remove_link(&dev->kobj, "iommu_group");

		kfree(device);

		return ret;

		goto err_free_name;

	}



	kobject_get(group->devices_kobj);
@@ -424,8 +418,10 @@ rename: 
	mutex_lock(&group->mutex);

	list_add_tail(&device->list, &group->devices);

	if (group->domain)

		__iommu_attach_device(group->domain, dev);

		ret = __iommu_attach_device(group->domain, dev);

	mutex_unlock(&group->mutex);

	if (ret)

		goto err_put_group;



	/* Notify any listeners about change to group. */

	blocking_notifier_call_chain(&group->notifier,
@@ -436,6 +432,21 @@ rename: 
	pr_info("Adding device %s to group %d\n", dev_name(dev), group->id);



	return 0;



err_put_group:

	mutex_lock(&group->mutex);

	list_del(&device->list);

	mutex_unlock(&group->mutex);

	dev->iommu_group = NULL;

	kobject_put(group->devices_kobj);

err_free_name:

	kfree(device->name);

err_remove_link:

	sysfs_remove_link(&dev->kobj, "iommu_group");

err_free_device:

	kfree(device);

	pr_err("Failed to add device %s to group %d: %d\n", dev_name(dev), group->id, ret);

	return ret;

}

EXPORT_SYMBOL_GPL(iommu_group_add_device);

CRA Git | Maintained and supported by SUSTech CRA and CCSE