Re: [PATCH v2 03/14] iommu: Make __iommu_group_set_domain() handle error unwind

[Baolu Lu <baolu.lu@linux.intel.com>]

On 3/30/23 7:40 AM, Jason Gunthorpe wrote:
> Let's try to have a consistent and clear strategy for error handling
> during domain attach failures.
> 
> There are two broad categories, the first is callers doing destruction and
> trying to set the domain back to a previously good domain. These cases
> cannot handle failure during destruction flows and must succeed, or at
> least avoid a UAF on the current group->domain which is likely about to be
> freed.
> 
> Many of the drivers are well behaved here and will not hit the WARN_ON's
> or a UAF, but some are doing hypercalls/etc that can fail unpredictably
> and don't meet the expectations.
> 
> The second case is attaching a domain for the first time in a failable
> context, failure should restore the attachment back to group->domain using
> the above unfailable operation.
> 
> Have __iommu_group_set_domain_internal() execute a common algorithm that
> tries to achieve this, and in the worst case, would leave a device
> "detached" or assigned to a global blocking domain. This relies on some
> existing common driver behaviors where attach failure will also do detatch
> and true IOMMU_DOMAIN_BLOCK implementations that are not allowed to ever
> fail.
> 
> Name the first case with __iommu_group_set_domain_nofail() to make it
> clear.
> 
> Pull all the error handling and WARN_ON generation into
> __iommu_group_set_domain_internal().
> 
> Avoid the obfuscating use of __iommu_group_for_each_dev() and be more
> careful about what should happen during failures by only touching devices
> we've already touched.
> 
> Reviewed-by: Kevin Tian<kevin.tian@intel.com>
> Signed-off-by: Jason Gunthorpe<jgg@nvidia.com>

Reviewed-by: Lu Baolu <baolu.lu@linux.intel.com>

Best regards,
baolu