Skip to content

Commit

Permalink
vfio/type1: Fix task tracking for QEMU vCPU hotplug
Browse files Browse the repository at this point in the history 
MAP_DMA ioctls might be called from various threads within a process,
for example when using QEMU, the vCPU threads are often generating
these calls and we therefore take a reference to that vCPU task.
However, QEMU also supports vCPU hotplug on some machines and the task
that called MAP_DMA may have exited by the time UNMAP_DMA is called,
resulting in the mm_struct pointer being NULL and thus a failure to
match against the existing mapping.

To resolve this, we instead take a reference to the thread
group_leader, which has the same mm_struct and resource limits, but
is less likely exit, at least in the QEMU case.  A difficulty here is
guaranteeing that the capabilities of the group_leader match that of
the calling thread, which we resolve by tracking CAP_IPC_LOCK at the
time of calling rather than at an indeterminate time in the future.
Potentially this also results in better efficiency as this is now
recorded once per MAP_DMA ioctl.

Reported-by: Xu Yandong <xuyandong2@huawei.com>
Signed-off-by: Alex Williamson <alex.williamson@redhat.com>
  • Loading branch information
@awilliam
awilliam committed Jun 8, 2018
1 parent 29dcea8 commit 48d8476
Showing 1 changed file with 47 additions and 26 deletions.
73 changes: 47 additions & 26 deletions drivers/vfio/vfio_iommu_type1.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,7 @@ struct vfio_dma {
size_t size; /* Map size (bytes) */
int prot; /* IOMMU_READ/WRITE */
bool iommu_mapped;
bool lock_cap; /* capable(CAP_IPC_LOCK) */
struct task_struct *task;
struct rb_root pfn_list; /* Ex-user pinned pfn list */
};
Expand Down Expand Up @@ -253,29 +254,25 @@ static int vfio_iova_put_vfio_pfn(struct vfio_dma *dma, struct vfio_pfn *vpfn)
return ret;
}

static int vfio_lock_acct(struct task_struct *task, long npage, bool *lock_cap)
static int vfio_lock_acct(struct vfio_dma *dma, long npage, bool async)
{
struct mm_struct *mm;
bool is_current;
int ret;

if (!npage)
return 0;

is_current = (task->mm == current->mm);

mm = is_current ? task->mm : get_task_mm(task);
mm = async ? get_task_mm(dma->task) : dma->task->mm;
if (!mm)
return -ESRCH; /* process exited */

ret = down_write_killable(&mm->mmap_sem);
if (!ret) {
if (npage > 0) {
if (lock_cap ? !*lock_cap :
!has_capability(task, CAP_IPC_LOCK)) {
if (!dma->lock_cap) {
unsigned long limit;

limit = task_rlimit(task,
limit = task_rlimit(dma->task,
RLIMIT_MEMLOCK) >> PAGE_SHIFT;

if (mm->locked_vm + npage > limit)
Expand All @@ -289,7 +286,7 @@ static int vfio_lock_acct(struct task_struct *task, long npage, bool *lock_cap)
up_write(&mm->mmap_sem);
}

if (!is_current)
if (async)
mmput(mm);

return ret;
Expand Down Expand Up @@ -400,7 +397,7 @@ static int vaddr_get_pfn(struct mm_struct *mm, unsigned long vaddr,
*/
static long vfio_pin_pages_remote(struct vfio_dma *dma, unsigned long vaddr,
long npage, unsigned long *pfn_base,
bool lock_cap, unsigned long limit)
unsigned long limit)
{
unsigned long pfn = 0;
long ret, pinned = 0, lock_acct = 0;
Expand All @@ -423,7 +420,7 @@ static long vfio_pin_pages_remote(struct vfio_dma *dma, unsigned long vaddr,
* pages are already counted against the user.
*/
if (!rsvd && !vfio_find_vpfn(dma, iova)) {
if (!lock_cap && current->mm->locked_vm + 1 > limit) {
if (!dma->lock_cap && current->mm->locked_vm + 1 > limit) {
put_pfn(*pfn_base, dma->prot);
pr_warn("%s: RLIMIT_MEMLOCK (%ld) exceeded\n", __func__,
limit << PAGE_SHIFT);
Expand All @@ -449,7 +446,7 @@ static long vfio_pin_pages_remote(struct vfio_dma *dma, unsigned long vaddr,
}

if (!rsvd && !vfio_find_vpfn(dma, iova)) {
if (!lock_cap &&
if (!dma->lock_cap &&
current->mm->locked_vm + lock_acct + 1 > limit) {
put_pfn(pfn, dma->prot);
pr_warn("%s: RLIMIT_MEMLOCK (%ld) exceeded\n",
Expand All @@ -462,7 +459,7 @@ static long vfio_pin_pages_remote(struct vfio_dma *dma, unsigned long vaddr,
}

out:
ret = vfio_lock_acct(current, lock_acct, &lock_cap);
ret = vfio_lock_acct(dma, lock_acct, false);

unpin_out:
if (ret) {
Expand Down Expand Up @@ -493,7 +490,7 @@ static long vfio_unpin_pages_remote(struct vfio_dma *dma, dma_addr_t iova,
}

if (do_accounting)
vfio_lock_acct(dma->task, locked - unlocked, NULL);
vfio_lock_acct(dma, locked - unlocked, true);

return unlocked;
}
Expand All @@ -510,7 +507,7 @@ static int vfio_pin_page_external(struct vfio_dma *dma, unsigned long vaddr,

ret = vaddr_get_pfn(mm, vaddr, dma->prot, pfn_base);
if (!ret && do_accounting && !is_invalid_reserved_pfn(*pfn_base)) {
ret = vfio_lock_acct(dma->task, 1, NULL);
ret = vfio_lock_acct(dma, 1, true);
if (ret) {
put_pfn(*pfn_base, dma->prot);
if (ret == -ENOMEM)
Expand All @@ -537,7 +534,7 @@ static int vfio_unpin_page_external(struct vfio_dma *dma, dma_addr_t iova,
unlocked = vfio_iova_put_vfio_pfn(dma, vpfn);

if (do_accounting)
vfio_lock_acct(dma->task, -unlocked, NULL);
vfio_lock_acct(dma, -unlocked, true);

return unlocked;
}
Expand Down Expand Up @@ -829,7 +826,7 @@ static long vfio_unmap_unpin(struct vfio_iommu *iommu, struct vfio_dma *dma,
unlocked += vfio_sync_unpin(dma, domain, &unmapped_region_list);

if (do_accounting) {
vfio_lock_acct(dma->task, -unlocked, NULL);
vfio_lock_acct(dma, -unlocked, true);
return 0;
}
return unlocked;
Expand Down Expand Up @@ -1044,14 +1041,12 @@ static int vfio_pin_map_dma(struct vfio_iommu *iommu, struct vfio_dma *dma,
size_t size = map_size;
long npage;
unsigned long pfn, limit = rlimit(RLIMIT_MEMLOCK) >> PAGE_SHIFT;
bool lock_cap = capable(CAP_IPC_LOCK);
int ret = 0;

while (size) {
/* Pin a contiguous chunk of memory */
npage = vfio_pin_pages_remote(dma, vaddr + dma->size,
size >> PAGE_SHIFT, &pfn,
lock_cap, limit);
size >> PAGE_SHIFT, &pfn, limit);
if (npage <= 0) {
WARN_ON(!npage);
ret = (int)npage;
Expand Down Expand Up @@ -1126,8 +1121,36 @@ static int vfio_dma_do_map(struct vfio_iommu *iommu,
dma->iova = iova;
dma->vaddr = vaddr;
dma->prot = prot;
get_task_struct(current);
dma->task = current;

/*
* We need to be able to both add to a task's locked memory and test
* against the locked memory limit and we need to be able to do both
* outside of this call path as pinning can be asynchronous via the
* external interfaces for mdev devices. RLIMIT_MEMLOCK requires a
* task_struct and VM locked pages requires an mm_struct, however
* holding an indefinite mm reference is not recommended, therefore we
* only hold a reference to a task. We could hold a reference to
* current, however QEMU uses this call path through vCPU threads,
* which can be killed resulting in a NULL mm and failure in the unmap
* path when called via a different thread. Avoid this problem by
* using the group_leader as threads within the same group require
* both CLONE_THREAD and CLONE_VM and will therefore use the same
* mm_struct.
*
* Previously we also used the task for testing CAP_IPC_LOCK at the
* time of pinning and accounting, however has_capability() makes use
* of real_cred, a copy-on-write field, so we can't guarantee that it
* matches group_leader, or in fact that it might not change by the
* time it's evaluated. If a process were to call MAP_DMA with
* CAP_IPC_LOCK but later drop it, it doesn't make sense that they
* possibly see different results for an iommu_mapped vfio_dma vs
* externally mapped. Therefore track CAP_IPC_LOCK in vfio_dma at the
* time of calling MAP_DMA.
*/
get_task_struct(current->group_leader);
dma->task = current->group_leader;
dma->lock_cap = capable(CAP_IPC_LOCK);

dma->pfn_list = RB_ROOT;

/* Insert zero-sized and grow as we map chunks of it */
Expand Down Expand Up @@ -1162,7 +1185,6 @@ static int vfio_iommu_replay(struct vfio_iommu *iommu,
struct vfio_domain *d;
struct rb_node *n;
unsigned long limit = rlimit(RLIMIT_MEMLOCK) >> PAGE_SHIFT;
bool lock_cap = capable(CAP_IPC_LOCK);
int ret;

/* Arbitrarily pick the first domain in the list for lookups */
Expand Down Expand Up @@ -1209,8 +1231,7 @@ static int vfio_iommu_replay(struct vfio_iommu *iommu,

npage = vfio_pin_pages_remote(dma, vaddr,
n >> PAGE_SHIFT,
&pfn, lock_cap,
limit);
&pfn, limit);
if (npage <= 0) {
WARN_ON(!npage);
ret = (int)npage;
Expand Down Expand Up @@ -1487,7 +1508,7 @@ static void vfio_iommu_unmap_unpin_reaccount(struct vfio_iommu *iommu)
if (!is_invalid_reserved_pfn(vpfn->pfn))
locked++;
}
vfio_lock_acct(dma->task, locked - unlocked, NULL);
vfio_lock_acct(dma, locked - unlocked, true);
}
}

Expand Down

0 comments on commit 48d8476

Please sign in to comment.