linux-kernel - Re: [PATCH v3 2/8] iommu/sva: Use iopf domain attach/detach interface

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: <20240308174605.GV9225@ziepe.ca>
Date: Fri, 8 Mar 2024 13:46:05 -0400
From: Jason Gunthorpe <jgg@...pe.ca>
To: Lu Baolu <baolu.lu@...ux.intel.com>
Cc: Kevin Tian <kevin.tian@...el.com>, Joerg Roedel <joro@...tes.org>,
	Will Deacon <will@...nel.org>, Robin Murphy <robin.murphy@....com>,
	Jean-Philippe Brucker <jean-philippe@...aro.org>,
	Nicolin Chen <nicolinc@...dia.com>, Yi Liu <yi.l.liu@...el.com>,
	Jacob Pan <jacob.jun.pan@...ux.intel.com>,
	Joel Granados <j.granados@...sung.com>, iommu@...ts.linux.dev,
	virtualization@...ts.linux-foundation.org,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH v3 2/8] iommu/sva: Use iopf domain attach/detach interface

On Mon, Jan 22, 2024 at 03:38:57PM +0800, Lu Baolu wrote:
> @@ -215,7 +202,23 @@ static struct iopf_group *iopf_group_alloc(struct iommu_fault_param *iopf_param,
>  		group = abort_group;
>  	}
>  
> +	cookie = iopf_pasid_cookie_get(iopf_param->dev, pasid);
> +	if (!cookie && pasid != IOMMU_NO_PASID)
> +		cookie = iopf_pasid_cookie_get(iopf_param->dev, IOMMU_NO_PASID);
> +	if (IS_ERR(cookie) || !cookie) {
> +		/*
> +		 * The PASID of this device was not attached by an I/O-capable
> +		 * domain. Ask the caller to abort handling of this fault.
> +		 * Otherwise, the reference count will be switched to the new
> +		 * iopf group and will be released in iopf_free_group().
> +		 */
> +		kfree(group);
> +		group = abort_group;
> +		cookie = NULL;
> +	}

If this is the main point of the cookie mechansim - why not just have
a refcount inside the domain itself?

I'm really having a hard time making sense of this cookie thing, we
have a lifetime issue on the domain pointer, why is adding another
structure the answer?

I see we also need to stick a pointer in the domain for iommufd to get
back to the hwpt, but that doesn't seem to need such a big system to
accomplish - just add a void *. It would make sense for the domain to
have some optional pointer to a struct for all the fault related data
that becomes allocated when a PRI domain is created..

Also, I thought we'd basically said that domain detatch is supposed to
flush any open PRIs before returning, what happened to that as a
solution to the lifetime problem?

Regardless I think we should split this into two series - improve the PRI
lifetime model for domains and then put iommufd on top of it..

Jason