linux-kernel - Re: [PATCH v4 05/11] iommu/sva: Assign a PASID to mm on PASID allocation and free it on mm exit

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: <tencent_5151C34BC7D86055C4C7768C1A4653505A06@qq.com>
Date:   Mon, 25 Apr 2022 10:57:52 +0800
From:   "zhangfei.gao@...mail.com" <zhangfei.gao@...mail.com>
To:     Dave Hansen <dave.hansen@...el.com>,
        Jean-Philippe Brucker <jean-philippe@...aro.org>
Cc:     Joerg Roedel <joro@...tes.org>, Fenghua Yu <fenghua.yu@...el.com>,
        Ravi V Shankar <ravi.v.shankar@...el.com>,
        Tony Luck <tony.luck@...el.com>,
        Ashok Raj <ashok.raj@...el.com>,
        Peter Zijlstra <peterz@...radead.org>,
        Dave Hansen <dave.hansen@...ux.intel.com>,
        x86 <x86@...nel.org>,
        linux-kernel <linux-kernel@...r.kernel.org>,
        iommu <iommu@...ts.linux-foundation.org>,
        Ingo Molnar <mingo@...hat.com>, Borislav Petkov <bp@...en8.de>,
        Andy Lutomirski <luto@...nel.org>,
        Josh Poimboeuf <jpoimboe@...hat.com>,
        Thomas Gleixner <tglx@...utronix.de>,
        zhangfei <zhangfei.gao@...aro.org>
Subject: Re: [PATCH v4 05/11] iommu/sva: Assign a PASID to mm on PASID
 allocation and free it on mm exit

Hi, Dave

On 2022/4/12 下午11:35, Dave Hansen wrote:
> On 4/12/22 08:10, Jean-Philippe Brucker wrote:
>>> I wonder if the Intel and ARM IOMMU code differ in the way they keep
>>> references to the mm, or if this affects Intel as well, but we just
>>> haven't tested the code enough.
>> The Arm code was written expecting the PASID to be freed on unbind(), not
>> mm exit. I missed the change of behavior, sorry (I thought your plan was
>> to extend PASID lifetime, not shorten it?) but as is it seems very broken.
>> For example in the iommu_sva_unbind_device(), we have
>> arm_smmu_mmu_notifier_put() clearing the PASID table entry for
>> "mm->pasid", which is going to end badly if the PASID has been cleared or
>> reallocated. We can't clear the PASID entry in mm exit because at that
>> point the device may still be issuing DMA for that PASID and we need to
>> quiesce the entry rather than deactivate it.
> I think we ended up flipping some of this around on the Intel side.
> Instead of having to quiesce the device on mm exit, we don't let the mm
> exit until the device is done.
>
> When you program the pasid into the device, it's a lot like when you
> create a thread.  We bump the reference count on the mm when we program
> the page table pointer into a CPU.  We drop the thread's reference to
> the mm when the thread exits and will no longer be using the page tables.
>
> Same thing with pasids.  We bump the refcount on the mm when the pasid
> is programmed into the device.  Once the device is done with the mm, we
> drop the mm.
>
> Basically, instead of recounting the pasid itself, we just refcount the mm.
This has issue, since refcount the mm will block  fops_release to be 
called, where unbind may really happen.

For example, user driver are ended unexpectedly,
usually system will end all applications via close fd -> fops_release -> 
unbind may happen here.
Now mmget is called, fops_release -> unbind has NO chance to be called, 
so ioasid can NOT be freed.

Thanks