linux-kernel - Re: [PATCH v4 1/3] iommu: Sort out domain user data

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: <Z8qKyvNyNU6dkN7V@Asurada-Nvidia>
Date: Thu, 6 Mar 2025 21:57:30 -0800
From: Nicolin Chen <nicolinc@...dia.com>
To: Baolu Lu <baolu.lu@...ux.intel.com>
CC: <jgg@...dia.com>, <kevin.tian@...el.com>, <robin.murphy@....com>,
	<joro@...tes.org>, <will@...nel.org>, <iommu@...ts.linux.dev>,
	<linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v4 1/3] iommu: Sort out domain user data

Hi Baolu,

On Fri, Mar 07, 2025 at 10:28:20AM +0800, Baolu Lu wrote:
> On 3/7/25 05:00, Nicolin Chen wrote:
> > From: Robin Murphy<robin.murphy@....com>

Robin had remarks here, wrt iommu_set_fault_handler():

> > The fact is that all these cookie types are
> > mutually exclusive, in the sense that combining them makes zero sense
> > and/or would be catastrophic (iommu_set_fault_handler() on an SVA
> > domain, anyone?) - the only combination which*might* be reasonable is
> > perhaps a fault handler and an MSI cookie, but nobody's doing that at
> > the moment, so let's rule it out as well for the sake of being clear and
> > robust.
[...]
> > @@ -224,10 +234,10 @@ struct iommu_domain {
> >   		      phys_addr_t msi_addr);
> >   #endif
> > -	union { /* Pointer usable by owner of the domain */
> > -		struct iommufd_hw_pagetable *iommufd_hwpt; /* iommufd */
> > -	};
> > -	union { /* Fault handler */
> > +	union { /* cookie */
> > +		struct iommu_dma_cookie *iova_cookie;
> > +		struct iommu_dma_msi_cookie *msi_cookie;
> > +		struct iommufd_hw_pagetable *iommufd_hwpt;
> >   		struct {
> >   			iommu_fault_handler_t handler;
> >   			void *handler_token;
> 
> My feeling is that IOMMU_COOKIE_FAULT_HANDLER isn't exclusive to
> IOMMU_COOKIE_DMA_IOVA; both might be used for kernel DMA with a paging
> domain.
> 
> I am afraid that iommu_set_fault_handler() doesn't work anymore as the
> domain's cookie type has already been set to IOMMU_COOKIE_DMA_IOVA.

All three existing iommu_set_fault_handler() callers in the tree
are UNMANAGED domain users:
   5    451  drivers/gpu/drm/msm/msm_iommu.c <<msm_iommu_gpu_new>>
             iommu_set_fault_handler(iommu->domain, msm_fault_handler, iommu);
   6    453  drivers/infiniband/hw/usnic/usnic_uiom.c <<usnic_uiom_alloc_pd>>
             iommu_set_fault_handler(pd->domain, usnic_uiom_dma_fault, NULL);
   8    118  drivers/remoteproc/remoteproc_core.c <<rproc_enable_iommu>>
             iommu_set_fault_handler(domain, rproc_iommu_fault, rproc);

On the other hand, IOMMU_COOKIE_DMA_IOVA is a private cookie for
dma-iommu only.

So, I think we are probably fine?

Thanks
Nicolin