On 12/16/2022 9:09 AM, Jason Gunthorpe wrote: > On Thu, Dec 15, 2022 at 01:56:59PM -0800, Steve Sistare wrote: >> When a vfio container is preserved across exec, the task does not change, >> but it gets a new mm with locked_vm=0. If the user later unmaps a dma >> mapping, locked_vm underflows to a large unsigned value, and a subsequent >> dma map request fails with ENOMEM in __account_locked_vm. >> >> To avoid underflow, grab and save the mm at the time a dma is mapped. >> Use that mm when adjusting locked_vm, rather than re-acquiring the saved >> task's mm, which may have changed. If the saved mm is dead, do nothing. >> >> Signed-off-by: Steve Sistare <steven.sistare@xxxxxxxxxx> >> --- >> drivers/vfio/vfio_iommu_type1.c | 17 ++++++++++------- >> 1 file changed, 10 insertions(+), 7 deletions(-) > > Add fixes lines and a CC stable This predates the update vaddr functionality, so AFAICT: Fixes: 73fa0d10d077 ("vfio: Type1 IOMMU implementation") I'll wait on cc'ing stable until alex has chimed in. > The subject should be more like 'vfio/typ1: Prevent corruption of mm->locked_vm via exec()' Underflow is a more precise description of the first corruption. How about: vfio/type1: Prevent underflow of locked_vm via exec() >> @@ -1687,6 +1689,8 @@ static int vfio_dma_do_map(struct vfio_iommu *iommu, >> get_task_struct(current->group_leader); >> dma->task = current->group_leader; >> dma->lock_cap = capable(CAP_IPC_LOCK); >> + dma->mm = dma->task->mm; > > This should be current->mm, current->group_leader->mm is not quite the > same thing (and maybe another bug, I'm not sure) When are they different -- when the leader is a zombie? BTW I just noticed I need to update the comments about mm preceding these lines. - Steve