linux-kernel - RE: [PATCH v3 1/5] iommufd: Add iommufd_object_tombstone

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <BL1PR11MB5271410ADB704F459C77BC248C46A@BL1PR11MB5271.namprd11.prod.outlook.com>
Date: Mon, 30 Jun 2025 05:52:26 +0000
From: "Tian, Kevin" <kevin.tian@...el.com>
To: Xu Yilun <yilun.xu@...ux.intel.com>, "jgg@...dia.com" <jgg@...dia.com>,
	"jgg@...pe.ca" <jgg@...pe.ca>, "will@...nel.org" <will@...nel.org>,
	"aneesh.kumar@...nel.org" <aneesh.kumar@...nel.org>
CC: "iommu@...ts.linux.dev" <iommu@...ts.linux.dev>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"joro@...tes.org" <joro@...tes.org>, "robin.murphy@....com"
	<robin.murphy@....com>, "shuah@...nel.org" <shuah@...nel.org>,
	"nicolinc@...dia.com" <nicolinc@...dia.com>, "aik@....com" <aik@....com>,
	"Williams, Dan J" <dan.j.williams@...el.com>, "baolu.lu@...ux.intel.com"
	<baolu.lu@...ux.intel.com>, "Xu, Yilun" <yilun.xu@...el.com>
Subject: RE: [PATCH v3 1/5] iommufd: Add iommufd_object_tombstone_user()
 helper

> From: Xu Yilun <yilun.xu@...ux.intel.com>
> Sent: Friday, June 27, 2025 11:38 AM
>
> @@ -239,6 +239,7 @@ static int iommufd_fops_release(struct inode *inode,
> struct file *filp)
>  	struct iommufd_sw_msi_map *next;
>  	struct iommufd_sw_msi_map *cur;
>  	struct iommufd_object *obj;
> +	bool empty;

move into for(;;) loop

> 
>  	/*
>  	 * The objects in the xarray form a graph of "users" counts, and we
> have
> @@ -249,23 +250,35 @@ static int iommufd_fops_release(struct inode
> *inode, struct file *filp)
>  	 * until the entire list is destroyed. If this can't progress then there
>  	 * is some bug related to object refcounting.
>  	 */
> -	while (!xa_empty(&ictx->objects)) {
> +	for (;;) {
>  		unsigned int destroyed = 0;
>  		unsigned long index;
> 
> +		empty = true;
>  		xa_for_each(&ictx->objects, index, obj) {
> +			empty = false;
>  			if (!refcount_dec_if_one(&obj->users))
>  				continue;
> +
>  			destroyed++;
>  			xa_erase(&ictx->objects, index);
>  			iommufd_object_ops[obj->type].destroy(obj);
>  			kfree(obj);
>  		}
> +
> +		if (empty)
> +			break;
> +
>  		/* Bug related to users refcount */
>  		if (WARN_ON(!destroyed))
>  			break;
>  	}
> -	WARN_ON(!xa_empty(&ictx->groups));

I didn't get the rationale of this change. tombstone only means the
object ID reserved, but all the destroy work for the object has been
done, e.g. after all idevice objects are destroyed there should be no
valid igroup entries in ictx->groups (and there is no tombstone
state for igroup).

Is it a wrong change by misreading ictx->groups as ictx->objects?

> +
> +	/*
> +	 * There may be some tombstones left over from
> +	 * iommufd_object_tombstone_user()
> +	 */
> +	xa_destroy(&ictx->groups);
> 

And here should be ictx->objects?