Re: [PATCH v5 01/27] iommu/arm-smmu-v3: Do not allow a SVA domain to be set on the wrong PASID

[Jason Gunthorpe <jgg@nvidia.com>]

On Tue, Mar 26, 2024 at 07:06:18PM +0000, Mostafa Saleh wrote:
> On Tue, Mar 26, 2024 at 03:30:16PM -0300, Jason Gunthorpe wrote:
> > On Fri, Mar 22, 2024 at 05:48:52PM +0000, Mostafa Saleh wrote:
> > > Hi Jason,
> > > 
> > > On Mon, Mar 04, 2024 at 07:43:49PM -0400, Jason Gunthorpe wrote:
> > > > The SVA code is wired to assume that the SVA is programmed onto the
> > > > mm->pasid. The current core code always does this, so it is fine.
> > > > 
> > > > Add a check for clarity.
> > > > 
> > > > Tested-by: Nicolin Chen <nicolinc@nvidia.com>
> > > > Signed-off-by: Jason Gunthorpe <jgg@nvidia.com>
> > > > ---
> > > >  drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c | 3 +++
> > > >  1 file changed, 3 insertions(+)
> > > > 
> > > > diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c
> > > > index 2610e82c0ecd0d..347c2fdd865c1a 100644
> > > > --- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c
> > > > +++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c
> > > > @@ -581,6 +581,9 @@ static int arm_smmu_sva_set_dev_pasid(struct iommu_domain *domain,
> > > >  	int ret = 0;
> > > >  	struct mm_struct *mm = domain->mm;
> > > >  
> > > > +	if (mm_get_enqcmd_pasid(mm) != id)
> > > > +		return -EINVAL;
> > > > +
> > > I am not sure if that is needed, the only caller in the tree is the IOMMU code
> > > and it does the right thing, as that check is removed later anyway, I don’t
> > > think this patch adds much.
> > 
> > It really should be backported, when we get drivers that do other
> > things it creates a hazard. I've added a fixes line.
> 
> Maybe I am misunderstanding the case, but AFAIU, the only caller for this is
> iommu_sva_bind_device() which is the function populating the pasid, so this
> condition should never hit with the current code.

Yes, but iommufd is going to break this soon.

> And Linux won’t backport new drivers, so there is no need to do that?

Heh, you never know what -stable will backport and the distros are
quite a bit more lax. Seems nicer to fail quickly than get all
corrupted.

Jason