linux-kernel - Re: [PATCH v1 02/20] iommu: Introduce a test

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: <20251027232310.GD1018328@nvidia.com>
Date: Mon, 27 Oct 2025 20:23:10 -0300
From: Jason Gunthorpe <jgg@...dia.com>
To: Nicolin Chen <nicolinc@...dia.com>
Cc: joro@...tes.org, kevin.tian@...el.com, suravee.suthikulpanit@....com,
	will@...nel.org, robin.murphy@....com, sven@...nel.org,
	j@...nau.net, jean-philippe@...aro.org,
	robin.clark@....qualcomm.com, dwmw2@...radead.org,
	baolu.lu@...ux.intel.com, yong.wu@...iatek.com,
	matthias.bgg@...il.com, angelogioacchino.delregno@...labora.com,
	tjeznach@...osinc.com, pjw@...nel.org, palmer@...belt.com,
	aou@...s.berkeley.edu, heiko@...ech.de, schnelle@...ux.ibm.com,
	mjrosato@...ux.ibm.com, wens@...e.org, jernej.skrabec@...il.com,
	samuel@...lland.org, thierry.reding@...il.com, jonathanh@...dia.com,
	iommu@...ts.linux.dev, linux-kernel@...r.kernel.org,
	asahi@...ts.linux.dev, linux-arm-kernel@...ts.infradead.org,
	linux-arm-msm@...r.kernel.org, linux-mediatek@...ts.infradead.org,
	linux-riscv@...ts.infradead.org, linux-rockchip@...ts.infradead.org,
	linux-s390@...r.kernel.org, linux-sunxi@...ts.linux.dev,
	linux-tegra@...r.kernel.org, virtualization@...ts.linux.dev,
	patches@...ts.linux.dev
Subject: Re: [PATCH v1 02/20] iommu: Introduce a test_dev domain op and an
 internal helper

On Mon, Oct 20, 2025 at 11:51:49AM -0700, Nicolin Chen wrote:
> On Mon, Oct 20, 2025 at 01:27:36PM -0300, Jason Gunthorpe wrote:
> > On Sun, Oct 12, 2025 at 05:04:59PM -0700, Nicolin Chen wrote:
> > 
> > > And keep them within the group->mutex, so drivers can simply move all the
> > > sanity and compatibility tests from their attach_dev callbacks to the new
> > > test_dev callbacks without concerning about a race condition.
> > 
> > I'm not sure about this.. For the problem we are trying to solve this
> > would be racy as the test would be done and the group mutex
> > unlocked. Then later it will be re-tested and attached.
> 
> Oh right, we'll have to retest in iommu_dev_reset_done(). I missed
> that.
> 
> > > @@ -751,6 +760,8 @@ struct iommu_ops {
> > >   * @free: Release the domain after use.
> > >   */
> > >  struct iommu_domain_ops {
> > > +	int (*test_dev)(struct iommu_domain *domain, struct device *dev,
> > > +			ioasid_t pasid, struct iommu_domain *old);
> > 
> > Because of the starting remark I'm skeptical that old should be
> > included here.
> 
> Hmm, the followings functions sanitizes "old":
>  - qcom_iommu_identity_attach() drivers/iommu/arm/arm-smmu/qcom_iommu.c

That shouldn't be copied over to test??

        if (domain == identity_domain || !domain)
                return 0;

That is just optimizing away the attach if it has nothing to do

        qcom_domain = to_qcom_iommu_domain(domain);
        if (WARN_ON(!qcom_domain->iommu))
                return -EINVAL;

That can't never happen

>  - iommu_sva_set_dev_pasid() in drivers/iommu/amd/pasid.c

Its broken, you are not required by API to detach a domain before
setting a new one. Keep it in attach, hope someone fixes this driver
someday.

Jason