linux-kernel - Re: [PATCH v4 4/7] iommufd: Destroy vdevice on idevice destroy

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: <aHKfwQ41x28bNWAL@yilunxu-OptiPlex-7050>
Date: Sun, 13 Jul 2025 01:47:45 +0800
From: Xu Yilun <yilun.xu@...ux.intel.com>
To: Nicolin Chen <nicolinc@...dia.com>
Cc: jgg@...dia.com, jgg@...pe.ca, kevin.tian@...el.com, will@...nel.org,
	aneesh.kumar@...nel.org, iommu@...ts.linux.dev,
	linux-kernel@...r.kernel.org, joro@...tes.org, robin.murphy@....com,
	shuah@...nel.org, aik@....com, dan.j.williams@...el.com,
	baolu.lu@...ux.intel.com, yilun.xu@...el.com
Subject: Re: [PATCH v4 4/7] iommufd: Destroy vdevice on idevice destroy

> [..]
> 
> >  out_put_idev:
> > -	iommufd_put_object(ucmd->ictx, &idev->obj);
> > +	if (rc)
> > +		iommufd_put_object(ucmd->ictx, &idev->obj);
> 
> So, this actually holds both the short term user and (covertly)
> the regular user too.
> 
> Though it doesn't hurt to do that, holding the regular one seems
> to be useless, because its refcount check is way behind this new
> pre_destroy op:
> 
> 183         if (flags & REMOVE_WAIT_SHORTTERM) {                                                                                                                                                                                                                                                                                                             
> 184                 ret = iommufd_object_dec_wait_shortterm(ictx, to_destroy);
> ==>			/* pre_destroy op */
> ...
> 214         if (!refcount_dec_if_one(&obj->users)) {
> 215                 ret = -EBUSY;
> 216                 goto err_xa;
> 217         }
> 
> So, I think we could just do, exactly reflecting the comments:
> +	vdev->idev = idev;
> +	refcount_inc(&idev->obj.shortterm_users);

I think this makes things more clear, we have 3 types of refcounts:

1. shortterm_users + 1, users + 1, temporary refcount, can't be
   revoked by referenced object.
2. users + 1, long term refcount, can't be revoked either.
3. shortterm_users + 1, can be revoked by referenced object.

> 
> Then, keep the exit patch unchanged.
> out_put_idev:
> 	iommufd_put_object(ucmd->ictx, &idev->obj);

Yeah, this is the part I like the most.

Thanks,
Yilun

> 
> Thanks
> Nicolin