linux-kernel - Re: [PATCH v5 4/8] 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: <20250715133737.GO2067380@nvidia.com>
Date: Tue, 15 Jul 2025 10:37:37 -0300
From: Jason Gunthorpe <jgg@...dia.com>
To: Xu Yilun <yilun.xu@...ux.intel.com>
Cc: 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,
	nicolinc@...dia.com, aik@....com, dan.j.williams@...el.com,
	baolu.lu@...ux.intel.com, yilun.xu@...el.com
Subject: Re: [PATCH v5 4/8] iommufd: Destroy vdevice on idevice destroy

On Tue, Jul 15, 2025 at 02:32:41PM +0800, Xu Yilun wrote:
> Destroy iommufd_vdevice (vdev) on iommufd_idevice (idev) destruction so
> that vdev can't outlive idev.
> 
> idev represents the physical device bound to iommufd, while the vdev
> represents the virtual instance of the physical device in the VM. The
> lifecycle of the vdev should not be longer than idev. This doesn't
> cause real problem on existing use cases cause vdev doesn't impact the
> physical device, only provides virtualization information. But to
> extend vdev for Confidential Computing (CC), there are needs to do
> secure configuration for the vdev, e.g. TSM Bind/Unbind. These
> configurations should be rolled back on idev destroy, or the external
> driver (VFIO) functionality may be impact.
> 
> The idev is created by external driver so its destruction can't fail.
> The idev implements pre_destroy() op to actively remove its associated
> vdev before destroying itself. There are 3 cases on idev pre_destroy():
> 
>   1. vdev is already destroyed by userspace. No extra handling needed.
>   2. vdev is still alive. Use iommufd_object_tombstone_user() to
>      destroy vdev and tombstone the vdev ID.
>   3. vdev is being destroyed by userspace. The vdev ID is already
>      freed, but vdev destroy handler is not completed. This requires
>      multi-threads syncing - vdev holds idev's short term users
>      reference until vdev destruction completes, idev leverages
>      existing wait_shortterm mechanism for syncing.
> 
> idev should also block any new reference to it after pre_destroy(),
> or the following wait shortterm would timeout. Introduce a 'destroying'
> flag, set it to true on idev pre_destroy(). Any attempt to reference
> idev should honor this flag under the protection of
> idev->igroup->lock.
> 
> Originally-by: Nicolin Chen <nicolinc@...dia.com>
> Suggested-by: Jason Gunthorpe <jgg@...dia.com>
> Reviewed-by: Kevin Tian <kevin.tian@...el.com>
> Reviewed-by: Nicolin Chen <nicolinc@...dia.com>
> Co-developed-by: Aneesh Kumar K.V (Arm) <aneesh.kumar@...nel.org>
> Signed-off-by: Aneesh Kumar K.V (Arm) <aneesh.kumar@...nel.org>
> Signed-off-by: Xu Yilun <yilun.xu@...ux.intel.com>
> ---
>  drivers/iommu/iommufd/device.c          | 51 ++++++++++++++++++++++++
>  drivers/iommu/iommufd/iommufd_private.h | 12 ++++++
>  drivers/iommu/iommufd/main.c            |  2 +
>  drivers/iommu/iommufd/viommu.c          | 52 +++++++++++++++++++++++--
>  include/linux/iommufd.h                 |  1 +
>  include/uapi/linux/iommufd.h            |  5 +++
>  6 files changed, 119 insertions(+), 4 deletions(-)

Reviewed-by: Jason Gunthorpe <jgg@...dia.com>

Jason