[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <41ed3405-66d9-0cde-fc01-b3eacb85a081@intel.com>
Date: Tue, 12 Apr 2022 07:36:21 -0700
From: Dave Hansen <dave.hansen@...el.com>
To: "zhangfei.gao@...mail.com" <zhangfei.gao@...mail.com>,
Joerg Roedel <joro@...tes.org>,
Fenghua Yu <fenghua.yu@...el.com>,
jean-philippe <jean-philippe@...aro.org>
Cc: 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>
Subject: Re: [PATCH v4 05/11] iommu/sva: Assign a PASID to mm on PASID
allocation and free it on mm exit
On 4/12/22 00:04, zhangfei.gao@...mail.com wrote:
> master process quit, mmput -> mm_pasid_drop->ioasid_free
> But this ignore driver's iommu_sva_unbind_device function,
> iommu_sva_bind_device and iommu_sva_unbind_device are not pair, So
> driver does not know ioasid is freed.
>
> Any suggestion?
It sounds like you're saying that the device is still abound to the
PASID even though the mm has exited and freed the PASID. This is
essentially a use-after-free for the PASID. Right?
The right thing to do here is to have the PASID code hold a reference on
the mm. The mm "owns" the PASID for its entire lifetime and if anything
needs the PASID to live longer, its only recourse for doing that is via
an mmget(). I _think_ mmget() is the right thing as opposed to mmgrab()
because the PASID users actually need the page tables to be around.
This would still be nice to confirm with some traces of fork()/exit()
and the iommu_sva_{bind,unbind} and ioasid_{alloc,free} functions.
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.
Powered by blists - more mailing lists