Re: [PATCH v3 03/17] iommufd: Replace the hwpt->devices list with iommufd_group

From: Jason Gunthorpe <jgg@nvidia.com>
To: "Tian, Kevin" <kevin.tian@intel.com>
Cc: "iommu@lists.linux.dev" <iommu@lists.linux.dev>,
	"linux-kselftest@vger.kernel.org"
	<linux-kselftest@vger.kernel.org>,
	Lu Baolu <baolu.lu@linux.intel.com>,
	"kvm@vger.kernel.org" <kvm@vger.kernel.org>,
	Nicolin Chen <nicolinc@nvidia.com>,
	"Liu, Yi L" <yi.l.liu@intel.com>
Subject: Re: [PATCH v3 03/17] iommufd: Replace the hwpt->devices list with iommufd_group
Date: Tue, 11 Apr 2023 11:31:09 -0300	[thread overview]
Message-ID: <ZDVvLbSN2TR1Er1c@nvidia.com> (raw)
In-Reply-To: <BN9PR11MB5276E42B629C3E5AF019B6748C879@BN9PR11MB5276.namprd11.prod.outlook.com>

On Thu, Mar 23, 2023 at 07:21:42AM +0000, Tian, Kevin wrote:

> If no oversight then we can directly put the lock in
> iommufd_hw_pagetable_attach/detach() which can also simplify a bit on 
> its callers in device.c.

So, I did this, and syzkaller explains why this can't be done:

https://lore.kernel.org/r/0000000000006e66d605f83e09bc@google.com

We can't allow the hwpt to be discovered by a parallel
iommufd_hw_pagetable_attach() until it is done being setup, otherwise
if we fail to set it up we can't destroy the hwpt.

	if (immediate_attach) {
		rc = iommufd_hw_pagetable_attach(hwpt, idev);
		if (rc)
			goto out_abort;
	}

	rc = iopt_table_add_domain(&hwpt->ioas->iopt, hwpt->domain);
	if (rc)
		goto out_detach;
	list_add_tail(&hwpt->hwpt_item, &hwpt->ioas->hwpt_list);
	return hwpt;

out_detach:
	if (immediate_attach)
		iommufd_hw_pagetable_detach(idev);
out_abort:
	iommufd_object_abort_and_destroy(ictx, &hwpt->obj);

As some other idev could be pointing at it too now.

So the lock has to come back out..

Jason